Revert "Cache and uncache buffers in the same transaction"
This reverts commit 75ce1ea078444100db9f9eef06a9ef35ad8a0446.
Reason for revert: b/266481887
Change-Id: I147947d55672c8f14d9bbe51df54eadfc43edeb2
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index cefb9a7..a77ca04 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -63,8 +63,7 @@
Vector<ComposerState>& state, const Vector<DisplayState>& displays,
uint32_t flags, const sp<IBinder>& applyToken,
const InputWindowCommands& commands, int64_t desiredPresentTime,
- bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
+ bool isAutoTimestamp, const client_cache_t& uncacheBuffer,
bool hasListenerCallbacks,
const std::vector<ListenerCallbacks>& listenerCallbacks,
uint64_t transactionId) override {
@@ -88,11 +87,8 @@
SAFE_PARCEL(commands.write, data);
SAFE_PARCEL(data.writeInt64, desiredPresentTime);
SAFE_PARCEL(data.writeBool, isAutoTimestamp);
- SAFE_PARCEL(data.writeUint32, static_cast<uint32_t>(uncacheBuffers.size()));
- for (const client_cache_t& uncacheBuffer : uncacheBuffers) {
- SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote());
- SAFE_PARCEL(data.writeUint64, uncacheBuffer.id);
- }
+ SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote());
+ SAFE_PARCEL(data.writeUint64, uncacheBuffer.id);
SAFE_PARCEL(data.writeBool, hasListenerCallbacks);
SAFE_PARCEL(data.writeVectorSize, listenerCallbacks);
@@ -162,14 +158,11 @@
SAFE_PARCEL(data.readInt64, &desiredPresentTime);
SAFE_PARCEL(data.readBool, &isAutoTimestamp);
- SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize());
- std::vector<client_cache_t> uncacheBuffers(count);
+ client_cache_t uncachedBuffer;
sp<IBinder> tmpBinder;
- for (size_t i = 0; i < count; i++) {
- SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder);
- uncacheBuffers[i].token = tmpBinder;
- SAFE_PARCEL(data.readUint64, &uncacheBuffers[i].id);
- }
+ SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder);
+ uncachedBuffer.token = tmpBinder;
+ SAFE_PARCEL(data.readUint64, &uncachedBuffer.id);
bool hasListenerCallbacks = false;
SAFE_PARCEL(data.readBool, &hasListenerCallbacks);
@@ -189,7 +182,7 @@
return setTransactionState(frameTimelineInfo, state, displays, stateFlags, applyToken,
inputWindowCommands, desiredPresentTime, isAutoTimestamp,
- uncacheBuffers, hasListenerCallbacks, listenerCallbacks,
+ uncachedBuffer, hasListenerCallbacks, listenerCallbacks,
transactionId);
}
default: {
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 21a7f78..92125ea 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -565,13 +565,11 @@
return NO_ERROR;
}
- uint64_t cache(const sp<GraphicBuffer>& buffer,
- std::optional<client_cache_t>& outUncacheBuffer) {
+ uint64_t cache(const sp<GraphicBuffer>& buffer) {
std::lock_guard<std::mutex> lock(mMutex);
if (mBuffers.size() >= BUFFER_CACHE_MAX_SIZE) {
- outUncacheBuffer = findLeastRecentlyUsedBuffer();
- mBuffers.erase(outUncacheBuffer->id);
+ evictLeastRecentlyUsedBuffer();
}
buffer->addDeathCallback(removeDeadBufferCallback, nullptr);
@@ -582,13 +580,16 @@
void uncache(uint64_t cacheId) {
std::lock_guard<std::mutex> lock(mMutex);
- if (mBuffers.erase(cacheId)) {
- SurfaceComposerClient::doUncacheBufferTransaction(cacheId);
- }
+ uncacheLocked(cacheId);
+ }
+
+ void uncacheLocked(uint64_t cacheId) REQUIRES(mMutex) {
+ mBuffers.erase(cacheId);
+ SurfaceComposerClient::doUncacheBufferTransaction(cacheId);
}
private:
- client_cache_t findLeastRecentlyUsedBuffer() REQUIRES(mMutex) {
+ void evictLeastRecentlyUsedBuffer() REQUIRES(mMutex) {
auto itr = mBuffers.begin();
uint64_t minCounter = itr->second;
auto minBuffer = itr;
@@ -602,8 +603,7 @@
}
itr++;
}
-
- return {.token = getToken(), .id = minBuffer->first};
+ uncacheLocked(minBuffer->first);
}
uint64_t getCounter() REQUIRES(mMutex) {
@@ -741,18 +741,6 @@
InputWindowCommands inputWindowCommands;
inputWindowCommands.read(*parcel);
- count = static_cast<size_t>(parcel->readUint32());
- if (count > parcel->dataSize()) {
- return BAD_VALUE;
- }
- std::vector<client_cache_t> uncacheBuffers(count);
- for (size_t i = 0; i < count; i++) {
- sp<IBinder> tmpBinder;
- SAFE_PARCEL(parcel->readStrongBinder, &tmpBinder);
- uncacheBuffers[i].token = tmpBinder;
- SAFE_PARCEL(parcel->readUint64, &uncacheBuffers[i].id);
- }
-
// Parsing was successful. Update the object.
mId = transactionId;
mTransactionNestCount = transactionNestCount;
@@ -767,7 +755,6 @@
mComposerStates = composerStates;
mInputWindowCommands = inputWindowCommands;
mApplyToken = applyToken;
- mUncacheBuffers = std::move(uncacheBuffers);
return NO_ERROR;
}
@@ -819,13 +806,6 @@
}
mInputWindowCommands.write(*parcel);
-
- SAFE_PARCEL(parcel->writeUint32, static_cast<uint32_t>(mUncacheBuffers.size()));
- for (const client_cache_t& uncacheBuffer : mUncacheBuffers) {
- SAFE_PARCEL(parcel->writeStrongBinder, uncacheBuffer.token.promote());
- SAFE_PARCEL(parcel->writeUint64, uncacheBuffer.id);
- }
-
return NO_ERROR;
}
@@ -893,10 +873,6 @@
}
}
- for (const auto& cacheId : other.mUncacheBuffers) {
- mUncacheBuffers.push_back(cacheId);
- }
-
mInputWindowCommands.merge(other.mInputWindowCommands);
mMayContainBuffer |= other.mMayContainBuffer;
@@ -915,7 +891,6 @@
mDisplayStates.clear();
mListenerCallbacks.clear();
mInputWindowCommands.clear();
- mUncacheBuffers.clear();
mMayContainBuffer = false;
mTransactionNestCount = 0;
mAnimation = false;
@@ -938,10 +913,10 @@
uncacheBuffer.token = BufferCache::getInstance().getToken();
uncacheBuffer.id = cacheId;
Vector<ComposerState> composerStates;
- status_t status = sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},
- ISurfaceComposer::eOneWay,
- Transaction::getDefaultApplyToken(), {}, systemTime(),
- true, {uncacheBuffer}, false, {}, generateId());
+ status_t status =
+ sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},
+ ISurfaceComposer::eOneWay, Transaction::getDefaultApplyToken(),
+ {}, systemTime(), true, uncacheBuffer, false, {}, generateId());
if (status != NO_ERROR) {
ALOGE_AND_TRACE("SurfaceComposerClient::doUncacheBufferTransaction - %s",
strerror(-status));
@@ -979,11 +954,7 @@
s->bufferData->buffer = nullptr;
} else {
// Cache-miss. Include the buffer and send the new cacheId.
- std::optional<client_cache_t> uncacheBuffer;
- cacheId = BufferCache::getInstance().cache(s->bufferData->buffer, uncacheBuffer);
- if (uncacheBuffer) {
- mUncacheBuffers.push_back(*uncacheBuffer);
- }
+ cacheId = BufferCache::getInstance().cache(s->bufferData->buffer);
}
s->bufferData->flags |= BufferData::BufferDataChange::cachedBufferChanged;
s->bufferData->cachedBuffer.token = BufferCache::getInstance().getToken();
@@ -1116,7 +1087,8 @@
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,
mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,
- mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId);
+ {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,
+ hasListenerCallbacks, listenerCallbacks, mId);
mId = generateId();
// Clear the current states and flags
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index ae56f9f..045cc2a 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -113,9 +113,8 @@
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
- bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffer,
- bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) = 0;
+ bool isAutoTimestamp, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
+ const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) = 0;
};
// ----------------------------------------------------------------------------
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 0e51dcf..2458a40 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -402,7 +402,6 @@
SortedVector<DisplayState> mDisplayStates;
std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>
mListenerCallbacks;
- std::vector<client_cache_t> mUncacheBuffers;
uint64_t mId;
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 32d60cd..3014804 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -701,7 +701,7 @@
const sp<IBinder>& /*applyToken*/,
const InputWindowCommands& /*inputWindowCommands*/,
int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,
- const std::vector<client_cache_t>& /*cachedBuffer*/,
+ const client_cache_t& /*cachedBuffer*/,
bool /*hasListenerCallbacks*/,
const std::vector<ListenerCallbacks>& /*listenerCallbacks*/,
uint64_t /*transactionId*/) override {