[DO NOT MERGE] GraphicBufferAllocator: stall alloc for async frees

This change makes GraphicBufferAllocator::alloc wait for pending async frees to
complete before attempting to allocate a gralloc buffer if there are more than
8 pending async frees.

Bug: 7696861
Change-Id: I1fae86e13edefcaa153b8ce9fd057f335716059e

diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp
index 72acd7d..fb43410 100644 (file)
--- a/libs/ui/GraphicBufferAllocator.cpp
+++ b/libs/ui/GraphicBufferAllocator.cpp
@@ -90,59 +90,36 @@ void GraphicBufferAllocator::dumpToSystemLog()
     ALOGD("%s", s.string());
 }
 
-status_t GraphicBufferAllocator::alloc(uint32_t w, uint32_t h, PixelFormat format,
-        int usage, buffer_handle_t* handle, int32_t* stride)
-{
-    ATRACE_CALL();
-    // make sure to not allocate a N x 0 or 0 x N buffer, since this is
-    // allowed from an API stand-point allocate a 1x1 buffer instead.
-    if (!w || !h)
-        w = h = 1;
+class BufferLiberatorThread : public Thread {
+public:
 
-    // we have a h/w allocator and h/w buffer is requested
-    status_t err; 
-    
-    err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride);
+    static void queueCaptiveBuffer(buffer_handle_t handle) {
+        size_t queueSize;
+        {
+            Mutex::Autolock lock(sMutex);
+            if (sThread == NULL) {
+                sThread = new BufferLiberatorThread;
+                sThread->run("BufferLiberator");
+            }
 
-    ALOGW_IF(err, "alloc(%u, %u, %d, %08x, ...) failed %d (%s)",
-            w, h, format, usage, err, strerror(-err));
-    
-    if (err == NO_ERROR) {
-        Mutex::Autolock _l(sLock);
-        KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList);
-        int bpp = bytesPerPixel(format);
-        if (bpp < 0) {
-            // probably a HAL custom format. in any case, we don't know
-            // what its pixel size is.
-            bpp = 0;
+            sThread->mQueue.push_back(handle);
+            sThread->mQueuedCondition.signal();
+            queueSize = sThread->mQueue.size();
         }
-        alloc_rec_t rec;
-        rec.w = w;
-        rec.h = h;
-        rec.s = *stride;
-        rec.format = format;
-        rec.usage = usage;
-        rec.size = h * stride[0] * bpp;
-        list.add(*handle, rec);
     }
 
-    return err;
-}
+    static void waitForLiberation() {
+        Mutex::Autolock lock(sMutex);
 
-class BufferLiberatorThread : public Thread {
-public:
+        waitForLiberationLocked();
+    }
 
-    static void queueCaptiveBuffer(buffer_handle_t handle) {
-        static sp<BufferLiberatorThread> thread(new BufferLiberatorThread);
-        static bool running = false;
-        if (!running) {
-            thread->run("BufferLiberator");
-            running = true;
-        }
-        {
-            Mutex::Autolock lock(thread->mMutex);
-            thread->mQueue.push_back(handle);
-            thread->mCondition.signal();
+    static void maybeWaitForLiberation() {
+        Mutex::Autolock lock(sMutex);
+        if (sThread != NULL) {
+            if (sThread->mQueue.size() > 8) {
+                waitForLiberationLocked();
+            }
         }
     }
 
@@ -152,13 +129,12 @@ private:
 
     virtual bool threadLoop() {
         buffer_handle_t handle;
-        {
-            Mutex::Autolock lock(mMutex);
+        { // Scope for mutex
+            Mutex::Autolock lock(sMutex);
             while (mQueue.isEmpty()) {
-                mCondition.wait(mMutex);
+                mQueuedCondition.wait(sMutex);
             }
             handle = mQueue[0];
-            mQueue.removeAt(0);
         }
 
         status_t err;
@@ -176,14 +152,94 @@ private:
             list.removeItem(handle);
         }
 
+        { // Scope for mutex
+            Mutex::Autolock lock(sMutex);
+            mQueue.removeAt(0);
+            mFreedCondition.broadcast();
+        }
+
         return true;
     }
 
+    static void waitForLiberationLocked() {
+        if (sThread == NULL) {
+            return;
+        }
+
+        const nsecs_t timeout = 500 * 1000 * 1000;
+        nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC);
+        nsecs_t timeToStop = now + timeout;
+        while (!sThread->mQueue.isEmpty() && now < timeToStop) {
+            sThread->mFreedCondition.waitRelative(sMutex, timeToStop - now);
+            now = systemTime(SYSTEM_TIME_MONOTONIC);
+        }
+
+        if (!sThread->mQueue.isEmpty()) {
+            ALOGW("waitForLiberationLocked timed out");
+        }
+    }
+
+    static Mutex sMutex;
+    static sp<BufferLiberatorThread> sThread;
     Vector<buffer_handle_t> mQueue;
-    Condition mCondition;
-    Mutex mMutex;
+    Condition mQueuedCondition;
+    Condition mFreedCondition;
 };
 
+Mutex BufferLiberatorThread::sMutex;
+sp<BufferLiberatorThread> BufferLiberatorThread::sThread;
+
+status_t GraphicBufferAllocator::alloc(uint32_t w, uint32_t h, PixelFormat format,
+        int usage, buffer_handle_t* handle, int32_t* stride)
+{
+    ATRACE_CALL();
+    // make sure to not allocate a N x 0 or 0 x N buffer, since this is
+    // allowed from an API stand-point allocate a 1x1 buffer instead.
+    if (!w || !h)
+        w = h = 1;
+
+    // we have a h/w allocator and h/w buffer is requested
+    status_t err;
+
+    // If too many async frees are queued up then wait for some of them to
+    // complete before attempting to allocate more memory.  This is exercised
+    // by the android.opengl.cts.GLSurfaceViewTest CTS test.
+    BufferLiberatorThread::maybeWaitForLiberation();
+
+    err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride);
+
+    if (err != NO_ERROR) {
+        ALOGW("WOW! gralloc alloc failed, waiting for pending frees!");
+        BufferLiberatorThread::waitForLiberation();
+        err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride);
+    }
+
+    ALOGW_IF(err, "alloc(%u, %u, %d, %08x, ...) failed %d (%s)",
+            w, h, format, usage, err, strerror(-err));
+
+    if (err == NO_ERROR) {
+        Mutex::Autolock _l(sLock);
+        KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList);
+        int bpp = bytesPerPixel(format);
+        if (bpp < 0) {
+            // probably a HAL custom format. in any case, we don't know
+            // what its pixel size is.
+            bpp = 0;
+        }
+        alloc_rec_t rec;
+        rec.w = w;
+        rec.h = h;
+        rec.s = *stride;
+        rec.format = format;
+        rec.usage = usage;
+        rec.size = h * stride[0] * bpp;
+        list.add(*handle, rec);
+    }
+
+    return err;
+}
+
+
 status_t GraphicBufferAllocator::free(buffer_handle_t handle)
 {
     BufferLiberatorThread::queueCaptiveBuffer(handle);