Merge "SF: Fix many flakes in IdleTimerTest" (3e326092) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/Scheduler/IdleTimer.cpp

+13 −7

Original line number	Diff line number	Diff line
		@@ -54,18 +54,24 @@ void IdleTimer::loop() {
		if (mState == TimerState::IDLE) {
		mCondition.wait(mMutex);
		} else if (mState == TimerState::RESET) {
		auto triggerTime = std::chrono::steady_clock::now() + mInterval;
		mState = TimerState::WAITING;
		if (mCondition.wait_for(mMutex, mInterval) == std::cv_status::timeout) {
		while (mState == 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::WAITING &&
		(triggerTime - std::chrono::steady_clock::now()) <= zero) {
		if (mTimeoutCallback) {
		mTimeoutCallback();
		}
		}
		if (mState == TimerState::WAITING) {

		mState = TimerState::IDLE;
		}
		}
		}
		}
		} // namespace scheduler

		void IdleTimer::reset() {
		{

services/surfaceflinger/Scheduler/Scheduler.cpp

+4 −1

Original line number	Diff line number	Diff line
		@@ -81,7 +81,10 @@ Scheduler::Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function)
		}
		}

		Scheduler::~Scheduler() = default;
		Scheduler::~Scheduler() {
		// Ensure the IdleTimer thread is joined before we start destroying state.
		mIdleTimer.reset();
		}

		sp<Scheduler::ConnectionHandle> Scheduler::createConnection(
		const char* connectionName, int64_t phaseOffsetNs, ResyncCallback resyncCallback,

services/surfaceflinger/tests/unittests/IdleTimerTest.cpp

+59 −33

Original line number	Diff line number	Diff line
		@@ -34,91 +34,117 @@ protected:
		IdleTimerTest() = default;
		~IdleTimerTest() override = default;

		// This timeout should be used when a 3ms callback is expected.
		// While the tests typically request a callback after 3ms, the scheduler
		// does not always cooperate, at it can take significantly longer (observed
		// 30ms).
		static constexpr auto waitTimeForExpected3msCallback = 100ms;

		// This timeout should be used when an 3ms callback is not expected.
		// Note that there can be false-negatives if the callback happens later.
		static constexpr auto waitTimeForUnexpected3msCallback = 6ms;

		AsyncCallRecorder<void (*)()> mExpiredTimerCallback;

		std::unique_ptr<IdleTimer> mIdleTimer;

		void clearPendingCallbacks() {
		while (mExpiredTimerCallback.waitForCall(0us).has_value()) {
		}
		}
		};

		namespace {
		TEST_F(IdleTimerTest, createAndDestroyTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(30ms, [] {});
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, [] {});
		}

		TEST_F(IdleTimerTest, startStopTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(30ms, mExpiredTimerCallback.getInvocable());
		auto startTime = std::chrono::steady_clock::now();
		mIdleTimer->start();
		std::this_thread::sleep_for(std::chrono::milliseconds(10));
		// The timer expires after 30 ms, so the call to the callback should not happen.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall().has_value());
		// The idle timer fires after 30ms, so there should be no callback within
		// 25ms (waiting for a ballback for the full 30ms would be problematic).
		bool callbackCalled = mExpiredTimerCallback.waitForCall(25ms).has_value();
		// Under ideal conditions there should be no event. But occasionally
		// it is possible that the wait just prior takes more than 30ms, and
		// a callback is observed. We check the elapsed time since before the IdleTimer
		// thread was started as a sanity check to not have a flakey test.
		EXPECT_FALSE(callbackCalled && std::chrono::steady_clock::now() - startTime < 30ms);
		mIdleTimer->stop();
		}

		TEST_F(IdleTimerTest, resetTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(20ms, mExpiredTimerCallback.getInvocable());
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(30ms, mExpiredTimerCallback.getInvocable());
		mIdleTimer->start();
		std::this_thread::sleep_for(std::chrono::milliseconds(10));
		// The timer expires after 30 ms, so the call to the callback should not happen.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(1us).has_value());
		// Observe any event that happens in about 25ms. We don't care if one was
		// observed or not.
		mExpiredTimerCallback.waitForCall(25ms).has_value();
		mIdleTimer->reset();
		// The timer was reset, so the call to the callback should not happen.
		std::this_thread::sleep_for(std::chrono::milliseconds(15));
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(1us).has_value());
		// There may have been a race with the reset. Clear any callbacks we
		// received right afterwards.
		clearPendingCallbacks();
		// A single callback should be generated after 30ms
		EXPECT_TRUE(
		mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).has_value());
		// After one event, it should be idle, and not generate another.
		EXPECT_FALSE(
		mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback * 10).has_value());
		mIdleTimer->stop();
		// Final quick check that no more callback were observed.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value());
		}

		TEST_F(IdleTimerTest, startNotCalledTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, mExpiredTimerCallback.getInvocable());
		std::this_thread::sleep_for(6ms);
		// The start hasn't happened, so the callback does not happen.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(1us).has_value());
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
		mIdleTimer->stop();
		// Final quick check that no more callback were observed.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value());
		}

		TEST_F(IdleTimerTest, idleTimerIdlesTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, mExpiredTimerCallback.getInvocable());
		mIdleTimer->start();
		std::this_thread::sleep_for(6ms);
		// The timer expires after 3 ms, so the call to the callback happens.
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(1us).has_value());
		std::this_thread::sleep_for(6ms);
		// Timer can be idle.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(1us).has_value());
		// Timer can be reset.
		// A callback should be generated after 3ms
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value());
		// After one event, it should be idle, and not generate another.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
		// Once reset, it should generate another
		mIdleTimer->reset();
		std::this_thread::sleep_for(6ms);
		// Timer fires again.
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(1us).has_value());
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value());
		mIdleTimer->stop();
		// Final quick check that no more callback were observed.
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value());
		}

		TEST_F(IdleTimerTest, timeoutCallbackExecutionTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, mExpiredTimerCallback.getInvocable());

		mIdleTimer->start();
		std::this_thread::sleep_for(6ms);
		// The timer expires after 3 ms, so the call to the callback should happen.
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(1us).has_value());
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value());
		mIdleTimer->stop();
		}

		TEST_F(IdleTimerTest, noCallbacksAfterStopAndResetTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, mExpiredTimerCallback.getInvocable());
		mIdleTimer->start();
		std::this_thread::sleep_for(6ms);
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value());
		EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value());
		mIdleTimer->stop();
		clearPendingCallbacks();
		mIdleTimer->reset();
		std::this_thread::sleep_for(6ms);
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall().has_value());
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
		}

		TEST_F(IdleTimerTest, noCallbacksAfterStopTest) {
		mIdleTimer = std::make_unique<scheduler::IdleTimer>(3ms, mExpiredTimerCallback.getInvocable());
		mIdleTimer->start();
		std::this_thread::sleep_for(1ms);
		mIdleTimer->stop();
		std::this_thread::sleep_for(3ms);
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(1us).has_value());
		clearPendingCallbacks();
		mIdleTimer->reset();
		// No more idle events should be observed
		EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
		}

		} // namespace

services/surfaceflinger/tests/unittests/libsurfaceflinger_unittest_main.cpp

+11 −0

Original line number	Diff line number	Diff line
		@@ -16,6 +16,11 @@

		#include <gtest/gtest.h>

		#include <sched.h>

		#include <processgroup/sched_policy.h>
		#include <system/graphics.h>

		#include "libsurfaceflinger_unittest_main.h"

		// ------------------------------------------------------------------------
		@@ -39,6 +44,12 @@ bool g_noSlowTests = false;
		int main(int argc, char **argv) {
		::testing::InitGoogleTest(&argc, argv);

		// The SurfaceFlinger implementation assumes that threads resume
		// execution as quickly as possible once they become unblocked.
		// (These same calls are made in main_surfaceflinger.cpp)
		setpriority(PRIO_PROCESS, 0, HAL_PRIORITY_URGENT_DISPLAY);
		set_sched_policy(0, SP_FOREGROUND);

		for (int i = 1; i < argc; i++) {
		if (strcmp(argv[i], "--no-slow") == 0) {
		g_noSlowTests = true;