am f96a7581: am fb6d43ff: Merge "libgui: Change BufferQueue to use free lists"
* commit 'f96a758139be0d5c298abad8e27083b0f0849818':
libgui: Change BufferQueue to use free lists
diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h
index 40026bd..a6065a9 100644
--- a/include/gui/BufferQueueCore.h
+++ b/include/gui/BufferQueueCore.h
@@ -30,6 +30,9 @@
#include <utils/Trace.h>
#include <utils/Vector.h>
+#include <list>
+#include <set>
+
#define BQ_LOGV(x, ...) ALOGV("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
#define BQ_LOGD(x, ...) ALOGD("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
#define BQ_LOGI(x, ...) ALOGI("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
@@ -123,6 +126,10 @@
// waitWhileAllocatingLocked blocks until mIsAllocating is false.
void waitWhileAllocatingLocked() const;
+ // validateConsistencyLocked ensures that the free lists are in sync with
+ // the information stored in mSlots
+ void validateConsistencyLocked() const;
+
// mAllocator is the connection to SurfaceFlinger that is used to allocate
// new GraphicBuffer objects.
sp<IGraphicBufferAlloc> mAllocator;
@@ -177,6 +184,14 @@
// mQueue is a FIFO of queued buffers used in synchronous mode.
Fifo mQueue;
+ // mFreeSlots contains all of the slots which are FREE and do not currently
+ // have a buffer attached
+ std::set<int> mFreeSlots;
+
+ // mFreeBuffers contains all of the slots which are FREE and currently have
+ // a buffer attached
+ std::list<int> mFreeBuffers;
+
// mOverrideMaxBufferCount is the limit on the number of buffers that will
// be allocated at one time. This value is set by the producer by calling
// setBufferCount. The default is 0, which means that the producer doesn't
diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 526c3b7..c7d5e00 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -120,6 +120,7 @@
if (mCore->stillTracking(front)) {
// Front buffer is still in mSlots, so mark the slot as free
mSlots[front->mSlot].mBufferState = BufferSlot::FREE;
+ mCore->mFreeBuffers.push_back(front->mSlot);
}
mCore->mQueue.erase(front);
front = mCore->mQueue.begin();
@@ -173,6 +174,8 @@
ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size());
+ mCore->validateConsistencyLocked();
+
return NO_ERROR;
}
@@ -199,6 +202,7 @@
mCore->freeBufferLocked(slot);
mCore->mDequeueCondition.broadcast();
+ mCore->validateConsistencyLocked();
return NO_ERROR;
}
@@ -217,18 +221,11 @@
Mutex::Autolock lock(mCore->mMutex);
- // Make sure we don't have too many acquired buffers and find a free slot
- // to put the buffer into (the oldest if there are multiple).
+ // Make sure we don't have too many acquired buffers
int numAcquiredBuffers = 0;
- int found = BufferQueueCore::INVALID_BUFFER_SLOT;
for (int s = 0; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
if (mSlots[s].mBufferState == BufferSlot::ACQUIRED) {
++numAcquiredBuffers;
- } else if (mSlots[s].mBufferState == BufferSlot::FREE) {
- if (found == BufferQueueCore::INVALID_BUFFER_SLOT ||
- mSlots[s].mFrameNumber < mSlots[found].mFrameNumber) {
- found = s;
- }
}
}
@@ -238,6 +235,17 @@
mCore->mMaxAcquiredBufferCount);
return INVALID_OPERATION;
}
+
+ // Find a free slot to put the buffer into
+ int found = BufferQueueCore::INVALID_BUFFER_SLOT;
+ if (!mCore->mFreeSlots.empty()) {
+ auto slot = mCore->mFreeSlots.begin();
+ found = *slot;
+ mCore->mFreeSlots.erase(slot);
+ } else if (!mCore->mFreeBuffers.empty()) {
+ found = mCore->mFreeBuffers.front();
+ mCore->mFreeBuffers.remove(found);
+ }
if (found == BufferQueueCore::INVALID_BUFFER_SLOT) {
BQ_LOGE("attachBuffer(P): could not find free buffer slot");
return NO_MEMORY;
@@ -271,6 +279,8 @@
// for attached buffers.
mSlots[*outSlot].mAcquireCalled = false;
+ mCore->validateConsistencyLocked();
+
return NO_ERROR;
}
@@ -311,6 +321,7 @@
mSlots[slot].mEglFence = eglFence;
mSlots[slot].mFence = releaseFence;
mSlots[slot].mBufferState = BufferSlot::FREE;
+ mCore->mFreeBuffers.push_back(slot);
listener = mCore->mConnectedProducerListener;
BQ_LOGV("releaseBuffer: releasing slot %d", slot);
} else if (mSlots[slot].mNeedsCleanupOnRelease) {
@@ -325,6 +336,7 @@
}
mCore->mDequeueCondition.broadcast();
+ mCore->validateConsistencyLocked();
} // Autolock scope
// Call back without lock held
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index edebc45..29415c9 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -53,6 +53,8 @@
mConnectedProducerListener(),
mSlots(),
mQueue(),
+ mFreeSlots(),
+ mFreeBuffers(),
mOverrideMaxBufferCount(0),
mDequeueCondition(),
mUseAsyncBuffer(true),
@@ -76,6 +78,9 @@
BQ_LOGE("createGraphicBufferAlloc failed");
}
}
+ for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) {
+ mFreeSlots.insert(slot);
+ }
}
BufferQueueCore::~BufferQueueCore() {}
@@ -190,12 +195,20 @@
void BufferQueueCore::freeBufferLocked(int slot) {
BQ_LOGV("freeBufferLocked: slot %d", slot);
+ bool hadBuffer = mSlots[slot].mGraphicBuffer != NULL;
mSlots[slot].mGraphicBuffer.clear();
if (mSlots[slot].mBufferState == BufferSlot::ACQUIRED) {
mSlots[slot].mNeedsCleanupOnRelease = true;
}
+ if (mSlots[slot].mBufferState != BufferSlot::FREE) {
+ mFreeSlots.insert(slot);
+ } else if (hadBuffer) {
+ // If the slot was FREE, but we had a buffer, we need to move this slot
+ // from the free buffers list to the the free slots list
+ mFreeBuffers.remove(slot);
+ mFreeSlots.insert(slot);
+ }
mSlots[slot].mBufferState = BufferSlot::FREE;
- mSlots[slot].mFrameNumber = UINT32_MAX;
mSlots[slot].mAcquireCalled = false;
// Destroy fence as BufferQueue now takes ownership
@@ -204,6 +217,7 @@
mSlots[slot].mEglFence = EGL_NO_SYNC_KHR;
}
mSlots[slot].mFence = Fence::NO_FENCE;
+ validateConsistencyLocked();
}
void BufferQueueCore::freeAllBuffersLocked() {
@@ -236,4 +250,48 @@
}
}
+void BufferQueueCore::validateConsistencyLocked() const {
+ static const useconds_t PAUSE_TIME = 0;
+ for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) {
+ bool isInFreeSlots = mFreeSlots.count(slot) != 0;
+ bool isInFreeBuffers =
+ std::find(mFreeBuffers.cbegin(), mFreeBuffers.cend(), slot) !=
+ mFreeBuffers.cend();
+ if (mSlots[slot].mBufferState == BufferSlot::FREE) {
+ if (mSlots[slot].mGraphicBuffer == NULL) {
+ if (!isInFreeSlots) {
+ BQ_LOGE("Slot %d is FREE but is not in mFreeSlots", slot);
+ usleep(PAUSE_TIME);
+ }
+ if (isInFreeBuffers) {
+ BQ_LOGE("Slot %d is in mFreeSlots "
+ "but is also in mFreeBuffers", slot);
+ usleep(PAUSE_TIME);
+ }
+ } else {
+ if (!isInFreeBuffers) {
+ BQ_LOGE("Slot %d is FREE but is not in mFreeBuffers", slot);
+ usleep(PAUSE_TIME);
+ }
+ if (isInFreeSlots) {
+ BQ_LOGE("Slot %d is in mFreeBuffers "
+ "but is also in mFreeSlots", slot);
+ usleep(PAUSE_TIME);
+ }
+ }
+ } else {
+ if (isInFreeSlots) {
+ BQ_LOGE("Slot %d is in mFreeSlots but is not FREE (%d)",
+ slot, mSlots[slot].mBufferState);
+ usleep(PAUSE_TIME);
+ }
+ if (isInFreeBuffers) {
+ BQ_LOGE("Slot %d is in mFreeBuffers but is not FREE (%d)",
+ slot, mSlots[slot].mBufferState);
+ usleep(PAUSE_TIME);
+ }
+ }
+ }
+}
+
} // namespace android
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 6452cdd..a27d5f0 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -161,8 +161,6 @@
}
}
- // 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 @@
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 @@
}
}
+ *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,19 @@
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();
+ // Only return free slots up to the max buffer count
+ if (*slot < maxBufferCount) {
+ *found = *slot;
+ mCore->mFreeSlots.erase(slot);
+ }
+ }
}
// If no buffer is found, or if the queue has too many buffers
@@ -335,6 +339,8 @@
*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 +361,6 @@
return NO_INIT;
}
- mSlots[*outSlot].mFrameNumber = UINT32_MAX;
mSlots[*outSlot].mGraphicBuffer = graphicBuffer;
} // Autolock scope
}
@@ -414,6 +419,7 @@
mCore->freeBufferLocked(slot);
mCore->mDequeueCondition.broadcast();
+ mCore->validateConsistencyLocked();
return NO_ERROR;
}
@@ -438,27 +444,19 @@
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 +504,8 @@
mSlots[*outSlot].mFence = Fence::NO_FENCE;
mSlots[*outSlot].mRequestBufferCalled = true;
+ mCore->validateConsistencyLocked();
+
return returnFlags;
}
@@ -640,9 +640,7 @@
// 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 +662,8 @@
// Take a ticket for the callback functions
callbackTicket = mNextCallbackTicket++;
+
+ mCore->validateConsistencyLocked();
} // Autolock scope
// Wait without lock held
@@ -724,10 +724,11 @@
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 +1010,19 @@
}
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
}
}