From b21a4e3b5f7f07ed160ca6e1809313e2a8e2a6a4 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Thu, 6 Dec 2012 17:51:53 -0800 Subject: 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 --- libs/gui/ConsumerBase.cpp | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 624d7e0..cfc0293 100644 --- 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 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(); } } -- cgit v1.1