commit 6ba5125b9d5b88a4e9292f842f401827c0ad55f2
author Tang Lee <tanglee@google.com> Tue Aug 13 15:06:28 2024 +0800
committer Tang Lee <tanglee@google.com> Thu Aug 22 07:02:20 2024 +0000
tree f28ec4bdb780f20cfb1d3a4f1893aff14aa57bef
parent bb0348ec6a0a0c49f6c6db048d196a2bdf098161

Enlarge mLast4FrameMetricsInfos from 4 to 6 entries

As a result of tuning SurfaceFlinger durations
(debug.sf.late.sf.duration etc.), the ring buffer of the frame metrics
infos is sometimes too small, and already popped the frame's info when
its GPU complete is triggered and doing onSurfaceStatsAvailable(), so
finishFrame() is not called for this frame, which causes incorrect
jank judgement on the next frame.

Increase the ring buffer size from 4 to 6 can avoid the issue.

Bug: 354618175
Test: no skipped frame in onSurfaceStatsAvailable
Flag: EXEMPT bugfix
Change-Id: I2865365213e16cfebe63b949aae332cea32d8927

libs/hwui/renderthread/CanvasContext.cpp

diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 8bb11ba..dfda25d 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -761,8 +761,8 @@
         if (mExpectSurfaceStats) {
             reportMetricsWithPresentTime();
             {  // acquire lock
-                std::lock_guard lock(mLast4FrameMetricsInfosMutex);
-                FrameMetricsInfo& next = mLast4FrameMetricsInfos.next();
+                std::lock_guard lock(mLastFrameMetricsInfosMutex);
+                FrameMetricsInfo& next = mLastFrameMetricsInfos.next();
                 next.frameInfo = mCurrentFrameInfo;
                 next.frameNumber = frameCompleteNr;
                 next.surfaceId = mSurfaceControlGenerationId;
@@ -816,12 +816,12 @@
     int32_t surfaceControlId;
 
     {  // acquire lock
-        std::scoped_lock lock(mLast4FrameMetricsInfosMutex);
-        if (mLast4FrameMetricsInfos.size() != mLast4FrameMetricsInfos.capacity()) {
+        std::scoped_lock lock(mLastFrameMetricsInfosMutex);
+        if (mLastFrameMetricsInfos.size() != mLastFrameMetricsInfos.capacity()) {
             // Not enough frames yet
             return;
         }
-        auto frameMetricsInfo = mLast4FrameMetricsInfos.front();
+        auto frameMetricsInfo = mLastFrameMetricsInfos.front();
         forthBehind = frameMetricsInfo.frameInfo;
         frameNumber = frameMetricsInfo.frameNumber;
         surfaceControlId = frameMetricsInfo.surfaceId;
@@ -869,12 +869,12 @@
     }
 }
 
-FrameInfo* CanvasContext::getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId) {
-    std::scoped_lock lock(mLast4FrameMetricsInfosMutex);
-    for (size_t i = 0; i < mLast4FrameMetricsInfos.size(); i++) {
-        if (mLast4FrameMetricsInfos[i].frameNumber == frameNumber &&
-            mLast4FrameMetricsInfos[i].surfaceId == surfaceControlId) {
-            return mLast4FrameMetricsInfos[i].frameInfo;
+FrameInfo* CanvasContext::getFrameInfoFromLastFew(uint64_t frameNumber, uint32_t surfaceControlId) {
+    std::scoped_lock lock(mLastFrameMetricsInfosMutex);
+    for (size_t i = 0; i < mLastFrameMetricsInfos.size(); i++) {
+        if (mLastFrameMetricsInfos[i].frameNumber == frameNumber &&
+            mLastFrameMetricsInfos[i].surfaceId == surfaceControlId) {
+            return mLastFrameMetricsInfos[i].frameInfo;
         }
     }
 
@@ -894,7 +894,7 @@
     }
     uint64_t frameNumber = functions.getFrameNumberFunc(stats);
 
-    FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);
+    FrameInfo* frameInfo = instance->getFrameInfoFromLastFew(frameNumber, surfaceControlId);
 
     if (frameInfo != nullptr) {
         std::scoped_lock lock(instance->mFrameInfoMutex);

libs/hwui/renderthread/CanvasContext.h

diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index e2e3fa3..cb37538 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -260,7 +260,7 @@
     void finishFrame(FrameInfo* frameInfo);
 
     /**
-     * Invoke 'reportFrameMetrics' on the last frame stored in 'mLast4FrameInfos'.
+     * Invoke 'reportFrameMetrics' on the last frame stored in 'mLastFrameInfos'.
      * Populate the 'presentTime' field before calling.
      */
     void reportMetricsWithPresentTime();
@@ -271,7 +271,7 @@
         int32_t surfaceId;
     };
 
-    FrameInfo* getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId);
+    FrameInfo* getFrameInfoFromLastFew(uint64_t frameNumber, uint32_t surfaceControlId);
 
     Frame getFrame();
 
@@ -336,9 +336,9 @@
 
     // List of data of frames that are awaiting GPU completion reporting. Used to compute frame
     // metrics and determine whether or not to report the metrics.
-    RingBuffer<FrameMetricsInfo, 4> mLast4FrameMetricsInfos
-            GUARDED_BY(mLast4FrameMetricsInfosMutex);
-    std::mutex mLast4FrameMetricsInfosMutex;
+    RingBuffer<FrameMetricsInfo, 6> mLastFrameMetricsInfos
+            GUARDED_BY(mLastFrameMetricsInfosMutex);
+    std::mutex mLastFrameMetricsInfosMutex;
 
     std::string mName;
     JankTracker mJankTracker;