Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0de7ea75 authored by Dan Stoza's avatar Dan Stoza
Browse files

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.

Fixes an issue with the first version of this change, found in bugs
20482952, 20443314, and 20464549.

Bug: 13175420
Change-Id: I9b6e83cfe8f9b4329a976025cb8e291d51fb6d4a
parent de5c15b8

include/gui/BufferQueueCore.h

+15 −0
Original line number Diff line number Diff line
@@ -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 @@ private: 
    // 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 @@ private: 
    // 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

libs/gui/BufferQueueConsumer.cpp

+20 −8
Original line number Diff line number Diff line
@@ -120,6 +120,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, 
            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 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, 


    ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size());



    mCore->validateConsistencyLocked();



    return NO_ERROR;

}
@@ -199,6 +202,7 @@ status_t BufferQueueConsumer::detachBuffer(int slot) { 


    mCore->freeBufferLocked(slot);

    mCore->mDequeueCondition.broadcast();

    mCore->validateConsistencyLocked();



    return NO_ERROR;

}
@@ -217,18 +221,11 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot, 


    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 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot, 
                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 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot, 
    // for attached buffers.

    mSlots[*outSlot].mAcquireCalled = false;



    mCore->validateConsistencyLocked();



    return NO_ERROR;

}
@@ -311,6 +321,7 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, 
            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 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, 
        }



        mCore->mDequeueCondition.broadcast();

        mCore->validateConsistencyLocked();

    } // Autolock scope



    // Call back without lock held

libs/gui/BufferQueueCore.cpp

+59 −1
Original line number Diff line number Diff line
@@ -53,6 +53,8 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) : 
    mConnectedProducerListener(),

    mSlots(),

    mQueue(),

    mFreeSlots(),

    mFreeBuffers(),

    mOverrideMaxBufferCount(0),

    mDequeueCondition(),

    mUseAsyncBuffer(true),
@@ -76,6 +78,9 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) : 
            BQ_LOGE("createGraphicBufferAlloc failed");

        }

    }

    for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) {

        mFreeSlots.insert(slot);

    }

}



BufferQueueCore::~BufferQueueCore() {}
@@ -190,12 +195,20 @@ status_t BufferQueueCore::setDefaultMaxBufferCountLocked(int count) { 


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 @@ void BufferQueueCore::freeBufferLocked(int slot) { 
        mSlots[slot].mEglFence = EGL_NO_SYNC_KHR;

    }

    mSlots[slot].mFence = Fence::NO_FENCE;

    validateConsistencyLocked();

}



void BufferQueueCore::freeAllBuffersLocked() {
@@ -236,4 +250,48 @@ 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

libs/gui/BufferQueueProducer.cpp

+37 −30
Original line number Diff line number Diff line
@@ -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,19 @@ 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();

                // 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 @@ 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 +361,6 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot, 
                return NO_INIT;

            }



            mSlots[*outSlot].mFrameNumber = UINT32_MAX;

            mSlots[*outSlot].mGraphicBuffer = graphicBuffer;

        } // Autolock scope

    }
@@ -414,6 +419,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) { 


    mCore->freeBufferLocked(slot);

    mCore->mDequeueCondition.broadcast();

    mCore->validateConsistencyLocked();



    return NO_ERROR;

}
@@ -438,27 +444,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 +504,8 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot, 
    mSlots[*outSlot].mFence = Fence::NO_FENCE;

    mSlots[*outSlot].mRequestBufferCalled = true;



    mCore->validateConsistencyLocked();



    return returnFlags;

}
@@ -640,9 +640,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 +662,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 +724,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 +1010,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

    }

}