SF: Delete SchedulerUtils.{h,cpp}
Remove dead code, inline trivial helpers, and replace the tracing
helpers with ftl::Concat.
Make the TimerDispatch trace slightly less cryptic.
Bug: 185535769
Test: TimerDispatch traces
Change-Id: I9b36d8344f4bc926fc9960ab6ac605eac95f3dac
diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp
index 74a2ca7..0efc28b 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp
@@ -33,7 +33,6 @@
#include "../Layer.h"
#include "LayerInfo.h"
-#include "SchedulerUtils.h"
namespace android::scheduler {
diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h
index 2d88a4f..8a3b0b9 100644
--- a/services/surfaceflinger/Scheduler/LayerInfo.h
+++ b/services/surfaceflinger/Scheduler/LayerInfo.h
@@ -29,7 +29,6 @@
#include "LayerHistory.h"
#include "RefreshRateConfigs.h"
-#include "SchedulerUtils.h"
namespace android {
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index a94952f..42dc539 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -618,7 +618,7 @@
const auto [refreshRate, score] = *i;
ALOGV("%s scores %.2f", refreshRate->getName().c_str(), score);
- ATRACE_INT(refreshRate->getName().c_str(), round<int>(score * 100));
+ ATRACE_INT(refreshRate->getName().c_str(), static_cast<int>(std::round(score * 100)));
if (score > max * (1 + kEpsilon)) {
max = score;
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index fc45d2b..de5afb7 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -30,7 +30,6 @@
#include "DisplayHardware/DisplayMode.h"
#include "DisplayHardware/HWComposer.h"
#include "Scheduler/OneShotTimer.h"
-#include "Scheduler/SchedulerUtils.h"
#include "Scheduler/StrongTyping.h"
namespace android::scheduler {
diff --git a/services/surfaceflinger/Scheduler/RefreshRateStats.h b/services/surfaceflinger/Scheduler/RefreshRateStats.h
index 23ebb06..f1ad755 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateStats.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateStats.h
@@ -25,7 +25,6 @@
#include <scheduler/Fps.h>
-#include "Scheduler/SchedulerUtils.h"
#include "TimeStats/TimeStats.h"
namespace android::scheduler {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 4173088..d4b3917 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -44,7 +44,6 @@
#include "EventThread.h"
#include "InjectVSyncSource.h"
#include "OneShotTimer.h"
-#include "SchedulerUtils.h"
#include "SurfaceFlingerProperties.h"
#include "VSyncPredictor.h"
#include "VSyncReactor.h"
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 548c34b..2f3d952 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -17,6 +17,7 @@
#pragma once
#include <atomic>
+#include <cstdint>
#include <functional>
#include <future>
#include <memory>
@@ -38,9 +39,37 @@
#include "MessageQueue.h"
#include "OneShotTimer.h"
#include "RefreshRateConfigs.h"
-#include "SchedulerUtils.h"
#include "VsyncSchedule.h"
+namespace android::scheduler {
+
+// Opaque handle to scheduler connection.
+struct ConnectionHandle {
+ using Id = std::uintptr_t;
+ static constexpr Id INVALID_ID = static_cast<Id>(-1);
+
+ Id id = INVALID_ID;
+
+ explicit operator bool() const { return id != INVALID_ID; }
+};
+
+inline bool operator==(ConnectionHandle lhs, ConnectionHandle rhs) {
+ return lhs.id == rhs.id;
+}
+
+} // namespace android::scheduler
+
+namespace std {
+
+template <>
+struct hash<android::scheduler::ConnectionHandle> {
+ size_t operator()(android::scheduler::ConnectionHandle handle) const {
+ return hash<android::scheduler::ConnectionHandle::Id>()(handle.id);
+ }
+};
+
+} // namespace std
+
namespace android {
class FenceTime;
diff --git a/services/surfaceflinger/Scheduler/SchedulerUtils.cpp b/services/surfaceflinger/Scheduler/SchedulerUtils.cpp
deleted file mode 100644
index e8e0444..0000000
--- a/services/surfaceflinger/Scheduler/SchedulerUtils.cpp
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "SchedulerUtils.h"
-
-#include <cinttypes>
-#include <numeric>
-#include <unordered_map>
-#include <vector>
-
-namespace android {
-namespace scheduler {
-
-int64_t calculate_median(std::vector<int64_t>* v) {
- if (!v || v->empty()) {
- return 0;
- }
-
- size_t n = v->size() / 2;
- nth_element(v->begin(), v->begin() + static_cast<long>(n), v->end());
- return v->at(n);
-}
-
-} // namespace scheduler
-} // namespace android
-
diff --git a/services/surfaceflinger/Scheduler/SchedulerUtils.h b/services/surfaceflinger/Scheduler/SchedulerUtils.h
deleted file mode 100644
index 04a4cd1..0000000
--- a/services/surfaceflinger/Scheduler/SchedulerUtils.h
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Copyright 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <utils/Timers.h>
-#include <cinttypes>
-#include <numeric>
-#include <unordered_map>
-#include <vector>
-
-namespace android::scheduler {
-
-// Opaque handle to scheduler connection.
-struct ConnectionHandle {
- using Id = std::uintptr_t;
- static constexpr Id INVALID_ID = static_cast<Id>(-1);
-
- Id id = INVALID_ID;
-
- explicit operator bool() const { return id != INVALID_ID; }
-};
-
-inline bool operator==(ConnectionHandle lhs, ConnectionHandle rhs) {
- return lhs.id == rhs.id;
-}
-
-// Calculates the statistical mean (average) in the data structure (array, vector). The
-// function does not modify the contents of the array.
-template <typename T>
-auto calculate_mean(const T& v) {
- using V = typename T::value_type;
- V sum = std::accumulate(v.begin(), v.end(), static_cast<V>(0));
- return sum / static_cast<V>(v.size());
-}
-
-// Calculates the statistical median in the vector. Return 0 if the vector is empty. The
-// function modifies the vector contents.
-int64_t calculate_median(std::vector<int64_t>* v);
-
-// Calculates the statistical mode in the vector. Return 0 if the vector is empty.
-template <typename T>
-auto calculate_mode(const T& v) {
- if (v.empty()) {
- return 0;
- }
-
- // Create a map with all the counts for the indivicual values in the vector.
- std::unordered_map<int64_t, int> counts;
- for (int64_t value : v) {
- counts[value]++;
- }
-
- // Sort the map, and return the number with the highest count. If two numbers have
- // the same count, first one is returned.
- using ValueType = const decltype(counts)::value_type&;
- const auto compareCounts = [](ValueType l, ValueType r) { return l.second <= r.second; };
- return static_cast<int>(std::max_element(counts.begin(), counts.end(), compareCounts)->first);
-}
-
-template <class T, size_t N>
-constexpr size_t arrayLen(T (&)[N]) {
- return N;
-}
-
-static constexpr size_t max64print = std::numeric_limits<nsecs_t>::digits10 + 1;
-
-template <typename T>
-static inline T round(float f) {
- return static_cast<T>(std::round(f));
-}
-
-} // namespace android::scheduler
-
-namespace std {
-
-template <>
-struct hash<android::scheduler::ConnectionHandle> {
- size_t operator()(android::scheduler::ConnectionHandle handle) const {
- return hash<android::scheduler::ConnectionHandle::Id>()(handle.id);
- }
-};
-
-} // namespace std
diff --git a/services/surfaceflinger/Scheduler/Timer.cpp b/services/surfaceflinger/Scheduler/Timer.cpp
index 22c3a70..10383ea 100644
--- a/services/surfaceflinger/Scheduler/Timer.cpp
+++ b/services/surfaceflinger/Scheduler/Timer.cpp
@@ -25,16 +25,14 @@
#include <sys/timerfd.h>
#include <sys/unistd.h>
-#include <android-base/stringprintf.h>
+#include <ftl/concat.h>
#include <ftl/enum.h>
#include <log/log.h>
#include <utils/Trace.h>
-#include "SchedulerUtils.h"
#include "Timer.h"
namespace android::scheduler {
-using base::StringAppendF;
static constexpr size_t kReadPipe = 0;
static constexpr size_t kWritePipe = 1;
@@ -165,9 +163,6 @@
}
uint64_t iteration = 0;
- char const traceNamePrefix[] = "TimerIteration #";
- static constexpr size_t maxlen = arrayLen(traceNamePrefix) + max64print;
- std::array<char, maxlen> str_buffer;
while (true) {
setDebugState(DebugState::Waiting);
@@ -176,9 +171,8 @@
setDebugState(DebugState::Running);
if (ATRACE_ENABLED()) {
- snprintf(str_buffer.data(), str_buffer.size(), "%s%" PRIu64, traceNamePrefix,
- iteration++);
- ATRACE_NAME(str_buffer.data());
+ ftl::Concat trace("TimerIteration #", iteration++);
+ ATRACE_NAME(trace.c_str());
}
if (nfds == -1) {
@@ -221,7 +215,9 @@
void Timer::dump(std::string& result) const {
std::lock_guard lock(mMutex);
- StringAppendF(&result, "\t\tDebugState: %s\n", ftl::enum_string(mDebugState).c_str());
+ result.append("\t\tDebugState: ");
+ result.append(ftl::enum_string(mDebugState));
+ result.push_back('\n');
}
} // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index b805bf6..c0646ad 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -15,15 +15,19 @@
*/
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-#include <android-base/stringprintf.h>
-#include <utils/Trace.h>
+
#include <vector>
+#include <android-base/stringprintf.h>
+#include <ftl/concat.h>
+#include <utils/Trace.h>
+
#include "TimeKeeper.h"
#include "VSyncDispatchTimerQueue.h"
#include "VSyncTracker.h"
namespace android::scheduler {
+
using base::StringAppendF;
namespace {
@@ -222,16 +226,6 @@
rearmTimerSkippingUpdateFor(now, mCallbacks.end());
}
-void VSyncDispatchTimerQueue::TraceBuffer::note(std::string_view name, nsecs_t alarmIn,
- nsecs_t vsFor) {
- if (ATRACE_ENABLED()) {
- snprintf(str_buffer.data(), str_buffer.size(), "%.4s%s%" PRId64 "%s%" PRId64,
- name.substr(0, kMaxNamePrint).data(), kTraceNamePrefix, alarmIn,
- kTraceNameSeparator, vsFor);
- }
- ATRACE_NAME(str_buffer.data());
-}
-
void VSyncDispatchTimerQueue::rearmTimerSkippingUpdateFor(
nsecs_t now, CallbackMap::iterator const& skipUpdateIt) {
std::optional<nsecs_t> min;
@@ -247,16 +241,18 @@
callback->update(mTracker, now);
}
auto const wakeupTime = *callback->wakeupTime();
- if (!min || (min && *min > wakeupTime)) {
+ if (!min || *min > wakeupTime) {
nextWakeupName = callback->name();
min = wakeupTime;
targetVsync = callback->targetVsync();
}
}
- if (min && (min < mIntendedWakeupTime)) {
- if (targetVsync && nextWakeupName) {
- mTraceBuffer.note(*nextWakeupName, *min - now, *targetVsync - now);
+ if (min && min < mIntendedWakeupTime) {
+ if (ATRACE_ENABLED() && nextWakeupName && targetVsync) {
+ ftl::Concat trace(ftl::truncated<5>(*nextWakeupName), " alarm in ", ns2us(*min - now),
+ "us; VSYNC in ", ns2us(*targetVsync - now), "us");
+ ATRACE_NAME(trace.c_str());
}
setTimer(*min, now);
} else {
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
index 26237b6..76e1caf 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
@@ -17,7 +17,6 @@
#pragma once
#include <android-base/thread_annotations.h>
-#include <array>
#include <functional>
#include <memory>
#include <mutex>
@@ -25,7 +24,6 @@
#include <string_view>
#include <unordered_map>
-#include "SchedulerUtils.h"
#include "VSyncDispatch.h"
namespace android::scheduler {
@@ -153,17 +151,6 @@
CallbackMap mCallbacks GUARDED_BY(mMutex);
nsecs_t mIntendedWakeupTime GUARDED_BY(mMutex) = kInvalidTime;
- struct TraceBuffer {
- static constexpr char const kTraceNamePrefix[] = "-alarm in:";
- static constexpr char const kTraceNameSeparator[] = " for vs:";
- static constexpr size_t kMaxNamePrint = 4;
- static constexpr size_t kNumTsPrinted = 2;
- static constexpr size_t maxlen = kMaxNamePrint + arrayLen(kTraceNamePrefix) +
- arrayLen(kTraceNameSeparator) - 1 + (kNumTsPrinted * max64print);
- std::array<char, maxlen> str_buffer;
- void note(std::string_view name, nsecs_t in, nsecs_t vs);
- } mTraceBuffer GUARDED_BY(mMutex);
-
// For debugging purposes
nsecs_t mLastTimerCallback GUARDED_BY(mMutex) = kInvalidTime;
nsecs_t mLastTimerSchedule GUARDED_BY(mMutex) = kInvalidTime;
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index e9bd92a..61d2fb7 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -18,24 +18,27 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wextra"
+#undef LOG_TAG
+#define LOG_TAG "VSyncPredictor"
+
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-//#define LOG_NDEBUG 0
-#include "VSyncPredictor.h"
+
+#include <algorithm>
+#include <chrono>
+#include <sstream>
+
#include <android-base/logging.h>
#include <android-base/stringprintf.h>
#include <cutils/compiler.h>
#include <cutils/properties.h>
#include <utils/Log.h>
#include <utils/Trace.h>
-#include <algorithm>
-#include <chrono>
-#include <sstream>
-#include "RefreshRateConfigs.h"
-#undef LOG_TAG
-#define LOG_TAG "VSyncPredictor"
+#include "RefreshRateConfigs.h"
+#include "VSyncPredictor.h"
namespace android::scheduler {
+
using base::StringAppendF;
static auto constexpr kMaxPercent = 100u;
@@ -121,7 +124,8 @@
mTimestamps[mLastTimestampIndex] = timestamp;
}
- if (mTimestamps.size() < kMinimumSamplesForPrediction) {
+ const size_t numSamples = mTimestamps.size();
+ if (numSamples < kMinimumSamplesForPrediction) {
mRateMap[mIdealPeriod] = {mIdealPeriod, 0};
return true;
}
@@ -141,36 +145,44 @@
//
// intercept = mean(Y) - slope * mean(X)
//
- std::vector<nsecs_t> vsyncTS(mTimestamps.size());
- std::vector<nsecs_t> ordinals(mTimestamps.size());
+ std::vector<nsecs_t> vsyncTS(numSamples);
+ std::vector<nsecs_t> ordinals(numSamples);
- // normalizing to the oldest timestamp cuts down on error in calculating the intercept.
- auto const oldest_ts = *std::min_element(mTimestamps.begin(), mTimestamps.end());
+ // Normalizing to the oldest timestamp cuts down on error in calculating the intercept.
+ const auto oldestTS = *std::min_element(mTimestamps.begin(), mTimestamps.end());
auto it = mRateMap.find(mIdealPeriod);
auto const currentPeriod = it->second.slope;
- // TODO (b/144707443): its important that there's some precision in the mean of the ordinals
- // for the intercept calculation, so scale the ordinals by 1000 to continue
- // fixed point calculation. Explore expanding
- // scheduler::utils::calculate_mean to have a fixed point fractional part.
- static constexpr int64_t kScalingFactor = 1000;
- for (auto i = 0u; i < mTimestamps.size(); i++) {
+ // The mean of the ordinals must be precise for the intercept calculation, so scale them up for
+ // fixed-point arithmetic.
+ constexpr int64_t kScalingFactor = 1000;
+
+ nsecs_t meanTS = 0;
+ nsecs_t meanOrdinal = 0;
+
+ for (size_t i = 0; i < numSamples; i++) {
traceInt64If("VSP-ts", mTimestamps[i]);
- vsyncTS[i] = mTimestamps[i] - oldest_ts;
- ordinals[i] = ((vsyncTS[i] + (currentPeriod / 2)) / currentPeriod) * kScalingFactor;
+ const auto timestamp = mTimestamps[i] - oldestTS;
+ vsyncTS[i] = timestamp;
+ meanTS += timestamp;
+
+ const auto ordinal = (vsyncTS[i] + currentPeriod / 2) / currentPeriod * kScalingFactor;
+ ordinals[i] = ordinal;
+ meanOrdinal += ordinal;
}
- auto meanTS = scheduler::calculate_mean(vsyncTS);
- auto meanOrdinal = scheduler::calculate_mean(ordinals);
- for (size_t i = 0; i < vsyncTS.size(); i++) {
+ meanTS /= numSamples;
+ meanOrdinal /= numSamples;
+
+ for (size_t i = 0; i < numSamples; i++) {
vsyncTS[i] -= meanTS;
ordinals[i] -= meanOrdinal;
}
- auto top = 0ll;
- auto bottom = 0ll;
- for (size_t i = 0; i < vsyncTS.size(); i++) {
+ nsecs_t top = 0;
+ nsecs_t bottom = 0;
+ for (size_t i = 0; i < numSamples; i++) {
top += vsyncTS[i] * ordinals[i];
bottom += ordinals[i] * ordinals[i];
}
@@ -365,4 +377,4 @@
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wextra"
\ No newline at end of file
+#pragma clang diagnostic pop // ignored "-Wextra"
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 40e6944..cfaf7d6 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -16,11 +16,12 @@
#pragma once
-#include <android-base/thread_annotations.h>
#include <mutex>
#include <unordered_map>
#include <vector>
-#include "SchedulerUtils.h"
+
+#include <android-base/thread_annotations.h>
+
#include "VSyncTracker.h"
namespace android::scheduler {