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

Commit 5ff4bb8f authored by Ady Abraham's avatar Ady Abraham
Browse files

SurfaceFlinger: use TFD_TIMER_ABSTIME for VSP timer 

Using the semantics of alarmIn in VSP timer may lead to lag
if the thread is getting preempted. For example:
Time 0: t1 calls to alarmIn(5)
 -- t1 gets preempted for 2ms --
Time 2: t1 calls to timerfd_settime(5)
Time 7: timer wakes up t1, results in 2ms lag

Switching to alarmAt semantics and using TFD_TIMER_ABSTIME to
schedule the timer solves this problem:
Time 0: t1 calls to alarmAt(5)
 -- t1 gets preempted for 2ms --
Time 2: t1 calls to timerfd_settime(5)
Time 5: timer wakes up t1

Bug: 159884130
Test: bouncy ball with simulated scheduling delays

Change-Id: I3d727530c2dd47c1a8d1d6a66114d654d7261d87
Merged-In: I3d727530c2dd47c1a8d1d6a66114d654d7261d87
parent acfe554b

services/surfaceflinger/Scheduler/TimeKeeper.h

+2 −2
Original line number Diff line number Diff line
@@ -43,10 +43,10 @@ public: 
    virtual ~TimeKeeper();



    /*

     * Arms callback to fired in time nanoseconds.

     * Arms callback to fired when time is current based on CLOCK_MONOTONIC

     * There is only one timer, and subsequent calls will reset the callback function and the time.

     */

    virtual void alarmIn(std::function<void()> const& callback, nsecs_t time) = 0;

    virtual void alarmAt(std::function<void()> const& callback, nsecs_t time) = 0;



    /*

     * Cancels an existing pending callback

services/surfaceflinger/Scheduler/Timer.cpp

+4 −4
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ nsecs_t Timer::now() const { 
    return systemTime(SYSTEM_TIME_MONOTONIC);

}



void Timer::alarmIn(std::function<void()> const& cb, nsecs_t fireIn) {

void Timer::alarmAt(std::function<void()> const& cb, nsecs_t time) {

    std::lock_guard<decltype(mMutex)> lk(mMutex);

    using namespace std::literals;

    static constexpr int ns_per_s =
@@ -99,11 +99,11 @@ void Timer::alarmIn(std::function<void()> const& cb, nsecs_t fireIn) { 
    struct itimerspec old_timer;

    struct itimerspec new_timer {

        .it_interval = {.tv_sec = 0, .tv_nsec = 0},

        .it_value = {.tv_sec = static_cast<long>(fireIn / ns_per_s),

                     .tv_nsec = static_cast<long>(fireIn % ns_per_s)},

        .it_value = {.tv_sec = static_cast<long>(time / ns_per_s),

                     .tv_nsec = static_cast<long>(time % ns_per_s)},

    };



    if (timerfd_settime(mTimerFd, 0, &new_timer, &old_timer)) {

    if (timerfd_settime(mTimerFd, TFD_TIMER_ABSTIME, &new_timer, &old_timer)) {

        ALOGW("Failed to set timerfd %s (%i)", strerror(errno), errno);

    }

}

services/surfaceflinger/Scheduler/Timer.h

+2 −2
Original line number Diff line number Diff line
@@ -30,9 +30,9 @@ public: 
    ~Timer();

    nsecs_t now() const final;



    // NB: alarmIn and alarmCancel are threadsafe; with the last-returning function being effectual

    // NB: alarmAt and alarmCancel are threadsafe; with the last-returning function being effectual

    //     Most users will want to serialize thes calls so as to be aware of the timer state.

    void alarmIn(std::function<void()> const& cb, nsecs_t fireIn) final;

    void alarmAt(std::function<void()> const& cb, nsecs_t time) final;

    void alarmCancel() final;

    void dump(std::string& result) const final;

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

+3 −3
Original line number Diff line number Diff line
@@ -180,10 +180,10 @@ void VSyncDispatchTimerQueue::cancelTimer() { 
    mTimeKeeper->alarmCancel();

}



void VSyncDispatchTimerQueue::setTimer(nsecs_t targetTime, nsecs_t now) {

void VSyncDispatchTimerQueue::setTimer(nsecs_t targetTime, nsecs_t /*now*/) {

    mIntendedWakeupTime = targetTime;

    mTimeKeeper->alarmIn(std::bind(&VSyncDispatchTimerQueue::timerCallback, this),

                         targetTime - now);

    mTimeKeeper->alarmAt(std::bind(&VSyncDispatchTimerQueue::timerCallback, this),

                         mIntendedWakeupTime);

    mLastTimerSchedule = mTimeKeeper->now();

}

services/surfaceflinger/tests/unittests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ cc_test { 
        "RegionSamplingTest.cpp",

        "TimeStatsTest.cpp",

        "FrameTracerTest.cpp",

        "TimerTest.cpp",

        "TransactionApplicationTest.cpp",

        "StrongTypingTest.cpp",

        "VSyncDispatchTimerQueueTest.cpp",