From 9b8b9a6e05edf0e24d9bedd0cd89eef80efb3894 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Thu, 21 Jul 2016 17:06:58 -0700 Subject: DO NOT MERGE ANYWHERE: BufferQueue consumers: Add discardFreeBuffer method This method releases all free buffers owned by the buffer queue, in order to save memory (at the cost of potential future reallocation of buffers). Bug: 28695173 Change-Id: I458d10373e639e3144faf673af2ba01aca36e65a (cherry picked from commit 8211047138ea7892c73f4e6f6291a85a11759e0c) --- include/gui/BufferQueueConsumer.h | 3 ++ include/gui/BufferQueueCore.h | 5 +++ include/gui/ConsumerBase.h | 3 ++ include/gui/IGraphicBufferConsumer.h | 5 +++ libs/gui/BufferQueueConsumer.cpp | 6 +++ libs/gui/BufferQueueCore.cpp | 10 +++++ libs/gui/ConsumerBase.cpp | 9 +++++ libs/gui/IGraphicBufferConsumer.cpp | 22 +++++++++++ libs/gui/tests/BufferQueue_test.cpp | 77 ++++++++++++++++++++++++++++++++++++ 9 files changed, 140 insertions(+) diff --git a/include/gui/BufferQueueConsumer.h b/include/gui/BufferQueueConsumer.h index b2daae47c3..3b681ee0da 100644 --- a/include/gui/BufferQueueConsumer.h +++ b/include/gui/BufferQueueConsumer.h @@ -136,6 +136,9 @@ public: // Retrieve the sideband buffer stream, if any. virtual sp getSidebandStream() const; + // See IGraphicBufferConsumer::discardFreeBuffers + virtual status_t discardFreeBuffers() override; + // dump our state in a String virtual void dump(String8& result, const char* prefix) const; diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h index 0371db780b..35c917044a 100644 --- a/include/gui/BufferQueueCore.h +++ b/include/gui/BufferQueueCore.h @@ -124,6 +124,11 @@ private: // all slots, even if they're currently dequeued, queued, or acquired. void freeAllBuffersLocked(); + // discardFreeBuffersLocked releases all currently-free buffers held by the + // queue, in order to reduce the memory consumption of the queue to the + // minimum possible without discarding data. + void discardFreeBuffersLocked(); + // If delta is positive, makes more slots available. If negative, takes // away slots. Returns false if the request can't be met. bool adjustAvailableSlotsLocked(int delta); diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 9307a26fb3..ba864784d2 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -85,6 +85,9 @@ public: // See IGraphicBufferConsumer::setDefaultBufferDataSpace status_t setDefaultBufferDataSpace(android_dataspace defaultDataSpace); + // See IGraphicBufferConsumer::discardFreeBuffers + status_t discardFreeBuffers(); + private: ConsumerBase(const ConsumerBase&); void operator=(const ConsumerBase&); diff --git a/include/gui/IGraphicBufferConsumer.h b/include/gui/IGraphicBufferConsumer.h index e983c160a5..f03d1ce4c2 100644 --- a/include/gui/IGraphicBufferConsumer.h +++ b/include/gui/IGraphicBufferConsumer.h @@ -265,6 +265,11 @@ public: // Retrieve the sideband buffer stream, if any. virtual sp getSidebandStream() const = 0; + // discardFreeBuffers releases all currently-free buffers held by the queue, + // in order to reduce the memory consumption of the queue to the minimum + // possible without discarding data. + virtual status_t discardFreeBuffers() = 0; + // dump state into a string virtual void dump(String8& result, const char* prefix) const = 0; diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index cbc88932ac..525938e7f2 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -717,6 +717,12 @@ sp BufferQueueConsumer::getSidebandStream() const { return mCore->mSidebandStream; } +status_t BufferQueueConsumer::discardFreeBuffers() { + Mutex::Autolock lock(mCore->mMutex); + mCore->discardFreeBuffersLocked(); + return NO_ERROR; +} + void BufferQueueConsumer::dump(String8& result, const char* prefix) const { const IPCThreadState* ipc = IPCThreadState::self(); const pid_t pid = ipc->getCallingPid(); diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index 569b8f9d06..9cb9c62401 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -244,6 +244,16 @@ void BufferQueueCore::freeAllBuffersLocked() { VALIDATE_CONSISTENCY(); } +void BufferQueueCore::discardFreeBuffersLocked() { + for (int s : mFreeBuffers) { + mFreeSlots.insert(s); + clearBufferSlotLocked(s); + } + mFreeBuffers.clear(); + + VALIDATE_CONSISTENCY(); +} + bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) { if (delta >= 0) { // If we're going to fail, do so before modifying anything diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index a6a971282e..c00526dd9c 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -235,6 +235,15 @@ status_t ConsumerBase::setDefaultBufferDataSpace( return mConsumer->setDefaultBufferDataSpace(defaultDataSpace); } +status_t ConsumerBase::discardFreeBuffers() { + Mutex::Autolock _l(mMutex); + if (mAbandoned) { + CB_LOGE("discardFreeBuffers: ConsumerBase is abandoned!"); + return NO_INIT; + } + return mConsumer->discardFreeBuffers(); +} + void ConsumerBase::dump(String8& result) const { dump(result, ""); } diff --git a/libs/gui/IGraphicBufferConsumer.cpp b/libs/gui/IGraphicBufferConsumer.cpp index cb1ad3525e..af6800b16f 100644 --- a/libs/gui/IGraphicBufferConsumer.cpp +++ b/libs/gui/IGraphicBufferConsumer.cpp @@ -51,6 +51,7 @@ enum { SET_CONSUMER_USAGE_BITS, SET_TRANSFORM_HINT, GET_SIDEBAND_STREAM, + DISCARD_FREE_BUFFERS, DUMP, }; @@ -260,6 +261,21 @@ public: return stream; } + virtual status_t discardFreeBuffers() { + Parcel data, reply; + data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor()); + status_t error = remote()->transact(DISCARD_FREE_BUFFERS, data, &reply); + if (error != NO_ERROR) { + return error; + } + int32_t result = NO_ERROR; + error = reply.readInt32(&result); + if (error != NO_ERROR) { + return error; + } + return result; + } + virtual void dump(String8& result, const char* prefix) const { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor()); @@ -409,6 +425,12 @@ status_t BnGraphicBufferConsumer::onTransact( } return NO_ERROR; } + case DISCARD_FREE_BUFFERS: { + CHECK_INTERFACE(IGraphicBufferConsumer, data, reply); + status_t result = discardFreeBuffers(); + status_t error = reply->writeInt32(result); + return error; + } case DUMP: { CHECK_INTERFACE(IGraphicBufferConsumer, data, reply); String8 result = data.readString8(); diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 85d63b4ba2..721667cdc2 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -850,4 +850,81 @@ TEST_F(BufferQueueTest, CanRetrieveLastQueuedBuffer) { returnedBuffer->getNativeBuffer()->handle); } +TEST_F(BufferQueueTest, TestDiscardFreeBuffers) { + createBufferQueue(); + sp dc(new DummyConsumer); + ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false)); + IGraphicBufferProducer::QueueBufferOutput output; + ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener, + NATIVE_WINDOW_API_CPU, false, &output)); + + int slot = BufferQueue::INVALID_BUFFER_SLOT; + sp fence = Fence::NO_FENCE; + sp buffer = nullptr; + IGraphicBufferProducer::QueueBufferInput input(0ull, true, + HAL_DATASPACE_UNKNOWN, Rect::INVALID_RECT, + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE); + BufferItem item{}; + + // Preallocate, dequeue, request, and cancel 4 buffers so we don't get + // BUFFER_NEEDS_REALLOCATION below + int slots[4] = {}; + mProducer->setMaxDequeuedBufferCount(4); + for (size_t i = 0; i < 4; ++i) { + status_t result = mProducer->dequeueBuffer(&slots[i], &fence, + 0, 0, 0, 0); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result); + ASSERT_EQ(OK, mProducer->requestBuffer(slots[i], &buffer)); + } + for (size_t i = 0; i < 4; ++i) { + ASSERT_EQ(OK, mProducer->cancelBuffer(slots[i], Fence::NO_FENCE)); + } + + // Get buffers in all states: dequeued, filled, acquired, free + + // Fill 3 buffers + ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0)); + ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output)); + ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0)); + ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output)); + ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0)); + ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output)); + // Dequeue 1 buffer + ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0)); + + // Acquire and free 1 buffer + ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0)); + ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, + EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE)); + // Acquire 1 buffer, leaving 1 filled buffer in queue + ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0)); + + // Now discard the free buffers + ASSERT_EQ(OK, mConsumer->discardFreeBuffers()); + + // Check no free buffers in dump + String8 dumpString; + mConsumer->dump(dumpString, nullptr); + + // Parse the dump to ensure that all buffer slots that are FREE also + // have a null GraphicBuffer + // Fragile - assumes the following format for the dump for a buffer entry: + // ":%p\][^:]*state=FREE" where %p is the buffer pointer in hex. + ssize_t idx = dumpString.find("state=FREE"); + while (idx != -1) { + ssize_t bufferPtrIdx = idx - 1; + while (bufferPtrIdx > 0) { + if (dumpString[bufferPtrIdx] == ':') { + bufferPtrIdx++; + break; + } + bufferPtrIdx--; + } + ASSERT_GT(bufferPtrIdx, 0) << "Can't parse queue dump to validate"; + ssize_t nullPtrIdx = dumpString.find("0x0]", bufferPtrIdx); + ASSERT_EQ(bufferPtrIdx, nullPtrIdx) << "Free buffer not discarded"; + idx = dumpString.find("FREE", idx + 1); + } +} + } // namespace android -- cgit v1.2.3