Skip to content

Commit 9b8b9a6

Browse files
Eino-Ville Talvalagitbuildkicker
authored andcommitted
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)
1 parent a08cb88 commit 9b8b9a6

9 files changed

+140
-0
lines changed

include/gui/BufferQueueConsumer.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ class BufferQueueConsumer : public BnGraphicBufferConsumer {
136136
// Retrieve the sideband buffer stream, if any.
137137
virtual sp<NativeHandle> getSidebandStream() const;
138138

139+
// See IGraphicBufferConsumer::discardFreeBuffers
140+
virtual status_t discardFreeBuffers() override;
141+
139142
// dump our state in a String
140143
virtual void dump(String8& result, const char* prefix) const;
141144

include/gui/BufferQueueCore.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ class BufferQueueCore : public virtual RefBase {
124124
// all slots, even if they're currently dequeued, queued, or acquired.
125125
void freeAllBuffersLocked();
126126

127+
// discardFreeBuffersLocked releases all currently-free buffers held by the
128+
// queue, in order to reduce the memory consumption of the queue to the
129+
// minimum possible without discarding data.
130+
void discardFreeBuffersLocked();
131+
127132
// If delta is positive, makes more slots available. If negative, takes
128133
// away slots. Returns false if the request can't be met.
129134
bool adjustAvailableSlotsLocked(int delta);

include/gui/ConsumerBase.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ class ConsumerBase : public virtual RefBase,
8585
// See IGraphicBufferConsumer::setDefaultBufferDataSpace
8686
status_t setDefaultBufferDataSpace(android_dataspace defaultDataSpace);
8787

88+
// See IGraphicBufferConsumer::discardFreeBuffers
89+
status_t discardFreeBuffers();
90+
8891
private:
8992
ConsumerBase(const ConsumerBase&);
9093
void operator=(const ConsumerBase&);

include/gui/IGraphicBufferConsumer.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ class IGraphicBufferConsumer : public IInterface {
265265
// Retrieve the sideband buffer stream, if any.
266266
virtual sp<NativeHandle> getSidebandStream() const = 0;
267267

268+
// discardFreeBuffers releases all currently-free buffers held by the queue,
269+
// in order to reduce the memory consumption of the queue to the minimum
270+
// possible without discarding data.
271+
virtual status_t discardFreeBuffers() = 0;
272+
268273
// dump state into a string
269274
virtual void dump(String8& result, const char* prefix) const = 0;
270275

libs/gui/BufferQueueConsumer.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,12 @@ sp<NativeHandle> BufferQueueConsumer::getSidebandStream() const {
717717
return mCore->mSidebandStream;
718718
}
719719

720+
status_t BufferQueueConsumer::discardFreeBuffers() {
721+
Mutex::Autolock lock(mCore->mMutex);
722+
mCore->discardFreeBuffersLocked();
723+
return NO_ERROR;
724+
}
725+
720726
void BufferQueueConsumer::dump(String8& result, const char* prefix) const {
721727
const IPCThreadState* ipc = IPCThreadState::self();
722728
const pid_t pid = ipc->getCallingPid();

libs/gui/BufferQueueCore.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,16 @@ void BufferQueueCore::freeAllBuffersLocked() {
244244
VALIDATE_CONSISTENCY();
245245
}
246246

247+
void BufferQueueCore::discardFreeBuffersLocked() {
248+
for (int s : mFreeBuffers) {
249+
mFreeSlots.insert(s);
250+
clearBufferSlotLocked(s);
251+
}
252+
mFreeBuffers.clear();
253+
254+
VALIDATE_CONSISTENCY();
255+
}
256+
247257
bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) {
248258
if (delta >= 0) {
249259
// If we're going to fail, do so before modifying anything

libs/gui/ConsumerBase.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,15 @@ status_t ConsumerBase::setDefaultBufferDataSpace(
235235
return mConsumer->setDefaultBufferDataSpace(defaultDataSpace);
236236
}
237237

238+
status_t ConsumerBase::discardFreeBuffers() {
239+
Mutex::Autolock _l(mMutex);
240+
if (mAbandoned) {
241+
CB_LOGE("discardFreeBuffers: ConsumerBase is abandoned!");
242+
return NO_INIT;
243+
}
244+
return mConsumer->discardFreeBuffers();
245+
}
246+
238247
void ConsumerBase::dump(String8& result) const {
239248
dump(result, "");
240249
}

libs/gui/IGraphicBufferConsumer.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ enum {
5151
SET_CONSUMER_USAGE_BITS,
5252
SET_TRANSFORM_HINT,
5353
GET_SIDEBAND_STREAM,
54+
DISCARD_FREE_BUFFERS,
5455
DUMP,
5556
};
5657

@@ -260,6 +261,21 @@ class BpGraphicBufferConsumer : public BpInterface<IGraphicBufferConsumer>
260261
return stream;
261262
}
262263

264+
virtual status_t discardFreeBuffers() {
265+
Parcel data, reply;
266+
data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor());
267+
status_t error = remote()->transact(DISCARD_FREE_BUFFERS, data, &reply);
268+
if (error != NO_ERROR) {
269+
return error;
270+
}
271+
int32_t result = NO_ERROR;
272+
error = reply.readInt32(&result);
273+
if (error != NO_ERROR) {
274+
return error;
275+
}
276+
return result;
277+
}
278+
263279
virtual void dump(String8& result, const char* prefix) const {
264280
Parcel data, reply;
265281
data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor());
@@ -409,6 +425,12 @@ status_t BnGraphicBufferConsumer::onTransact(
409425
}
410426
return NO_ERROR;
411427
}
428+
case DISCARD_FREE_BUFFERS: {
429+
CHECK_INTERFACE(IGraphicBufferConsumer, data, reply);
430+
status_t result = discardFreeBuffers();
431+
status_t error = reply->writeInt32(result);
432+
return error;
433+
}
412434
case DUMP: {
413435
CHECK_INTERFACE(IGraphicBufferConsumer, data, reply);
414436
String8 result = data.readString8();

libs/gui/tests/BufferQueue_test.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,4 +850,81 @@ TEST_F(BufferQueueTest, CanRetrieveLastQueuedBuffer) {
850850
returnedBuffer->getNativeBuffer()->handle);
851851
}
852852

853+
TEST_F(BufferQueueTest, TestDiscardFreeBuffers) {
854+
createBufferQueue();
855+
sp<DummyConsumer> dc(new DummyConsumer);
856+
ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false));
857+
IGraphicBufferProducer::QueueBufferOutput output;
858+
ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener,
859+
NATIVE_WINDOW_API_CPU, false, &output));
860+
861+
int slot = BufferQueue::INVALID_BUFFER_SLOT;
862+
sp<Fence> fence = Fence::NO_FENCE;
863+
sp<GraphicBuffer> buffer = nullptr;
864+
IGraphicBufferProducer::QueueBufferInput input(0ull, true,
865+
HAL_DATASPACE_UNKNOWN, Rect::INVALID_RECT,
866+
NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE);
867+
BufferItem item{};
868+
869+
// Preallocate, dequeue, request, and cancel 4 buffers so we don't get
870+
// BUFFER_NEEDS_REALLOCATION below
871+
int slots[4] = {};
872+
mProducer->setMaxDequeuedBufferCount(4);
873+
for (size_t i = 0; i < 4; ++i) {
874+
status_t result = mProducer->dequeueBuffer(&slots[i], &fence,
875+
0, 0, 0, 0);
876+
ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result);
877+
ASSERT_EQ(OK, mProducer->requestBuffer(slots[i], &buffer));
878+
}
879+
for (size_t i = 0; i < 4; ++i) {
880+
ASSERT_EQ(OK, mProducer->cancelBuffer(slots[i], Fence::NO_FENCE));
881+
}
882+
883+
// Get buffers in all states: dequeued, filled, acquired, free
884+
885+
// Fill 3 buffers
886+
ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
887+
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
888+
ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
889+
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
890+
ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
891+
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
892+
// Dequeue 1 buffer
893+
ASSERT_EQ(OK, mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, 0));
894+
895+
// Acquire and free 1 buffer
896+
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
897+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
898+
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
899+
// Acquire 1 buffer, leaving 1 filled buffer in queue
900+
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
901+
902+
// Now discard the free buffers
903+
ASSERT_EQ(OK, mConsumer->discardFreeBuffers());
904+
905+
// Check no free buffers in dump
906+
String8 dumpString;
907+
mConsumer->dump(dumpString, nullptr);
908+
909+
// Parse the dump to ensure that all buffer slots that are FREE also
910+
// have a null GraphicBuffer
911+
// Fragile - assumes the following format for the dump for a buffer entry:
912+
// ":%p\][^:]*state=FREE" where %p is the buffer pointer in hex.
913+
ssize_t idx = dumpString.find("state=FREE");
914+
while (idx != -1) {
915+
ssize_t bufferPtrIdx = idx - 1;
916+
while (bufferPtrIdx > 0) {
917+
if (dumpString[bufferPtrIdx] == ':') {
918+
bufferPtrIdx++;
919+
break;
920+
}
921+
bufferPtrIdx--;
922+
}
923+
ASSERT_GT(bufferPtrIdx, 0) << "Can't parse queue dump to validate";
924+
ssize_t nullPtrIdx = dumpString.find("0x0]", bufferPtrIdx);
925+
ASSERT_EQ(bufferPtrIdx, nullPtrIdx) << "Free buffer not discarded";
926+
idx = dumpString.find("FREE", idx + 1);
927+
}
928+
}
929+
853930
} // namespace android

0 commit comments

Comments
 (0)