SurfaceFlinger: VSyncReactor should not know VSyncDispatch
VSyncReactor doesn't use VSyncDispatch anymore with the exception
of logging the predicted vsync. Decouple those two for better logic
separation.
Test: SF unit tests
Test: examine systraces with debug.sf.show_predicted_vsync=1
Bug: 162888874
Change-Id: I368926f55a21663cdea478219bf467fc4772aadb
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 017872c..b0b5e2e 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -95,6 +95,26 @@
} // namespace
+class PredictedVsyncTracer {
+public:
+ PredictedVsyncTracer(scheduler::VSyncDispatch& dispatch)
+ : mRegistration(dispatch, std::bind(&PredictedVsyncTracer::callback, this),
+ "PredictedVsyncTracer") {
+ scheduleRegistration();
+ }
+
+private:
+ TracedOrdinal<bool> mParity = {"VSYNC-predicted", 0};
+ scheduler::VSyncCallbackRegistration mRegistration;
+
+ void scheduleRegistration() { mRegistration.schedule({0, 0, 0}); }
+
+ void callback() {
+ mParity = !mParity;
+ scheduleRegistration();
+ }
+};
+
Scheduler::Scheduler(const scheduler::RefreshRateConfigs& configs, ISchedulerCallback& callback)
: Scheduler(configs, callback,
{.supportKernelTimer = sysprop::support_kernel_idle_timer(false),
@@ -145,7 +165,11 @@
mVsyncSchedule(std::move(schedule)),
mLayerHistory(std::move(layerHistory)),
mSchedulerCallback(schedulerCallback),
- mRefreshRateConfigs(configs) {
+ mRefreshRateConfigs(configs),
+ mPredictedVsyncTracer(
+ base::GetBoolProperty("debug.sf.show_predicted_vsync", false)
+ ? std::make_unique<PredictedVsyncTracer>(*mVsyncSchedule.dispatch)
+ : nullptr) {
mSchedulerCallback.setVsyncEnabled(false);
}
@@ -163,9 +187,9 @@
// TODO(b/144707443): Tune constants.
constexpr size_t pendingFenceLimit = 20;
- auto sync = std::make_unique<scheduler::VSyncReactor>(std::move(clock), *dispatch, *tracker,
- pendingFenceLimit,
- options.supportKernelTimer);
+ auto sync =
+ std::make_unique<scheduler::VSyncReactor>(std::move(clock), *tracker, pendingFenceLimit,
+ options.supportKernelTimer);
return {std::move(sync), std::move(tracker), std::move(dispatch)};
}
@@ -600,6 +624,15 @@
mLayerHistory ? mLayerHistory->dump().c_str() : "(no layer history)");
}
+void Scheduler::dumpVSync(std::string& s) const {
+ using base::StringAppendF;
+
+ StringAppendF(&s, "VSyncReactor:\n");
+ mVsyncSchedule.sync->dump(s);
+ StringAppendF(&s, "VSyncDispatch:\n");
+ mVsyncSchedule.dispatch->dump(s);
+}
+
template <class T>
bool Scheduler::handleTimerStateChanged(T* currentState, T newState) {
HwcConfigIndexType newConfigId;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 44d0d77..9a4ac36 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -43,6 +43,7 @@
class DispSync;
class FenceTime;
class InjectVSyncSource;
+class PredictedVsyncTracer;
namespace scheduler {
class VSyncDispatch;
@@ -137,6 +138,7 @@
void dump(std::string&) const;
void dump(ConnectionHandle, std::string&) const;
+ void dumpVSync(std::string&) const;
// Get the appropriate refresh for current conditions.
std::optional<HwcConfigIndexType> getPreferredConfigId();
@@ -284,6 +286,8 @@
std::optional<hal::VsyncPeriodChangeTimeline> mLastVsyncPeriodChangeTimeline
GUARDED_BY(mVsyncTimelineLock);
static constexpr std::chrono::nanoseconds MAX_VSYNC_APPLIED_TIME = 200ms;
+
+ const std::unique_ptr<PredictedVsyncTracer> mPredictedVsyncTracer;
};
} // namespace android
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index 792313e..3c5b3f1 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -35,36 +35,11 @@
return systemTime(SYSTEM_TIME_MONOTONIC);
}
-class PredictedVsyncTracer {
-public:
- PredictedVsyncTracer(VSyncDispatch& dispatch)
- : mRegistration(dispatch, std::bind(&PredictedVsyncTracer::callback, this),
- "PredictedVsyncTracer") {
- scheduleRegistration();
- }
-
-private:
- TracedOrdinal<bool> mParity = {"VSYNC-predicted", 0};
- VSyncCallbackRegistration mRegistration;
-
- void scheduleRegistration() { mRegistration.schedule({0, 0, 0}); }
-
- void callback() {
- mParity = !mParity;
- scheduleRegistration();
- }
-};
-
-VSyncReactor::VSyncReactor(std::unique_ptr<Clock> clock, VSyncDispatch& dispatch,
- VSyncTracker& tracker, size_t pendingFenceLimit,
- bool supportKernelIdleTimer)
+VSyncReactor::VSyncReactor(std::unique_ptr<Clock> clock, VSyncTracker& tracker,
+ size_t pendingFenceLimit, bool supportKernelIdleTimer)
: mClock(std::move(clock)),
mTracker(tracker),
- mDispatch(dispatch),
mPendingLimit(pendingFenceLimit),
- mPredictedVsyncTracer(property_get_bool("debug.sf.show_predicted_vsync", false)
- ? std::make_unique<PredictedVsyncTracer>(mDispatch)
- : nullptr),
mSupportKernelIdleTimer(supportKernelIdleTimer) {}
VSyncReactor::~VSyncReactor() = default;
@@ -267,8 +242,6 @@
StringAppendF(&result, "VSyncTracker:\n");
mTracker.dump(result);
- StringAppendF(&result, "VSyncDispatch:\n");
- mDispatch.dump(result);
}
} // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.h b/services/surfaceflinger/Scheduler/VSyncReactor.h
index c6f2777..80b5232 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.h
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.h
@@ -29,13 +29,12 @@
class Clock;
class VSyncDispatch;
class VSyncTracker;
-class PredictedVsyncTracer;
// TODO (b/145217110): consider renaming.
class VSyncReactor : public android::DispSync {
public:
- VSyncReactor(std::unique_ptr<Clock> clock, VSyncDispatch& dispatch, VSyncTracker& tracker,
- size_t pendingFenceLimit, bool supportKernelIdleTimer);
+ VSyncReactor(std::unique_ptr<Clock> clock, VSyncTracker& tracker, size_t pendingFenceLimit,
+ bool supportKernelIdleTimer);
~VSyncReactor();
bool addPresentFence(const std::shared_ptr<FenceTime>& fence) final;
@@ -64,7 +63,6 @@
std::unique_ptr<Clock> const mClock;
VSyncTracker& mTracker;
- VSyncDispatch& mDispatch;
size_t const mPendingLimit;
mutable std::mutex mMutex;
@@ -77,7 +75,6 @@
std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);
std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);
- const std::unique_ptr<PredictedVsyncTracer> mPredictedVsyncTracer;
const bool mSupportKernelIdleTimer = false;
};
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index b30b0ec..ccbb569 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4352,8 +4352,7 @@
} else {
static const std::unordered_map<std::string, Dumper> dumpers = {
{"--display-id"s, dumper(&SurfaceFlinger::dumpDisplayIdentificationData)},
- {"--dispsync"s,
- dumper([this](std::string& s) { mScheduler->getPrimaryDispSync().dump(s); })},
+ {"--dispsync"s, dumper([this](std::string& s) { mScheduler->dumpVSync(s); })},
{"--edid"s, argsDumper(&SurfaceFlinger::dumpRawDisplayIdentificationData)},
{"--frame-events"s, dumper(&SurfaceFlinger::dumpFrameEventsLocked)},
{"--latency"s, argsDumper(&SurfaceFlinger::dumpStatsLocked)},
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 0c1ae14..0eae01e 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -95,16 +95,14 @@
class VSyncReactorTest : public testing::Test {
protected:
VSyncReactorTest()
- : mMockDispatch(std::make_shared<NiceMock<MockVSyncDispatch>>()),
- mMockTracker(std::make_shared<NiceMock<MockVSyncTracker>>()),
+ : mMockTracker(std::make_shared<NiceMock<MockVSyncTracker>>()),
mMockClock(std::make_shared<NiceMock<MockClock>>()),
- mReactor(std::make_unique<ClockWrapper>(mMockClock), *mMockDispatch, *mMockTracker,
- kPendingLimit, false /* supportKernelIdleTimer */) {
+ mReactor(std::make_unique<ClockWrapper>(mMockClock), *mMockTracker, kPendingLimit,
+ false /* supportKernelIdleTimer */) {
ON_CALL(*mMockClock, now()).WillByDefault(Return(mFakeNow));
ON_CALL(*mMockTracker, currentPeriod()).WillByDefault(Return(period));
}
- std::shared_ptr<MockVSyncDispatch> mMockDispatch;
std::shared_ptr<MockVSyncTracker> mMockTracker;
std::shared_ptr<MockClock> mMockClock;
static constexpr size_t kPendingLimit = 3;
@@ -498,9 +496,8 @@
TEST_F(VSyncReactorTest, periodIsMeasuredIfIgnoringComposer) {
// Create a reactor which supports the kernel idle timer
- auto idleReactor =
- VSyncReactor(std::make_unique<ClockWrapper>(mMockClock), *mMockDispatch, *mMockTracker,
- kPendingLimit, true /* supportKernelIdleTimer */);
+ auto idleReactor = VSyncReactor(std::make_unique<ClockWrapper>(mMockClock), *mMockTracker,
+ kPendingLimit, true /* supportKernelIdleTimer */);
bool periodFlushed = true;
EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(4);