Merge "SF: clamp refresh rate to nearest bucket in TimeStats" into sc-dev

2 files changed, 51 insertions, 26 deletions

diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 3d82afa43a..10d58a6087 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -427,8 +427,8 @@ bool TimeStats::recordReadyLocked(int32_t layerId, TimeRecord* timeRecord) {
     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 @@ void TimeStats::flushAvailableRecordsToStatsLocked(int32_t layerId, Fps displayR
     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 @@ void TimeStats::incrementJankyFrames(const JankyFramesInfo& info) {
     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::disable() {
 
 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 188ea758d4..ff53a7b09c 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -897,24 +897,24 @@ TEST_F(TimeStatsTest, canClearDumpOnlyTimeStats) {
     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, canSurviveMonkey) {
     }
 }
 
+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