Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit da8af4c6 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: VSyncDispatchTimerQueueEntry::update should not skip a frame 

Change-Id: Ic3e55c073a941a36a5bae9f502657e6383caef72
Test: SysUI Jank Regression
Bug: 324149129
Bug: 273702768
parent bf55489e

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

+31 −24
Original line number Diff line number Diff line
@@ -43,16 +43,6 @@ nsecs_t getExpectedCallbackTime(nsecs_t nextVsyncTime, 
    return nextVsyncTime - timing.readyDuration - timing.workDuration;

}



nsecs_t getExpectedCallbackTime(VSyncTracker& tracker, nsecs_t now,

                                const VSyncDispatch::ScheduleTiming& timing) {

    const auto nextVsyncTime =

            tracker.nextAnticipatedVSyncTimeFrom(std::max(timing.lastVsync,

                                                          now + timing.workDuration +

                                                                  timing.readyDuration),

                                                 timing.lastVsync);

    return getExpectedCallbackTime(nextVsyncTime, timing);

}



} // namespace



VSyncDispatch::~VSyncDispatch() = default;
@@ -128,8 +118,11 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim 
    return nextWakeupTime;

}



void VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(VSyncDispatch::ScheduleTiming timing) {

nsecs_t VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(

        VSyncTracker& tracker, nsecs_t now, VSyncDispatch::ScheduleTiming timing) {

    mWorkloadUpdateInfo = timing;

    const auto armedInfo = update(tracker, now, timing, mArmedInfo);

    return armedInfo.mActualWakeupTime;

}



bool VSyncDispatchTimerQueueEntry::hasPendingWorkloadUpdate() const {
@@ -157,6 +150,31 @@ nsecs_t VSyncDispatchTimerQueueEntry::adjustVsyncIfNeeded(VSyncTracker& tracker, 
    return nextVsyncTime;

}



auto VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now,

                                          VSyncDispatch::ScheduleTiming timing,

                                          std::optional<ArmingInfo> armedInfo) const -> ArmingInfo {

    const auto earliestReadyBy = now + timing.workDuration + timing.readyDuration;

    const auto earliestVsync = std::max(earliestReadyBy, timing.lastVsync);



    const auto nextVsyncTime =

            adjustVsyncIfNeeded(tracker, /*nextVsyncTime*/

                                tracker.nextAnticipatedVSyncTimeFrom(earliestVsync,

                                                                     timing.lastVsync));

    const auto nextReadyTime = nextVsyncTime - timing.readyDuration;

    const auto nextWakeupTime = nextReadyTime - timing.workDuration;



    bool const wouldSkipAVsyncTarget =

            armedInfo && (nextVsyncTime > (armedInfo->mActualVsyncTime + mMinVsyncDistance));

    bool const wouldSkipAWakeup =

            armedInfo && (nextWakeupTime > (armedInfo->mActualWakeupTime + mMinVsyncDistance));

    if (FlagManager::getInstance().dont_skip_on_early_ro() &&

        (wouldSkipAVsyncTarget || wouldSkipAWakeup)) {

        return *armedInfo;

    }



    return ArmingInfo{nextWakeupTime, nextVsyncTime, nextReadyTime};

}



void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {

    if (!mArmedInfo && !mWorkloadUpdateInfo) {

        return;
@@ -167,17 +185,7 @@ void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) { 
        mWorkloadUpdateInfo.reset();

    }



    const auto earliestReadyBy = now + mScheduleTiming.workDuration + mScheduleTiming.readyDuration;

    const auto earliestVsync = std::max(earliestReadyBy, mScheduleTiming.lastVsync);



    const auto nextVsyncTime =

            adjustVsyncIfNeeded(tracker, /*nextVsyncTime*/

                                tracker.nextAnticipatedVSyncTimeFrom(earliestVsync,

                                                                     mScheduleTiming.lastVsync));

    const auto nextReadyTime = nextVsyncTime - mScheduleTiming.readyDuration;

    const auto nextWakeupTime = nextReadyTime - mScheduleTiming.workDuration;



    mArmedInfo = {nextWakeupTime, nextVsyncTime, nextReadyTime};

    mArmedInfo = update(tracker, now, mScheduleTiming, mArmedInfo);

}



void VSyncDispatchTimerQueueEntry::disarm() {
@@ -394,8 +402,7 @@ ScheduleResult VSyncDispatchTimerQueue::scheduleLocked(CallbackToken token, 
     * timer recalculation to avoid cancelling a callback that is about to fire. */

    auto const rearmImminent = now > mIntendedWakeupTime;

    if (CC_UNLIKELY(rearmImminent)) {

        callback->addPendingWorkloadUpdate(scheduleTiming);

        return getExpectedCallbackTime(*mTracker, now, scheduleTiming);

        return callback->addPendingWorkloadUpdate(*mTracker, now, scheduleTiming);

    }



    const ScheduleResult result = callback->schedule(scheduleTiming, *mTracker, now);

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h

+9 −6
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ public: 


    // Adds a pending upload of the earliestVSync and workDuration that will be applied on the next

    // call to update()

    void addPendingWorkloadUpdate(VSyncDispatch::ScheduleTiming);

    nsecs_t addPendingWorkloadUpdate(VSyncTracker&, nsecs_t now, VSyncDispatch::ScheduleTiming);



    // Checks if there is a pending update to the workload, returning true if so.

    bool hasPendingWorkloadUpdate() const;
@@ -83,7 +83,15 @@ public: 
    void dump(std::string& result) const;



private:

    struct ArmingInfo {

        nsecs_t mActualWakeupTime;

        nsecs_t mActualVsyncTime;

        nsecs_t mActualReadyTime;

    };



    nsecs_t adjustVsyncIfNeeded(VSyncTracker& tracker, nsecs_t nextVsyncTime) const;

    ArmingInfo update(VSyncTracker&, nsecs_t now, VSyncDispatch::ScheduleTiming,

                      std::optional<ArmingInfo>) const;



    const std::string mName;

    const VSyncDispatch::Callback mCallback;
@@ -91,11 +99,6 @@ private: 
    VSyncDispatch::ScheduleTiming mScheduleTiming;

    const nsecs_t mMinVsyncDistance;



    struct ArmingInfo {

        nsecs_t mActualWakeupTime;

        nsecs_t mActualVsyncTime;

        nsecs_t mActualReadyTime;

    };

    std::optional<ArmingInfo> mArmedInfo;

    std::optional<nsecs_t> mLastDispatchTime;

services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp

+2 −1
Original line number Diff line number Diff line
@@ -175,7 +175,8 @@ void SchedulerFuzzer::fuzzVSyncDispatchTimerQueue() { 
    auto const wakeup = entry.wakeupTime();

    auto const ready = entry.readyTime();

    entry.callback(entry.executing(), *wakeup, *ready);

    entry.addPendingWorkloadUpdate({.workDuration = mFdp.ConsumeIntegral<nsecs_t>(),

    entry.addPendingWorkloadUpdate(*stubTracker, 0,

                                   {.workDuration = mFdp.ConsumeIntegral<nsecs_t>(),

                                    .readyDuration = mFdp.ConsumeIntegral<nsecs_t>(),

                                    .lastVsync = mFdp.ConsumeIntegral<nsecs_t>()});

    dump<scheduler::VSyncDispatchTimerQueueEntry>(&entry, &mFdp);

services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp

+40 −4
Original line number Diff line number Diff line
@@ -917,6 +917,8 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent 
// If the same callback tries to reschedule itself after it's too late, timer opts to apply the

// update later, as opposed to blocking the calling thread.

TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminentSameCallback) {

    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);



    Sequence seq;

    EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);

    EXPECT_CALL(mMockClock, alarmAt(_, 1630)).InSequence(seq);
@@ -938,6 +940,37 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent 
    EXPECT_THAT(cb.mCalls.size(), Eq(1));

}



// b/154303580.

// If the same callback tries to reschedule itself after it's too late, timer opts to apply the

// update later, as opposed to blocking the calling thread.

TEST_F(VSyncDispatchTimerQueueTest, doesntSkipSchedulingIfTimerReschedulingIsImminentSameCallback) {

    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);



    Sequence seq;

    EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);

    EXPECT_CALL(mMockClock, alarmAt(_, 1630)).InSequence(seq);

    CountingCallback cb(mDispatch);



    auto result =

            mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000});

    EXPECT_TRUE(result.has_value());

    EXPECT_EQ(600, *result);



    mMockClock.setLag(100);

    mMockClock.advanceBy(620);



    result = mDispatch->schedule(cb, {.workDuration = 370, .readyDuration = 0, .lastVsync = 2000});

    EXPECT_TRUE(result.has_value());

    EXPECT_EQ(600, *result);

    mMockClock.advanceBy(80);



    ASSERT_EQ(1, cb.mCalls.size());

    EXPECT_EQ(1000, cb.mCalls[0]);



    ASSERT_EQ(1, cb.mWakeupTime.size());

    EXPECT_EQ(600, cb.mWakeupTime[0]);

}



// b/154303580.

TEST_F(VSyncDispatchTimerQueueTest, skipsRearmingWhenNotNextScheduled) {

    Sequence seq;
@@ -1344,14 +1377,17 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, reportsScheduledIfStillTime) { 
                        .has_value());

}



TEST_F(VSyncDispatchTimerQueueEntryTest, storesPendingUpdatesUntilUpdate) {

TEST_F(VSyncDispatchTimerQueueEntryTest, storesPendingUpdatesUntilUpdateAndDontSkip) {

    static constexpr auto effectualOffset = 200;

    VSyncDispatchTimerQueueEntry entry(

            "test", [](auto, auto, auto) {}, mVsyncMoveThreshold);

    EXPECT_FALSE(entry.hasPendingWorkloadUpdate());

    entry.addPendingWorkloadUpdate({.workDuration = 100, .readyDuration = 0, .lastVsync = 400});

    entry.addPendingWorkloadUpdate(

            {.workDuration = effectualOffset, .readyDuration = 0, .lastVsync = 400});

    entry.addPendingWorkloadUpdate(*mStubTracker.get(), 0,

                                   {.workDuration = 100, .readyDuration = 0, .lastVsync = 400});

    entry.addPendingWorkloadUpdate(*mStubTracker.get(), 0,

                                   {.workDuration = effectualOffset,

                                    .readyDuration = 0,

                                    .lastVsync = 400});

    EXPECT_TRUE(entry.hasPendingWorkloadUpdate());

    entry.update(*mStubTracker.get(), 0);

    EXPECT_FALSE(entry.hasPendingWorkloadUpdate());