libgui: Change BufferQueue to use free lists

BufferQueue used to choose free buffers by scanning through its array
of slots and picking one based on timestamp. This changes that
mechanism to use a pair of free lists: one with buffers attached and
one without. This makes it easier to choose either type of free slot
depending on the needs of the current operation.

Bug: 13175420
Change-Id: Ic8398e7511bd11a60a1c82e3ad2ee271c9822be1

1 files changed, 34 insertions, 30 deletions

diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 6452cddcf9..b54641e98f 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -161,8 +161,6 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
             }
         }
 
-        // Look for a free buffer to give to the client
-        *found = BufferQueueCore::INVALID_BUFFER_SLOT;
         int dequeuedCount = 0;
         int acquiredCount = 0;
         for (int s = 0; s < maxBufferCount; ++s) {
@@ -173,15 +171,6 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
                 case BufferSlot::ACQUIRED:
                     ++acquiredCount;
                     break;
-                case BufferSlot::FREE:
-                    // We return the oldest of the free buffers to avoid
-                    // stalling the producer if possible, since the consumer
-                    // may still have pending reads of in-flight buffers
-                    if (*found == BufferQueueCore::INVALID_BUFFER_SLOT ||
-                            mSlots[s].mFrameNumber < mSlots[*found].mFrameNumber) {
-                        *found = s;
-                    }
-                    break;
                 default:
                     break;
             }
@@ -214,6 +203,8 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
             }
         }
 
+        *found = BufferQueueCore::INVALID_BUFFER_SLOT;
+
         // If we disconnect and reconnect quickly, we can be in a state where
         // our slots are empty but we have many buffers in the queue. This can
         // cause us to run out of memory if we outrun the consumer. Wait here if
@@ -223,6 +214,16 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
         if (tooManyBuffers) {
             BQ_LOGV("%s: queue size is %zu, waiting", caller,
                     mCore->mQueue.size());
+        } else {
+            if (!mCore->mFreeBuffers.empty()) {
+                auto slot = mCore->mFreeBuffers.begin();
+                *found = *slot;
+                mCore->mFreeBuffers.erase(slot);
+            } else if (!mCore->mFreeSlots.empty()) {
+                auto slot = mCore->mFreeSlots.begin();
+                *found = *slot;
+                mCore->mFreeSlots.erase(slot);
+            }
         }
 
         // If no buffer is found, or if the queue has too many buffers
@@ -335,6 +336,8 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
         *outFence = mSlots[found].mFence;
         mSlots[found].mEglFence = EGL_NO_SYNC_KHR;
         mSlots[found].mFence = Fence::NO_FENCE;
+
+        mCore->validateConsistencyLocked();
     } // Autolock scope
 
     if (returnFlags & BUFFER_NEEDS_REALLOCATION) {
@@ -355,7 +358,6 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
                 return NO_INIT;
             }
 
-            mSlots[*outSlot].mFrameNumber = UINT32_MAX;
             mSlots[*outSlot].mGraphicBuffer = graphicBuffer;
         } // Autolock scope
     }
@@ -414,6 +416,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) {
 
     mCore->freeBufferLocked(slot);
     mCore->mDequeueCondition.broadcast();
+    mCore->validateConsistencyLocked();
 
     return NO_ERROR;
 }
@@ -438,27 +441,19 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
         return NO_INIT;
     }
 
-    // Find the oldest valid slot
-    int found = BufferQueueCore::INVALID_BUFFER_SLOT;
-    for (int s = 0; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
-        if (mSlots[s].mBufferState == BufferSlot::FREE &&
-                mSlots[s].mGraphicBuffer != NULL) {
-            if (found == BufferQueueCore::INVALID_BUFFER_SLOT ||
-                    mSlots[s].mFrameNumber < mSlots[found].mFrameNumber) {
-                found = s;
-            }
-        }
-    }
-
-    if (found == BufferQueueCore::INVALID_BUFFER_SLOT) {
+    if (mCore->mFreeBuffers.empty()) {
         return NO_MEMORY;
     }
 
+    int found = mCore->mFreeBuffers.front();
+    mCore->mFreeBuffers.remove(found);
+
     BQ_LOGV("detachNextBuffer detached slot %d", found);
 
     *outBuffer = mSlots[found].mGraphicBuffer;
     *outFence = mSlots[found].mFence;
     mCore->freeBufferLocked(found);
+    mCore->validateConsistencyLocked();
 
     return NO_ERROR;
 }
@@ -506,6 +501,8 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot,
     mSlots[*outSlot].mFence = Fence::NO_FENCE;
     mSlots[*outSlot].mRequestBufferCalled = true;
 
+    mCore->validateConsistencyLocked();
+
     return returnFlags;
 }
 
@@ -640,9 +637,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
                 // mark it as freed
                 if (mCore->stillTracking(front)) {
                     mSlots[front->mSlot].mBufferState = BufferSlot::FREE;
-                    // Reset the frame number of the freed buffer so that it is
-                    // the first in line to be dequeued again
-                    mSlots[front->mSlot].mFrameNumber = 0;
+                    mCore->mFreeBuffers.push_front(front->mSlot);
                 }
                 // Overwrite the droppable buffer with the incoming one
                 *front = item;
@@ -664,6 +659,8 @@ status_t BufferQueueProducer::queueBuffer(int slot,
 
         // Take a ticket for the callback functions
         callbackTicket = mNextCallbackTicket++;
+
+        mCore->validateConsistencyLocked();
     } // Autolock scope
 
     // Wait without lock held
@@ -724,10 +721,11 @@ void BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
         return;
     }
 
+    mCore->mFreeBuffers.push_front(slot);
     mSlots[slot].mBufferState = BufferSlot::FREE;
-    mSlots[slot].mFrameNumber = 0;
     mSlots[slot].mFence = fence;
     mCore->mDequeueCondition.broadcast();
+    mCore->validateConsistencyLocked();
 }
 
 int BufferQueueProducer::query(int what, int *outValue) {
@@ -1009,13 +1007,19 @@ void BufferQueueProducer::allocateBuffers(bool async, uint32_t width,
                 }
                 mCore->freeBufferLocked(slot); // Clean up the slot first
                 mSlots[slot].mGraphicBuffer = buffers[i];
-                mSlots[slot].mFrameNumber = 0;
                 mSlots[slot].mFence = Fence::NO_FENCE;
+
+                // freeBufferLocked puts this slot on the free slots list. Since
+                // we then attached a buffer, move the slot to free buffer list.
+                mCore->mFreeSlots.erase(slot);
+                mCore->mFreeBuffers.push_front(slot);
+
                 BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d", slot);
             }
 
             mCore->mIsAllocating = false;
             mCore->mIsAllocatingCondition.broadcast();
+            mCore->validateConsistencyLocked();
         } // Autolock scope
     }
 }