SF: clamp refresh rate to nearest bucket in TimeStats
59.4Hz should be clamped to the 60 bucket and not 30.
Fixes: 186476812
Test: SF unit tests
Change-Id: I28ce997a3248577b521b38d40835878911854b39
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 3d82afa..10d58a6 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -427,8 +427,8 @@
return true;
}
-static int32_t clampToSmallestBucket(Fps fps, size_t bucketWidth) {
- return (fps.getIntValue() / bucketWidth) * bucketWidth;
+static int32_t clampToNearestBucket(Fps fps, size_t bucketWidth) {
+ return std::round(fps.getValue() / bucketWidth) * bucketWidth;
}
void TimeStats::flushAvailableRecordsToStatsLocked(int32_t layerId, Fps displayRefreshRate,
@@ -441,10 +441,10 @@
TimeRecord& prevTimeRecord = layerRecord.prevTimeRecord;
std::deque<TimeRecord>& timeRecords = layerRecord.timeRecords;
const int32_t refreshRateBucket =
- clampToSmallestBucket(displayRefreshRate, REFRESH_RATE_BUCKET_WIDTH);
+ clampToNearestBucket(displayRefreshRate, REFRESH_RATE_BUCKET_WIDTH);
const int32_t renderRateBucket =
- clampToSmallestBucket(renderRate ? *renderRate : displayRefreshRate,
- RENDER_RATE_BUCKET_WIDTH);
+ clampToNearestBucket(renderRate ? *renderRate : displayRefreshRate,
+ RENDER_RATE_BUCKET_WIDTH);
while (!timeRecords.empty()) {
if (!recordReadyLocked(layerId, &timeRecords[0])) break;
ALOGV("[%d]-[%" PRIu64 "]-presentFenceTime[%" PRId64 "]", layerId,
@@ -799,10 +799,10 @@
static const std::string kDefaultLayerName = "none";
const int32_t refreshRateBucket =
- clampToSmallestBucket(info.refreshRate, REFRESH_RATE_BUCKET_WIDTH);
+ clampToNearestBucket(info.refreshRate, REFRESH_RATE_BUCKET_WIDTH);
const int32_t renderRateBucket =
- clampToSmallestBucket(info.renderRate ? *info.renderRate : info.refreshRate,
- RENDER_RATE_BUCKET_WIDTH);
+ clampToNearestBucket(info.renderRate ? *info.renderRate : info.refreshRate,
+ RENDER_RATE_BUCKET_WIDTH);
const TimeStatsHelper::TimelineStatsKey timelineKey = {refreshRateBucket, renderRateBucket};
if (!mTimeStats.stats.count(timelineKey)) {
@@ -1021,6 +1021,7 @@
void TimeStats::clearAll() {
std::lock_guard<std::mutex> lock(mMutex);
+ mTimeStats.stats.clear();
clearGlobalLocked();
clearLayersLocked();
}
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index 188ea75..ff53a7b 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -897,24 +897,24 @@
EXPECT_THAT(result, HasSubstr("compositionStrategyChanges = 0"));
EXPECT_THAT(result, HasSubstr("averageFrameDuration = 0.000 ms"));
EXPECT_THAT(result, HasSubstr("averageRenderEngineTiming = 0.000 ms"));
- std::string expectedResult = "totalTimelineFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "jankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "sfLongCpuJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "sfLongGpuJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "sfUnattributedJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "appUnattributedJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "sfSchedulingJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "sfPredictionErrorJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
- expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(0);
- EXPECT_THAT(result, HasSubstr(expectedResult));
+ std::string expectedResult = "totalTimelineFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "jankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "sfLongCpuJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "sfLongGpuJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "sfUnattributedJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "appUnattributedJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "sfSchedulingJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "sfPredictionErrorJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
+ expectedResult = "appBufferStuffingJankyFrames = ";
+ EXPECT_THAT(result, Not(HasSubstr(expectedResult)));
}
TEST_F(TimeStatsTest, canDumpWithMaxLayers) {
@@ -1348,6 +1348,30 @@
}
}
+TEST_F(TimeStatsTest, refreshRateIsClampedToNearestBucket) {
+ // this stat is not in the proto so verify by checking the string dump
+ const auto verifyRefreshRateBucket = [&](Fps fps, int32_t bucket) {
+ EXPECT_TRUE(inputCommand(InputCommand::CLEAR, FMT_STRING).empty());
+ EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
+
+ insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000);
+ mTimeStats->incrementJankyFrames(
+ {fps, std::nullopt, UID_0, genLayerName(LAYER_ID_0), JankType::None, 0, 0, 0});
+ const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING));
+ std::string expectedResult = "displayRefreshRate = " + std::to_string(bucket) + " fps";
+ EXPECT_THAT(result, HasSubstr(expectedResult)) << "failed for " << fps;
+ };
+
+ verifyRefreshRateBucket(Fps(91.f), 90);
+ verifyRefreshRateBucket(Fps(89.f), 90);
+
+ verifyRefreshRateBucket(Fps(61.f), 60);
+ verifyRefreshRateBucket(Fps(59.f), 60);
+
+ verifyRefreshRateBucket(Fps(31.f), 30);
+ verifyRefreshRateBucket(Fps(29.f), 30);
+}
+
} // namespace
} // namespace android
