Replace SortedVector and maps with Vector in Transactions
Align the data in client so we can remove some duplicate parcelling and
clean up the interface to sf.
micro benchmark results (atest libgui_benchmarks) shows slight
improvements when merging and applying transactions. 5-10%
Note: we would ideally switch to std::vector but this would involve more
changes. Once we clean up the interfaces to sf, changing the container
would be easier.
Flag: EXEMPT refactor
Bug: 385156191
Test: presubmit
Change-Id: I2f14236e5f571af192d371b7b58f0a0fa0c62f49
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index c6ba7d8..852885b 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -19,6 +19,7 @@
#include <semaphore.h>
#include <stdint.h>
#include <sys/types.h>
+#include <algorithm>
#include <android/gui/BnWindowInfosReportedListener.h>
#include <android/gui/DisplayState.h>
@@ -844,7 +845,7 @@
void SurfaceComposerClient::Transaction::sanitize(int pid, int uid) {
uint32_t permissions = LayerStatePermissions::getTransactionPermissions(pid, uid);
- for (auto & [handle, composerState] : mComposerStates) {
+ for (auto& composerState : mComposerStates) {
composerState.state.sanitize(permissions);
}
if (!mInputWindowCommands.empty() &&
@@ -879,7 +880,7 @@
if (count > parcel->dataSize()) {
return BAD_VALUE;
}
- SortedVector<DisplayState> displayStates;
+ Vector<DisplayState> displayStates;
displayStates.setCapacity(count);
for (size_t i = 0; i < count; i++) {
DisplayState displayState;
@@ -922,17 +923,14 @@
if (count > parcel->dataSize()) {
return BAD_VALUE;
}
- std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> composerStates;
- composerStates.reserve(count);
+ Vector<ComposerState> composerStates;
+ composerStates.setCapacity(count);
for (size_t i = 0; i < count; i++) {
- sp<IBinder> surfaceControlHandle;
- SAFE_PARCEL(parcel->readStrongBinder, &surfaceControlHandle);
-
ComposerState composerState;
if (composerState.read(*parcel) == BAD_VALUE) {
return BAD_VALUE;
}
- composerStates[surfaceControlHandle] = composerState;
+ composerStates.add(composerState);
}
InputWindowCommands inputWindowCommands;
@@ -965,9 +963,9 @@
mDesiredPresentTime = desiredPresentTime;
mIsAutoTimestamp = isAutoTimestamp;
mFrameTimelineInfo = frameTimelineInfo;
- mDisplayStates = displayStates;
+ mDisplayStates = std::move(displayStates);
mListenerCallbacks = listenerCallbacks;
- mComposerStates = composerStates;
+ mComposerStates = std::move(composerStates);
mInputWindowCommands = inputWindowCommands;
mApplyToken = applyToken;
mUncacheBuffers = std::move(uncacheBuffers);
@@ -1015,8 +1013,7 @@
}
parcel->writeUint32(static_cast<uint32_t>(mComposerStates.size()));
- for (auto const& [handle, composerState] : mComposerStates) {
- SAFE_PARCEL(parcel->writeStrongBinder, handle);
+ for (auto const& composerState : mComposerStates) {
composerState.write(*parcel);
}
@@ -1073,23 +1070,31 @@
}
mMergedTransactionIds.insert(mMergedTransactionIds.begin(), other.mId);
- for (auto const& [handle, composerState] : other.mComposerStates) {
- if (mComposerStates.count(handle) == 0) {
- mComposerStates[handle] = composerState;
- } else {
- if (composerState.state.what & layer_state_t::eBufferChanged) {
- releaseBufferIfOverwriting(mComposerStates[handle].state);
+ for (auto const& otherState : other.mComposerStates) {
+ if (auto it = std::find_if(mComposerStates.begin(), mComposerStates.end(),
+ [&otherState](const auto& composerState) {
+ return composerState.state.surface ==
+ otherState.state.surface;
+ });
+ it != mComposerStates.end()) {
+ if (otherState.state.what & layer_state_t::eBufferChanged) {
+ releaseBufferIfOverwriting(it->state);
}
- mComposerStates[handle].state.merge(composerState.state);
+ it->state.merge(otherState.state);
+ } else {
+ mComposerStates.add(otherState);
}
}
for (auto const& state : other.mDisplayStates) {
- ssize_t index = mDisplayStates.indexOf(state);
- if (index < 0) {
- mDisplayStates.add(state);
+ if (auto it = std::find_if(mDisplayStates.begin(), mDisplayStates.end(),
+ [&state](const auto& displayState) {
+ return displayState.token == state.token;
+ });
+ it != mDisplayStates.end()) {
+ it->merge(state);
} else {
- mDisplayStates.editItemAt(static_cast<size_t>(index)).merge(state);
+ mDisplayStates.add(state);
}
}
@@ -1186,8 +1191,8 @@
}
size_t count = 0;
- for (auto& [handle, cs] : mComposerStates) {
- layer_state_t* s = &(mComposerStates[handle].state);
+ for (auto& cs : mComposerStates) {
+ layer_state_t* s = &cs.state;
if (!(s->what & layer_state_t::eBufferChanged)) {
continue;
} else if (s->bufferData &&
@@ -1312,15 +1317,6 @@
cacheBuffers();
- Vector<ComposerState> composerStates;
- Vector<DisplayState> displayStates;
-
- for (auto const& kv : mComposerStates) {
- composerStates.add(kv.second);
- }
-
- displayStates = std::move(mDisplayStates);
-
if (oneWay) {
if (synchronous) {
ALOGE("Transaction attempted to set synchronous and one way at the same time"
@@ -1340,7 +1336,7 @@
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
status_t binderStatus =
- sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, mFlags,
+ sf->setTransactionState(mFrameTimelineInfo, mComposerStates, mDisplayStates, mFlags,
applyToken, mInputWindowCommands, mDesiredPresentTime,
mIsAutoTimestamp, mUncacheBuffers, hasListenerCallbacks,
listenerCallbacks, mId, mMergedTransactionIds);
@@ -1456,18 +1452,21 @@
layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<SurfaceControl>& sc) {
auto handle = sc->getLayerStateHandle();
-
- if (mComposerStates.count(handle) == 0) {
- // we don't have it, add an initialized layer_state to our list
- ComposerState s;
-
- s.state.surface = handle;
- s.state.layerId = sc->getLayerId();
-
- mComposerStates[handle] = s;
+ if (auto it = std::find_if(mComposerStates.begin(), mComposerStates.end(),
+ [&handle](const auto& composerState) {
+ return composerState.state.surface == handle;
+ });
+ it != mComposerStates.end()) {
+ return &it->state;
}
- return &(mComposerStates[handle].state);
+ // we don't have it, add an initialized layer_state to our list
+ ComposerState s;
+ s.state.surface = handle;
+ s.state.layerId = sc->getLayerId();
+ mComposerStates.add(s);
+
+ return &mComposerStates.editItemAt(mComposerStates.size() - 1).state;
}
void SurfaceComposerClient::Transaction::registerSurfaceControlForCallback(
@@ -2492,15 +2491,17 @@
// ---------------------------------------------------------------------------
DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp<IBinder>& token) {
+ if (auto it = std::find_if(mDisplayStates.begin(), mDisplayStates.end(),
+ [token](const auto& display) { return display.token == token; });
+ it != mDisplayStates.end()) {
+ return *it;
+ }
+
+ // If display state doesn't exist, add a new one.
DisplayState s;
s.token = token;
- ssize_t index = mDisplayStates.indexOf(s);
- if (index < 0) {
- // we don't have it, add an initialized layer_state to our list
- s.what = 0;
- index = mDisplayStates.add(s);
- }
- return mDisplayStates.editItemAt(static_cast<size_t>(index));
+ mDisplayStates.add(s);
+ return mDisplayStates.editItemAt(mDisplayStates.size() - 1);
}
status_t SurfaceComposerClient::Transaction::setDisplaySurface(const sp<IBinder>& token,