commit c94ca83964accc75b08060a265abe2a346de1c2d
author Kevin DuBois <kevindubois@google.com> Tue Nov 26 12:56:24 2019 -0800
committer Kevin DuBois <kevindubois@google.com> Thu Dec 19 09:08:07 2019 -0800
tree 2beb0bcb75490a4b0ee392a770f76f509f30ef41
parent f91e9236bc80482b1c327f8fb7306c5b0b419e65

SF: VSyncDispatch: correct vsync prediction drift

Refine VSyncDispatch::schedule implementation so that refining
the prediction by small amounts would not lead to skipped callbacks.
The current implementation did not account for a case where
a valid vsync callback would be skipped. (exposed
in unit testing). Like the rest of VSyncDispatch, this
code is flagged off (ie, latent, not production code yet)

Fixes: 145213786
Bug: 146050690
Test: 6 new unit tests, 3 unit test change
Test: validation via systrace

Change-Id: I400fc5e3c181b49ab237b0dd0da2a62e38522fa0

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index a79fe98..48f2abb 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -29,8 +29,13 @@
 TimeKeeper::~TimeKeeper() = default;
 
 VSyncDispatchTimerQueueEntry::VSyncDispatchTimerQueueEntry(std::string const& name,
-                                                           std::function<void(nsecs_t)> const& cb)
-      : mName(name), mCallback(cb), mWorkDuration(0), mEarliestVsync(0) {}
+                                                           std::function<void(nsecs_t)> const& cb,
+                                                           nsecs_t minVsyncDistance)
+      : mName(name),
+        mCallback(cb),
+        mWorkDuration(0),
+        mEarliestVsync(0),
+        mMinVsyncDistance(minVsyncDistance) {}
 
 std::optional<nsecs_t> VSyncDispatchTimerQueueEntry::lastExecutedVsyncTarget() const {
     return mLastDispatchTime;
@@ -49,18 +54,28 @@
 
 ScheduleResult VSyncDispatchTimerQueueEntry::schedule(nsecs_t workDuration, nsecs_t earliestVsync,
                                                       VSyncTracker& tracker, nsecs_t now) {
-    auto const nextVsyncTime =
+    auto nextVsyncTime =
             tracker.nextAnticipatedVSyncTimeFrom(std::max(earliestVsync, now + workDuration));
-    if (mLastDispatchTime >= nextVsyncTime) { // already dispatched a callback for this vsync
-        return ScheduleResult::CannotSchedule;
+
+    bool const wouldSkipAVsyncTarget =
+            mArmedInfo && (nextVsyncTime > (mArmedInfo->mActualVsyncTime + mMinVsyncDistance));
+    if (wouldSkipAVsyncTarget) {
+        return ScheduleResult::Scheduled;
+    }
+
+    bool const alreadyDispatchedForVsync = mLastDispatchTime &&
+            ((*mLastDispatchTime + mMinVsyncDistance) >= nextVsyncTime &&
+             (*mLastDispatchTime - mMinVsyncDistance) <= nextVsyncTime);
+    if (alreadyDispatchedForVsync) {
+        nextVsyncTime =
+                tracker.nextAnticipatedVSyncTimeFrom(*mLastDispatchTime + mMinVsyncDistance);
     }
 
     auto const nextWakeupTime = nextVsyncTime - workDuration;
-    auto result = mArmedInfo ? ScheduleResult::ReScheduled : ScheduleResult::Scheduled;
     mWorkDuration = workDuration;
     mEarliestVsync = earliestVsync;
     mArmedInfo = {nextWakeupTime, nextVsyncTime};
-    return result;
+    return ScheduleResult::Scheduled;
 }
 
 void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {
@@ -101,8 +116,12 @@
 }
 
 VSyncDispatchTimerQueue::VSyncDispatchTimerQueue(std::unique_ptr<TimeKeeper> tk,
-                                                 VSyncTracker& tracker, nsecs_t timerSlack)
-      : mTimeKeeper(std::move(tk)), mTracker(tracker), mTimerSlack(timerSlack) {}
+                                                 VSyncTracker& tracker, nsecs_t timerSlack,
+                                                 nsecs_t minVsyncDistance)
+      : mTimeKeeper(std::move(tk)),
+        mTracker(tracker),
+        mTimerSlack(timerSlack),
+        mMinVsyncDistance(minVsyncDistance) {}
 
 VSyncDispatchTimerQueue::~VSyncDispatchTimerQueue() {
     std::lock_guard<decltype(mMutex)> lk(mMutex);
@@ -187,7 +206,8 @@
             mCallbacks
                     .emplace(++mCallbackToken,
                              std::make_shared<VSyncDispatchTimerQueueEntry>(callbackName,
-                                                                            callbackFn))
+                                                                            callbackFn,
+                                                                            mMinVsyncDistance))
                     .first->first};
 }
 
@@ -277,12 +297,16 @@
 }
 
 ScheduleResult VSyncCallbackRegistration::schedule(nsecs_t workDuration, nsecs_t earliestVsync) {
-    if (!mValidToken) return ScheduleResult::Error;
+    if (!mValidToken) {
+        return ScheduleResult::Error;
+    }
     return mDispatch.get().schedule(mToken, workDuration, earliestVsync);
 }
 
 CancelResult VSyncCallbackRegistration::cancel() {
-    if (!mValidToken) return CancelResult::Error;
+    if (!mValidToken) {
+        return CancelResult::Error;
+    }
     return mDispatch.get().cancel(mToken);
 }