Uncache the active buffer slot last
This allows the memory for the active buffer to be freed as soon as
possible by purging it from HWC cache as soon as the next buffer is sent
to the layer.
Bug: 258196272
Test: atest OutputLayerUncacheBufferTest
Change-Id: I96c24390de5757ac99b369119e9ba031afb0b042
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
index 2a5bfae..b86782f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
@@ -137,6 +137,7 @@
HwcBufferCache hwcBufferCache;
// The previously-active buffer for this layer.
+ uint64_t activeBufferId;
uint32_t activeBufferSlot;
// Set to true when overridden info has been sent to HW composer
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 766d7ea..60a2c83 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -617,13 +617,24 @@
return;
}
+ // Uncache the active buffer last so that it's the first buffer to be purged from the cache
+ // next time a buffer is sent to this layer.
+ bool uncacheActiveBuffer = false;
+
std::vector<uint32_t> slotsToClear;
for (uint64_t bufferId : bufferIdsToUncache) {
- uint32_t slot = state.hwc->hwcBufferCache.uncache(bufferId);
- if (slot != UINT32_MAX) {
- slotsToClear.push_back(slot);
+ if (bufferId == state.hwc->activeBufferId) {
+ uncacheActiveBuffer = true;
+ } else {
+ uint32_t slot = state.hwc->hwcBufferCache.uncache(bufferId);
+ if (slot != UINT32_MAX) {
+ slotsToClear.push_back(slot);
+ }
}
}
+ if (uncacheActiveBuffer) {
+ slotsToClear.push_back(state.hwc->hwcBufferCache.uncache(state.hwc->activeBufferId));
+ }
hal::Error error =
state.hwc->hwcLayer->setBufferSlotsToClear(slotsToClear, state.hwc->activeBufferSlot);
@@ -655,17 +666,20 @@
hwcSlotAndBuffer = state.hwc->hwcBufferCache.getOverrideHwcSlotAndBuffer(
state.overrideInfo.buffer->getBuffer());
hwcFence = state.overrideInfo.acquireFence;
+ // Keep track of the active buffer ID so when it's discarded we uncache it last so its
+ // slot will be used first, allowing the memory to be freed as soon as possible.
+ state.hwc->activeBufferId = state.overrideInfo.buffer->getBuffer()->getId();
} else {
hwcSlotAndBuffer =
state.hwc->hwcBufferCache.getHwcSlotAndBuffer(outputIndependentState.buffer);
hwcFence = outputIndependentState.acquireFence;
+ // Keep track of the active buffer ID so when it's discarded we uncache it last so its
+ // slot will be used first, allowing the memory to be freed as soon as possible.
+ state.hwc->activeBufferId = outputIndependentState.buffer->getId();
}
-
// Keep track of the active buffer slot, so we can restore it after clearing other buffer
// slots.
- if (hwcSlotAndBuffer.buffer) {
- state.hwc->activeBufferSlot = hwcSlotAndBuffer.slot;
- }
+ state.hwc->activeBufferSlot = hwcSlotAndBuffer.slot;
}
if (auto error = hwcLayer->setBuffer(hwcSlotAndBuffer.slot, hwcSlotAndBuffer.buffer, hwcFence);
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 0edc226..9ad2edb 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -1318,6 +1318,7 @@
struct OutputLayerUncacheBufferTest : public OutputLayerTest {
static const sp<GraphicBuffer> kBuffer1;
static const sp<GraphicBuffer> kBuffer2;
+ static const sp<GraphicBuffer> kBuffer3;
static const sp<Fence> kFence;
OutputLayerUncacheBufferTest() {
@@ -1343,6 +1344,10 @@
sp<GraphicBuffer>::make(2, 3, PIXEL_FORMAT_RGBA_8888,
AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN |
AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN);
+const sp<GraphicBuffer> OutputLayerUncacheBufferTest::kBuffer3 =
+ sp<GraphicBuffer>::make(4, 5, PIXEL_FORMAT_RGBA_8888,
+ AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN |
+ AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN);
const sp<Fence> OutputLayerUncacheBufferTest::kFence = sp<Fence>::make();
TEST_F(OutputLayerUncacheBufferTest, canUncacheAndReuseSlot) {
@@ -1360,26 +1365,38 @@
/*zIsOverridden*/ false, /*isPeekingThrough*/ false);
Mock::VerifyAndClearExpectations(&mHwcLayer);
- // buffer slots are cleared in HWC
- std::vector<uint32_t> slotsToClear = {0, 1};
- EXPECT_CALL(mHwcLayer,
- setBufferSlotsToClear(/*slotsToClear*/ slotsToClear, /*activeBufferSlot*/ 1));
- mOutputLayer.uncacheBuffers({kBuffer1->getId(), kBuffer2->getId()});
+ // Buffer3 is stored in slot 2
+ mLayerFEState.buffer = kBuffer3;
+ EXPECT_CALL(mHwcLayer, setBuffer(/*slot*/ 2, kBuffer3, kFence));
+ mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0,
+ /*zIsOverridden*/ false, /*isPeekingThrough*/ false);
Mock::VerifyAndClearExpectations(&mHwcLayer);
- // rather than allocating a new slot, the active buffer slot (slot 1) is reused first to free
- // the memory as soon as possible
+ // Buffer2 becomes the active buffer again (with a nullptr) and reuses slot 1
+ mLayerFEState.buffer = kBuffer2;
+ sp<GraphicBuffer> nullBuffer = nullptr;
+ EXPECT_CALL(mHwcLayer, setBuffer(/*slot*/ 1, nullBuffer, kFence));
+ mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0,
+ /*zIsOverridden*/ false, /*isPeekingThrough*/ false);
+ Mock::VerifyAndClearExpectations(&mHwcLayer);
+
+ // Buffer slots are cleared
+ std::vector<uint32_t> slotsToClear = {0, 2, 1}; // order doesn't matter
+ EXPECT_CALL(mHwcLayer, setBufferSlotsToClear(slotsToClear, /*activeBufferSlot*/ 1));
+ // Uncache the active buffer in between other buffers to exercise correct algorithmic behavior.
+ mOutputLayer.uncacheBuffers({kBuffer1->getId(), kBuffer2->getId(), kBuffer3->getId()});
+ Mock::VerifyAndClearExpectations(&mHwcLayer);
+
+ // Buffer1 becomes active again, and rather than allocating a new slot, or re-using slot 0,
+ // the active buffer slot (slot 1 for Buffer2) is reused first, which allows HWC to free the
+ // memory for the active buffer. Note: slot 1 is different from the first and last buffer slot
+ // requested to be cleared in slotsToClear (slot 1), above, indicating that the algorithm
+ // correctly identifies the active buffer as the buffer in slot 1, despite ping-ponging.
mLayerFEState.buffer = kBuffer1;
EXPECT_CALL(mHwcLayer, setBuffer(/*slot*/ 1, kBuffer1, kFence));
mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0,
/*zIsOverridden*/ false, /*isPeekingThrough*/ false);
Mock::VerifyAndClearExpectations(&mHwcLayer);
-
- // rather than allocating a new slot, slot 0 is reused
- mLayerFEState.buffer = kBuffer2;
- EXPECT_CALL(mHwcLayer, setBuffer(/*slot*/ 0, kBuffer2, kFence));
- mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0,
- /*zIsOverridden*/ false, /*isPeekingThrough*/ false);
}
/*