Add rate limiting to fps reporter.
From a user perspective, the jittery frame rate is jarring, and from a
performance perspective submitting a binder call every frame is
overkill. To address those two concerns add rate-limiting from
SurfaceFlinger to limit the frame rate reporting.
As part of this patch, extract OneShotTimer's fake clock interface to a
SF-common location so that FpsReporter can share that interface for
testing.
Bug: 186265654
Test: Manually inspect fps overlay
Test: libsurfaceflinger_unittest
Change-Id: I45eb994d847abbb9fc8d5ff7d07ad51a1a790176
diff --git a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
index a2291b2..dec0ff5 100644
--- a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
@@ -28,6 +28,7 @@
#include "FpsReporter.h"
#include "Layer.h"
#include "TestableSurfaceFlinger.h"
+#include "fake/FakeClock.h"
#include "mock/DisplayHardware/MockComposer.h"
#include "mock/MockEventThread.h"
#include "mock/MockFrameTimeline.h"
@@ -91,7 +92,9 @@
sp<Layer> mUnrelated;
sp<TestableFpsListener> mFpsListener;
- sp<FpsReporter> mFpsReporter = new FpsReporter(mFrameTimeline, *(mFlinger.flinger()));
+ fake::FakeClock* mClock = new fake::FakeClock();
+ sp<FpsReporter> mFpsReporter =
+ new FpsReporter(mFrameTimeline, *(mFlinger.flinger()), std::unique_ptr<Clock>(mClock));
};
FpsReporterTest::FpsReporterTest() {
@@ -178,6 +181,7 @@
.WillOnce(Return(expectedFps));
mFpsReporter->addListener(mFpsListener, kTaskId);
+ mClock->advanceTime(600ms);
mFpsReporter->dispatchLayerFps();
EXPECT_EQ(expectedFps, mFpsListener->lastReportedFps);
mFpsReporter->removeListener(mFpsListener);
@@ -187,5 +191,34 @@
mFpsReporter->dispatchLayerFps();
}
+TEST_F(FpsReporterTest, rateLimits) {
+ const constexpr int32_t kTaskId = 12;
+ LayerMetadata targetMetadata;
+ targetMetadata.setInt32(METADATA_TASK_ID, kTaskId);
+ mTarget = createBufferStateLayer(targetMetadata);
+ mFlinger.mutableCurrentState().layersSortedByZ.add(mTarget);
+
+ float firstFps = 44.0;
+ float secondFps = 53.0;
+
+ EXPECT_CALL(mFrameTimeline, computeFps(UnorderedElementsAre(mTarget->getSequence())))
+ .WillOnce(Return(firstFps))
+ .WillOnce(Return(secondFps));
+
+ mFpsReporter->addListener(mFpsListener, kTaskId);
+ mClock->advanceTime(600ms);
+ mFpsReporter->dispatchLayerFps();
+ EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
+ mClock->advanceTime(200ms);
+ mFpsReporter->dispatchLayerFps();
+ EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
+ mClock->advanceTime(200ms);
+ mFpsReporter->dispatchLayerFps();
+ EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
+ mClock->advanceTime(200ms);
+ mFpsReporter->dispatchLayerFps();
+ EXPECT_EQ(secondFps, mFpsListener->lastReportedFps);
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
index a1f0588..6916764 100644
--- a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
@@ -23,23 +23,13 @@
#include "AsyncCallRecorder.h"
#include "Scheduler/OneShotTimer.h"
+#include "fake/FakeClock.h"
using namespace std::chrono_literals;
namespace android {
namespace scheduler {
-class FakeClock : public OneShotTimer::Clock {
-public:
- virtual ~FakeClock() = default;
- std::chrono::steady_clock::time_point now() const override { return mNow; }
-
- void advanceTime(std::chrono::nanoseconds delta) { mNow += delta; }
-
-private:
- std::chrono::steady_clock::time_point mNow;
-};
-
class OneShotTimerTest : public testing::Test {
protected:
OneShotTimerTest() = default;
@@ -58,17 +48,17 @@
namespace {
TEST_F(OneShotTimerTest, createAndDestroyTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>(
- "TestTimer", 3ms, [] {}, [] {}, std::unique_ptr<FakeClock>(clock));
+ "TestTimer", 3ms, [] {}, [] {}, std::unique_ptr<fake::FakeClock>(clock));
}
TEST_F(OneShotTimerTest, startStopTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value());
@@ -83,11 +73,11 @@
}
TEST_F(OneShotTimerTest, resetTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -105,11 +95,11 @@
}
TEST_F(OneShotTimerTest, resetBackToBackTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -139,11 +129,11 @@
}
TEST_F(OneShotTimerTest, startNotCalledTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
// The start hasn't happened, so the callback does not happen.
EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value());
EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value());
@@ -155,11 +145,11 @@
}
TEST_F(OneShotTimerTest, idleTimerIdlesTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
clock->advanceTime(2ms);
@@ -180,11 +170,11 @@
}
TEST_F(OneShotTimerTest, timeoutCallbackExecutionTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -197,11 +187,11 @@
}
TEST_F(OneShotTimerTest, noCallbacksAfterStopAndResetTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
clock->advanceTime(2ms);
@@ -215,11 +205,11 @@
}
TEST_F(OneShotTimerTest, noCallbacksAfterStopTest) {
- FakeClock* clock = new FakeClock();
+ fake::FakeClock* clock = new fake::FakeClock();
mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms,
mResetTimerCallback.getInvocable(),
mExpiredTimerCallback.getInvocable(),
- std::unique_ptr<FakeClock>(clock));
+ std::unique_ptr<fake::FakeClock>(clock));
mIdleTimer->start();
EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value());
diff --git a/services/surfaceflinger/tests/unittests/fake/FakeClock.h b/services/surfaceflinger/tests/unittests/fake/FakeClock.h
new file mode 100644
index 0000000..6d9c764
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/fake/FakeClock.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include "../../Clock.h"
+
+namespace android::fake {
+
+class FakeClock : public Clock {
+public:
+ virtual ~FakeClock() = default;
+ std::chrono::steady_clock::time_point now() const override { return mNow; }
+
+ void advanceTime(std::chrono::nanoseconds delta) { mNow += delta; }
+
+private:
+ std::chrono::steady_clock::time_point mNow;
+};
+
+} // namespace android::fake
\ No newline at end of file