commit bc9e5290313a24c7c77d0a2f18f82e29a84d05dc
author Pablo Gamito <pablogamito@google.com> Mon Aug 23 17:12:29 2021 +0200
committer Pablo Gamito <pablogamito@google.com> Mon Sep 06 17:23:39 2021 +0000
tree b60fef7df9e7f9c4eb4b4db549e9e8b07e97cb32
parent 88660d77daced91f8ecb80a2c295330d72210660

Pass surface control id to callback to accurately identify surface metrics belongs to

Avoid getting the wrong frame info when duplicate frame numbers are found in the ring buffer. Will ensure there isn't a mismatch in the metrics data reported.

Test: Existing tests
Bug: 197515602
Change-Id: Iff9ba01f575f94e5a9872ee48c0dd1e5067880c3

libs/hwui/renderthread/CanvasContext.cpp

diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 19a93f8..e6d31d6 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -203,9 +203,10 @@
     mSurfaceControl = surfaceControl;
     mSurfaceControlGenerationId++;
     mExpectSurfaceStats = surfaceControl != nullptr;
-    if (mSurfaceControl != nullptr) {
+    if (mExpectSurfaceStats) {
         funcs.acquireFunc(mSurfaceControl);
-        funcs.registerListenerFunc(surfaceControl, this, &onSurfaceStatsAvailable);
+        funcs.registerListenerFunc(surfaceControl, mSurfaceControlGenerationId, this,
+                                   &onSurfaceStatsAvailable);
     }
 }
 
@@ -613,11 +614,13 @@
     if (requireSwap) {
         if (mExpectSurfaceStats) {
             reportMetricsWithPresentTime();
-            std::lock_guard lock(mLast4FrameMetricsInfosMutex);
-            FrameMetricsInfo& next = mLast4FrameMetricsInfos.next();
-            next.frameInfo = mCurrentFrameInfo;
-            next.frameNumber = frameCompleteNr;
-            next.surfaceId = mSurfaceControlGenerationId;
+            {  // acquire lock
+                std::lock_guard lock(mLast4FrameMetricsInfosMutex);
+                FrameMetricsInfo& next = mLast4FrameMetricsInfos.next();
+                next.frameInfo = mCurrentFrameInfo;
+                next.frameNumber = frameCompleteNr;
+                next.surfaceId = mSurfaceControlGenerationId;
+            }  // release lock
         } else {
             mCurrentFrameInfo->markFrameCompleted();
             mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted)
@@ -716,19 +719,20 @@
     }
 }
 
-CanvasContext::FrameMetricsInfo CanvasContext::getFrameMetricsInfoFromLast4(uint64_t frameNumber) {
+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) {
-            return mLast4FrameMetricsInfos[i];
+        if (mLast4FrameMetricsInfos[i].frameNumber == frameNumber &&
+            mLast4FrameMetricsInfos[i].surfaceId == surfaceControlId) {
+            return mLast4FrameMetricsInfos[i].frameInfo;
         }
     }
 
-    return {};
+    return nullptr;
 }
 
 void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* control,
-            ASurfaceControlStats* stats) {
+                                            int32_t surfaceControlId, ASurfaceControlStats* stats) {
     auto* instance = static_cast<CanvasContext*>(context);
 
     const ASurfaceControlFunctions& functions =
@@ -737,17 +741,15 @@
     nsecs_t gpuCompleteTime = functions.getAcquireTimeFunc(stats);
     uint64_t frameNumber = functions.getFrameNumberFunc(stats);
 
-    FrameMetricsInfo frameMetricsInfo = instance->getFrameMetricsInfoFromLast4(frameNumber);
+    FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);
 
-    FrameInfo* frameInfo = frameMetricsInfo.frameInfo;
     if (frameInfo != nullptr) {
         frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,
                 frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));
         frameInfo->set(FrameInfoIndex::GpuCompleted) = gpuCompleteTime;
         std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
-        instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter,
-                                           frameMetricsInfo.frameNumber,
-                                           frameMetricsInfo.surfaceId);
+        instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter, frameNumber,
+                                           surfaceControlId);
     }
 }