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

Commit 75d4ffca authored by Ana Krulec's avatar Ana Krulec
Browse files

Optimizing OneShotTimer::reset() function

When running simpleperf, reset() function caused every once
in a while locks, which we suspect cause occasional janking.

- Moving the state to atomic, so there is no time spent
  acquiring the lock.
- When using condition_variable, atomic still needs to be
  changed under mutex, so moving to semaphores to guard
  and signal.

See b/170665374 for analysis report.

Bug: 170665374
Test: atest DisplayMicrobenchTests while running simple perf
Test: atest OneShotTimerTest
Change-Id: Ic450ea074bc07175c3fc681b69a8009be84d30a0
parent 29258758
Loading
Loading
Loading
Loading
+74 −45
Original line number Diff line number Diff line
@@ -20,6 +20,21 @@
#include <sstream>
#include <thread>

namespace {
using namespace std::chrono_literals;

constexpr int64_t kNsToSeconds = std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();

// The syscall interface uses a pair of integers for the timestamp. The first
// (tv_sec) is the whole count of seconds. The second (tv_nsec) is the
// nanosecond part of the count. This function takes care of translation.
void calculateTimeoutTime(std::chrono::nanoseconds timestamp, timespec* spec) {
    clock_gettime(CLOCK_MONOTONIC, spec);
    spec->tv_sec += static_cast<__kernel_time_t>(timestamp.count() / kNsToSeconds);
    spec->tv_nsec += timestamp.count() % kNsToSeconds;
}
} // namespace

namespace android {
namespace scheduler {

@@ -32,81 +47,95 @@ OneShotTimer::~OneShotTimer() {
}

void OneShotTimer::start() {
    {
        std::lock_guard<std::mutex> lock(mMutex);
        mState = TimerState::RESET;
    }
    sem_init(&mSemaphore, 0, 0);

    if (!mThread.joinable()) {
        // Only create thread if it has not been created.
        mThread = std::thread(&OneShotTimer::loop, this);
    }
}

void OneShotTimer::stop() {
    {
        std::lock_guard<std::mutex> lock(mMutex);
        mState = TimerState::STOPPED;
    }
    mCondition.notify_all();
    mStopTriggered = true;
    sem_post(&mSemaphore);

    if (mThread.joinable()) {
        mThread.join();
        sem_destroy(&mSemaphore);
    }
}

void OneShotTimer::loop() {
    TimerState state = TimerState::RESET;
    while (true) {
        bool triggerReset = false;
        bool triggerTimeout = false;
        {
            std::lock_guard<std::mutex> lock(mMutex);
            if (mState == TimerState::STOPPED) {

        state = checkForResetAndStop(state);
        if (state == TimerState::STOPPED) {
            break;
        }

            if (mState == TimerState::IDLE) {
                mCondition.wait(mMutex);
        if (state == TimerState::IDLE) {
            sem_wait(&mSemaphore);
            continue;
        }

            if (mState == TimerState::RESET) {
        if (state == TimerState::RESET) {
            triggerReset = true;
        }
        }

        if (triggerReset && mResetCallback) {
            mResetCallback();
        }

        { // lock the mutex again. someone might have called stop meanwhile
            std::lock_guard<std::mutex> lock(mMutex);
            if (mState == TimerState::STOPPED) {
        state = checkForResetAndStop(state);
        if (state == TimerState::STOPPED) {
            break;
        }

        auto triggerTime = std::chrono::steady_clock::now() + mInterval;
            mState = TimerState::WAITING;
            while (mState == TimerState::WAITING) {
        state = TimerState::WAITING;
        while (state == TimerState::WAITING) {
            constexpr auto zero = std::chrono::steady_clock::duration::zero();
                auto waitTime = triggerTime - std::chrono::steady_clock::now();
                if (waitTime > zero) mCondition.wait_for(mMutex, waitTime);
                if (mState == TimerState::RESET) {
            // Wait for mInterval time for semaphore signal.
            struct timespec ts;
            calculateTimeoutTime(std::chrono::nanoseconds(mInterval), &ts);
            sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &ts);

            state = checkForResetAndStop(state);
            if (state == TimerState::RESET) {
                triggerTime = std::chrono::steady_clock::now() + mInterval;
                    mState = TimerState::WAITING;
                } else if (mState == TimerState::WAITING &&
                state = TimerState::WAITING;
            } else if (state == TimerState::WAITING &&
                       (triggerTime - std::chrono::steady_clock::now()) <= zero) {
                triggerTimeout = true;
                    mState = TimerState::IDLE;
                }
                state = TimerState::IDLE;
            }
        }

        if (triggerTimeout && mTimeoutCallback) {
            mTimeoutCallback();
        }
    }
}

void OneShotTimer::reset() {
    {
        std::lock_guard<std::mutex> lock(mMutex);
        mState = TimerState::RESET;
OneShotTimer::TimerState OneShotTimer::checkForResetAndStop(TimerState state) {
    // Stop takes precedence of the reset.
    if (mStopTriggered.exchange(false)) {
        return TimerState::STOPPED;
    }
    mCondition.notify_all();
    // If the state was stopped, the thread was joined, and we cannot reset
    // the timer anymore.
    if (state != TimerState::STOPPED && mResetTriggered.exchange(false)) {
        return TimerState::RESET;
    }
    return state;
}

void OneShotTimer::reset() {
    mResetTriggered = true;
    sem_post(&mSemaphore);
}

std::string OneShotTimer::dump() const {
+13 −8
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

#pragma once

#include <semaphore.h>
#include <chrono>
#include <condition_variable>
#include <thread>
@@ -70,17 +71,15 @@ private:
    // Function that loops until the condition for stopping is met.
    void loop();

    // Checks whether mResetTriggered and mStopTriggered were set and updates
    // mState if so.
    TimerState checkForResetAndStop(TimerState state);

    // Thread waiting for timer to expire.
    std::thread mThread;

    // Condition used to notify mThread.
    std::condition_variable_any mCondition;

    // Lock used for synchronizing the waiting thread with the application thread.
    std::mutex mMutex;

    // Current timer state
    TimerState mState GUARDED_BY(mMutex) = TimerState::RESET;
    // Semaphore to keep mThread synchronized.
    sem_t mSemaphore;

    // Interval after which timer expires.
    const Interval mInterval;
@@ -90,6 +89,12 @@ private:

    // Callback that happens when timer expires.
    const TimeoutCallback mTimeoutCallback;

    // After removing lock guarding mState, the state can be now accessed at
    // any time. Keep a bool if the reset or stop were requested, and occasionally
    // check in the main loop if they were.
    std::atomic<bool> mResetTriggered = false;
    std::atomic<bool> mStopTriggered = false;
};

} // namespace scheduler