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

Commit 3403a3f7 authored by Ady Abraham's avatar Ady Abraham
Browse files

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
parent 0d3d1feb

services/surfaceflinger/TimeStats/TimeStats.cpp

+9 −8
Original line number Diff line number Diff line
@@ -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,9 +441,9 @@ 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,

            clampToNearestBucket(renderRate ? *renderRate : displayRefreshRate,

                                 RENDER_RATE_BUCKET_WIDTH);

    while (!timeRecords.empty()) {

        if (!recordReadyLocked(layerId, &timeRecords[0])) break;
@@ -799,9 +799,9 @@ 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,

            clampToNearestBucket(info.renderRate ? *info.renderRate : info.refreshRate,

                                 RENDER_RATE_BUCKET_WIDTH);

    const TimeStatsHelper::TimelineStatsKey timelineKey = {refreshRateBucket, renderRateBucket};
@@ -1021,6 +1021,7 @@ void TimeStats::disable() { 


void TimeStats::clearAll() {

    std::lock_guard<std::mutex> lock(mMutex);

    mTimeStats.stats.clear();

    clearGlobalLocked();

    clearLayersLocked();

}

services/surfaceflinger/tests/unittests/TimeStatsTest.cpp

+42 −18
Original line number Diff line number Diff line
@@ -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