ConsumerBase: free buffers outside the lock

This change makes ConsumerBase::onBuffersReleased hold a reference to all its
gralloc buffers until after the mutex is unlocked.  This prevents slow
gralloc::free calls from causing lock contention with rendering threads.

Bug: 7675940
Change-Id: I0ec805d1b612afeeecfffec03f982371d27d93be

diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index 624d7e0..cfc0293 100644 (file)
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -109,21 +109,35 @@ void ConsumerBase::onFrameAvailable() {
 }
 
 void ConsumerBase::onBuffersReleased() {
-    Mutex::Autolock lock(mMutex);
+    sp<GraphicBuffer> bufRefs[BufferQueue::NUM_BUFFER_SLOTS];
+
+    { // Scope for the lock
+        Mutex::Autolock lock(mMutex);
+
+        CB_LOGV("onBuffersReleased");
 
-    CB_LOGV("onBuffersReleased");
+        if (mAbandoned) {
+            // Nothing to do if we're already abandoned.
+            return;
+        }
+
+        uint32_t mask = 0;
+        mBufferQueue->getReleasedBuffers(&mask);
+        for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
+            if (mask & (1 << i)) {
+                // Grab a local reference to the buffers so that they don't
+                // get freed while the lock is held.
+                bufRefs[i] = mSlots[i].mGraphicBuffer;
 
-    if (mAbandoned) {
-        // Nothing to do if we're already abandoned.
-        return;
+                freeBufferLocked(i);
+            }
+        }
     }
 
-    uint32_t mask = 0;
-    mBufferQueue->getReleasedBuffers(&mask);
+    // Clear the local buffer references.  This would happen automatically
+    // when the array gets dtor'd, but I'm doing it explicitly for clarity.
     for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
-        if (mask & (1 << i)) {
-            freeBufferLocked(i);
-        }
+        bufRefs[i].clear();
     }
 }