am d2c12e4c: am 7637e35d: Merge "Revert "libgui: Change BufferQueue to use free lists""

* commit 'd2c12e4cd394ab9ed642526f8bd0e1b3acd692ae':
  Revert "libgui: Change BufferQueue to use free lists"
This commit is contained in:
Dan Stoza 2015-04-22 19:17:56 +00:00 committed by Android Git Automerger
commit 41422980fa
4 changed files with 39 additions and 128 deletions

View File

@ -30,9 +30,6 @@
#include <utils/Trace.h> #include <utils/Trace.h>
#include <utils/Vector.h> #include <utils/Vector.h>
#include <list>
#include <set>
#define BQ_LOGV(x, ...) ALOGV("[%s] " x, mConsumerName.string(), ##__VA_ARGS__) #define BQ_LOGV(x, ...) ALOGV("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
#define BQ_LOGD(x, ...) ALOGD("[%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__) #define BQ_LOGI(x, ...) ALOGI("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
@ -126,10 +123,6 @@ private:
// waitWhileAllocatingLocked blocks until mIsAllocating is false. // waitWhileAllocatingLocked blocks until mIsAllocating is false.
void waitWhileAllocatingLocked() const; 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 // mAllocator is the connection to SurfaceFlinger that is used to allocate
// new GraphicBuffer objects. // new GraphicBuffer objects.
sp<IGraphicBufferAlloc> mAllocator; sp<IGraphicBufferAlloc> mAllocator;
@ -184,14 +177,6 @@ private:
// mQueue is a FIFO of queued buffers used in synchronous mode. // mQueue is a FIFO of queued buffers used in synchronous mode.
Fifo mQueue; 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 // 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 // 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 // setBufferCount. The default is 0, which means that the producer doesn't

View File

@ -120,7 +120,6 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
if (mCore->stillTracking(front)) { if (mCore->stillTracking(front)) {
// Front buffer is still in mSlots, so mark the slot as free // Front buffer is still in mSlots, so mark the slot as free
mSlots[front->mSlot].mBufferState = BufferSlot::FREE; mSlots[front->mSlot].mBufferState = BufferSlot::FREE;
mCore->mFreeBuffers.push_back(front->mSlot);
} }
mCore->mQueue.erase(front); mCore->mQueue.erase(front);
front = mCore->mQueue.begin(); front = mCore->mQueue.begin();
@ -174,8 +173,6 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size()); ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size());
mCore->validateConsistencyLocked();
return NO_ERROR; return NO_ERROR;
} }
@ -202,7 +199,6 @@ status_t BufferQueueConsumer::detachBuffer(int slot) {
mCore->freeBufferLocked(slot); mCore->freeBufferLocked(slot);
mCore->mDequeueCondition.broadcast(); mCore->mDequeueCondition.broadcast();
mCore->validateConsistencyLocked();
return NO_ERROR; return NO_ERROR;
} }
@ -221,11 +217,18 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot,
Mutex::Autolock lock(mCore->mMutex); Mutex::Autolock lock(mCore->mMutex);
// Make sure we don't have too many acquired buffers // 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).
int numAcquiredBuffers = 0; int numAcquiredBuffers = 0;
int found = BufferQueueCore::INVALID_BUFFER_SLOT;
for (int s = 0; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) { for (int s = 0; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
if (mSlots[s].mBufferState == BufferSlot::ACQUIRED) { if (mSlots[s].mBufferState == BufferSlot::ACQUIRED) {
++numAcquiredBuffers; ++numAcquiredBuffers;
} else if (mSlots[s].mBufferState == BufferSlot::FREE) {
if (found == BufferQueueCore::INVALID_BUFFER_SLOT ||
mSlots[s].mFrameNumber < mSlots[found].mFrameNumber) {
found = s;
}
} }
} }
@ -235,17 +238,6 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot,
mCore->mMaxAcquiredBufferCount); mCore->mMaxAcquiredBufferCount);
return INVALID_OPERATION; 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) { if (found == BufferQueueCore::INVALID_BUFFER_SLOT) {
BQ_LOGE("attachBuffer(P): could not find free buffer slot"); BQ_LOGE("attachBuffer(P): could not find free buffer slot");
return NO_MEMORY; return NO_MEMORY;
@ -279,8 +271,6 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot,
// for attached buffers. // for attached buffers.
mSlots[*outSlot].mAcquireCalled = false; mSlots[*outSlot].mAcquireCalled = false;
mCore->validateConsistencyLocked();
return NO_ERROR; return NO_ERROR;
} }
@ -321,7 +311,6 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber,
mSlots[slot].mEglFence = eglFence; mSlots[slot].mEglFence = eglFence;
mSlots[slot].mFence = releaseFence; mSlots[slot].mFence = releaseFence;
mSlots[slot].mBufferState = BufferSlot::FREE; mSlots[slot].mBufferState = BufferSlot::FREE;
mCore->mFreeBuffers.push_back(slot);
listener = mCore->mConnectedProducerListener; listener = mCore->mConnectedProducerListener;
BQ_LOGV("releaseBuffer: releasing slot %d", slot); BQ_LOGV("releaseBuffer: releasing slot %d", slot);
} else if (mSlots[slot].mNeedsCleanupOnRelease) { } else if (mSlots[slot].mNeedsCleanupOnRelease) {
@ -336,7 +325,6 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber,
} }
mCore->mDequeueCondition.broadcast(); mCore->mDequeueCondition.broadcast();
mCore->validateConsistencyLocked();
} // Autolock scope } // Autolock scope
// Call back without lock held // Call back without lock held

View File

@ -53,8 +53,6 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) :
mConnectedProducerListener(), mConnectedProducerListener(),
mSlots(), mSlots(),
mQueue(), mQueue(),
mFreeSlots(),
mFreeBuffers(),
mOverrideMaxBufferCount(0), mOverrideMaxBufferCount(0),
mDequeueCondition(), mDequeueCondition(),
mUseAsyncBuffer(true), mUseAsyncBuffer(true),
@ -78,9 +76,6 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) :
BQ_LOGE("createGraphicBufferAlloc failed"); BQ_LOGE("createGraphicBufferAlloc failed");
} }
} }
for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) {
mFreeSlots.insert(slot);
}
} }
BufferQueueCore::~BufferQueueCore() {} BufferQueueCore::~BufferQueueCore() {}
@ -195,20 +190,12 @@ status_t BufferQueueCore::setDefaultMaxBufferCountLocked(int count) {
void BufferQueueCore::freeBufferLocked(int slot) { void BufferQueueCore::freeBufferLocked(int slot) {
BQ_LOGV("freeBufferLocked: slot %d", slot); BQ_LOGV("freeBufferLocked: slot %d", slot);
bool hadBuffer = mSlots[slot].mGraphicBuffer != NULL;
mSlots[slot].mGraphicBuffer.clear(); mSlots[slot].mGraphicBuffer.clear();
if (mSlots[slot].mBufferState == BufferSlot::ACQUIRED) { if (mSlots[slot].mBufferState == BufferSlot::ACQUIRED) {
mSlots[slot].mNeedsCleanupOnRelease = true; 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].mBufferState = BufferSlot::FREE;
mSlots[slot].mFrameNumber = UINT32_MAX;
mSlots[slot].mAcquireCalled = false; mSlots[slot].mAcquireCalled = false;
// Destroy fence as BufferQueue now takes ownership // Destroy fence as BufferQueue now takes ownership
@ -217,7 +204,6 @@ void BufferQueueCore::freeBufferLocked(int slot) {
mSlots[slot].mEglFence = EGL_NO_SYNC_KHR; mSlots[slot].mEglFence = EGL_NO_SYNC_KHR;
} }
mSlots[slot].mFence = Fence::NO_FENCE; mSlots[slot].mFence = Fence::NO_FENCE;
validateConsistencyLocked();
} }
void BufferQueueCore::freeAllBuffersLocked() { void BufferQueueCore::freeAllBuffersLocked() {
@ -250,48 +236,4 @@ void BufferQueueCore::waitWhileAllocatingLocked() const {
} }
} }
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 } // namespace android

View File

@ -161,6 +161,8 @@ 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 dequeuedCount = 0;
int acquiredCount = 0; int acquiredCount = 0;
for (int s = 0; s < maxBufferCount; ++s) { for (int s = 0; s < maxBufferCount; ++s) {
@ -171,6 +173,15 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
case BufferSlot::ACQUIRED: case BufferSlot::ACQUIRED:
++acquiredCount; ++acquiredCount;
break; 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: default:
break; break;
} }
@ -203,8 +214,6 @@ 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 // 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 // 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 // cause us to run out of memory if we outrun the consumer. Wait here if
@ -214,16 +223,6 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
if (tooManyBuffers) { if (tooManyBuffers) {
BQ_LOGV("%s: queue size is %zu, waiting", caller, BQ_LOGV("%s: queue size is %zu, waiting", caller,
mCore->mQueue.size()); 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 // If no buffer is found, or if the queue has too many buffers
@ -336,8 +335,6 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
*outFence = mSlots[found].mFence; *outFence = mSlots[found].mFence;
mSlots[found].mEglFence = EGL_NO_SYNC_KHR; mSlots[found].mEglFence = EGL_NO_SYNC_KHR;
mSlots[found].mFence = Fence::NO_FENCE; mSlots[found].mFence = Fence::NO_FENCE;
mCore->validateConsistencyLocked();
} // Autolock scope } // Autolock scope
if (returnFlags & BUFFER_NEEDS_REALLOCATION) { if (returnFlags & BUFFER_NEEDS_REALLOCATION) {
@ -358,6 +355,7 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
return NO_INIT; return NO_INIT;
} }
mSlots[*outSlot].mFrameNumber = UINT32_MAX;
mSlots[*outSlot].mGraphicBuffer = graphicBuffer; mSlots[*outSlot].mGraphicBuffer = graphicBuffer;
} // Autolock scope } // Autolock scope
} }
@ -416,7 +414,6 @@ status_t BufferQueueProducer::detachBuffer(int slot) {
mCore->freeBufferLocked(slot); mCore->freeBufferLocked(slot);
mCore->mDequeueCondition.broadcast(); mCore->mDequeueCondition.broadcast();
mCore->validateConsistencyLocked();
return NO_ERROR; return NO_ERROR;
} }
@ -441,19 +438,27 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
return NO_INIT; return NO_INIT;
} }
if (mCore->mFreeBuffers.empty()) { // Find the oldest valid slot
return NO_MEMORY; 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;
}
}
} }
int found = mCore->mFreeBuffers.front(); if (found == BufferQueueCore::INVALID_BUFFER_SLOT) {
mCore->mFreeBuffers.remove(found); return NO_MEMORY;
}
BQ_LOGV("detachNextBuffer detached slot %d", found); BQ_LOGV("detachNextBuffer detached slot %d", found);
*outBuffer = mSlots[found].mGraphicBuffer; *outBuffer = mSlots[found].mGraphicBuffer;
*outFence = mSlots[found].mFence; *outFence = mSlots[found].mFence;
mCore->freeBufferLocked(found); mCore->freeBufferLocked(found);
mCore->validateConsistencyLocked();
return NO_ERROR; return NO_ERROR;
} }
@ -501,8 +506,6 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot,
mSlots[*outSlot].mFence = Fence::NO_FENCE; mSlots[*outSlot].mFence = Fence::NO_FENCE;
mSlots[*outSlot].mRequestBufferCalled = true; mSlots[*outSlot].mRequestBufferCalled = true;
mCore->validateConsistencyLocked();
return returnFlags; return returnFlags;
} }
@ -637,7 +640,9 @@ status_t BufferQueueProducer::queueBuffer(int slot,
// mark it as freed // mark it as freed
if (mCore->stillTracking(front)) { if (mCore->stillTracking(front)) {
mSlots[front->mSlot].mBufferState = BufferSlot::FREE; mSlots[front->mSlot].mBufferState = BufferSlot::FREE;
mCore->mFreeBuffers.push_front(front->mSlot); // 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;
} }
// Overwrite the droppable buffer with the incoming one // Overwrite the droppable buffer with the incoming one
*front = item; *front = item;
@ -659,8 +664,6 @@ status_t BufferQueueProducer::queueBuffer(int slot,
// Take a ticket for the callback functions // Take a ticket for the callback functions
callbackTicket = mNextCallbackTicket++; callbackTicket = mNextCallbackTicket++;
mCore->validateConsistencyLocked();
} // Autolock scope } // Autolock scope
// Wait without lock held // Wait without lock held
@ -721,11 +724,10 @@ void BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
return; return;
} }
mCore->mFreeBuffers.push_front(slot);
mSlots[slot].mBufferState = BufferSlot::FREE; mSlots[slot].mBufferState = BufferSlot::FREE;
mSlots[slot].mFrameNumber = 0;
mSlots[slot].mFence = fence; mSlots[slot].mFence = fence;
mCore->mDequeueCondition.broadcast(); mCore->mDequeueCondition.broadcast();
mCore->validateConsistencyLocked();
} }
int BufferQueueProducer::query(int what, int *outValue) { int BufferQueueProducer::query(int what, int *outValue) {
@ -1007,19 +1009,13 @@ void BufferQueueProducer::allocateBuffers(bool async, uint32_t width,
} }
mCore->freeBufferLocked(slot); // Clean up the slot first mCore->freeBufferLocked(slot); // Clean up the slot first
mSlots[slot].mGraphicBuffer = buffers[i]; mSlots[slot].mGraphicBuffer = buffers[i];
mSlots[slot].mFrameNumber = 0;
mSlots[slot].mFence = Fence::NO_FENCE; 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); BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d", slot);
} }
mCore->mIsAllocating = false; mCore->mIsAllocating = false;
mCore->mIsAllocatingCondition.broadcast(); mCore->mIsAllocatingCondition.broadcast();
mCore->validateConsistencyLocked();
} // Autolock scope } // Autolock scope
} }
} }