SF: Look up buffer caches in binder thread
Avoid locking inside the main thread and contention
with binder thread (via client token binder died).
Test: presubmit
Bug: 238781169
Change-Id: I8a440e9fe3e6f41761d90196ec6128d756735eee
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index cfebec7..849fd9c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -104,6 +104,7 @@
#include <optional>
#include <type_traits>
#include <unordered_map>
+#include <vector>
#include <ui/DisplayIdentification.h>
#include "BackgroundExecutor.h"
@@ -3878,7 +3879,7 @@
}
status_t SurfaceFlinger::setTransactionState(
- const FrameTimelineInfo& frameTimelineInfo, const Vector<ComposerState>& states,
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
bool isAutoTimestamp, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
@@ -3910,7 +3911,24 @@
IPCThreadState* ipc = IPCThreadState::self();
const int originPid = ipc->getCallingPid();
const int originUid = ipc->getCallingUid();
- TransactionState state{frameTimelineInfo, states,
+
+ std::vector<ResolvedComposerState> resolvedStates;
+ resolvedStates.reserve(states.size());
+ for (auto& state : states) {
+ resolvedStates.emplace_back(std::move(state));
+ auto& resolvedState = resolvedStates.back();
+ if (resolvedState.state.hasBufferChanges() && resolvedState.state.hasValidBuffer() &&
+ resolvedState.state.surface) {
+ resolvedState.externalTexture =
+ getExternalTextureFromBufferData(*resolvedState.state.bufferData,
+ std::to_string(resolvedState.state.layerId)
+ .c_str(),
+ transactionId);
+ mBufferCountTracker.increment(resolvedState.state.surface->localBinder());
+ }
+ }
+
+ TransactionState state{frameTimelineInfo, resolvedStates,
displays, flags,
applyToken, inputWindowCommands,
desiredPresentTime, isAutoTimestamp,
@@ -3919,11 +3937,6 @@
listenerCallbacks, originPid,
originUid, transactionId};
- // Check for incoming buffer updates and increment the pending buffer count.
- state.traverseStatesWithBuffers([&](const layer_state_t& state) {
- mBufferCountTracker.increment(state.surface->localBinder());
- });
-
if (mTransactionTracing) {
mTransactionTracing->addQueuedTransaction(state);
}
@@ -3942,7 +3955,7 @@
}
bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& states,
+ std::vector<ResolvedComposerState>& states,
const Vector<DisplayState>& displays, uint32_t flags,
const InputWindowCommands& inputWindowCommands,
const int64_t desiredPresentTime, bool isAutoTimestamp,
@@ -3964,13 +3977,12 @@
}
uint32_t clientStateFlags = 0;
- for (int i = 0; i < states.size(); i++) {
- ComposerState& state = states.editItemAt(i);
+ for (auto& resolvedState : states) {
clientStateFlags |=
- setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp,
- postTime, permissions, transactionId);
- if ((flags & eAnimation) && state.state.surface) {
- if (const auto layer = LayerHandle::getLayer(state.state.surface)) {
+ setClientStateLocked(frameTimelineInfo, resolvedState, desiredPresentTime,
+ isAutoTimestamp, postTime, permissions, transactionId);
+ if ((flags & eAnimation) && resolvedState.state.surface) {
+ if (const auto layer = LayerHandle::getLayer(resolvedState.state.surface)) {
using LayerUpdateType = scheduler::LayerHistory::LayerUpdateType;
mScheduler->recordLayerHistory(layer.get(),
isAutoTimestamp ? 0 : desiredPresentTime,
@@ -4082,7 +4094,7 @@
}
uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTimelineInfo,
- ComposerState& composerState,
+ ResolvedComposerState& composerState,
int64_t desiredPresentTime, bool isAutoTimestamp,
int64_t postTime, uint32_t permissions,
uint64_t transactionId) {
@@ -4377,11 +4389,9 @@
}
if (what & layer_state_t::eBufferChanged) {
- std::shared_ptr<renderengine::ExternalTexture> buffer =
- getExternalTextureFromBufferData(*s.bufferData, layer->getDebugName(),
- transactionId);
- if (layer->setBuffer(buffer, *s.bufferData, postTime, desiredPresentTime, isAutoTimestamp,
- dequeueBufferTimestamp, frameTimelineInfo)) {
+ if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime,
+ desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp,
+ frameTimelineInfo)) {
flags |= eTraversalNeeded;
}
} else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
@@ -4589,7 +4599,7 @@
LOG_ALWAYS_FATAL_IF(token == nullptr);
// reset screen orientation and use primary layer stack
- Vector<ComposerState> state;
+ std::vector<ResolvedComposerState> state;
Vector<DisplayState> displays;
DisplayState d;
d.what = DisplayState::eDisplayProjectionChanged |
@@ -6974,9 +6984,12 @@
}
if (result.error() == ClientCache::AddError::CacheFull) {
- mTransactionHandler
- .onTransactionQueueStalled(transactionId, bufferData.releaseBufferListener,
- "Buffer processing hung due to full buffer cache");
+ ALOGE("Attempted to create an ExternalTexture for layer %s but CacheFull", layerName);
+
+ if (bufferData.releaseBufferListener) {
+ bufferData.releaseBufferListener->onTransactionQueueStalled(
+ String8("Buffer processing hung due to full buffer cache"));
+ }
}
return nullptr;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 85c194b..e09d2b5 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -490,9 +490,8 @@
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
status_t setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- const Vector<ComposerState>& state,
- const Vector<DisplayState>& displays, uint32_t flags,
- const sp<IBinder>& applyToken,
+ Vector<ComposerState>& state, const Vector<DisplayState>& displays,
+ uint32_t flags, const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands,
int64_t desiredPresentTime, bool isAutoTimestamp,
const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
@@ -702,7 +701,8 @@
/*
* Transactions
*/
- bool applyTransactionState(const FrameTimelineInfo& info, Vector<ComposerState>& state,
+ bool applyTransactionState(const FrameTimelineInfo& info,
+ std::vector<ResolvedComposerState>& state,
const Vector<DisplayState>& displays, uint32_t flags,
const InputWindowCommands& inputWindowCommands,
const int64_t desiredPresentTime, bool isAutoTimestamp,
@@ -723,7 +723,7 @@
const TransactionHandler::TransactionFlushState& flushState)
REQUIRES(kMainThreadContext);
- uint32_t setClientStateLocked(const FrameTimelineInfo&, ComposerState&,
+ uint32_t setClientStateLocked(const FrameTimelineInfo&, ResolvedComposerState&,
int64_t desiredPresentTime, bool isAutoTimestamp,
int64_t postTime, uint32_t permissions, uint64_t transactionId)
REQUIRES(mStateLock);
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 3418c82..2f46487 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -310,10 +310,10 @@
int32_t layerCount = proto.layer_changes_size();
t.states.reserve(static_cast<size_t>(layerCount));
for (int i = 0; i < layerCount; i++) {
- ComposerState s;
+ ResolvedComposerState s;
s.state.what = 0;
fromProto(proto.layer_changes(i), s.state);
- t.states.add(s);
+ t.states.emplace_back(s);
}
int32_t displayCount = proto.display_changes_size();
diff --git a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
index 25fdd26..f1a6c0e 100644
--- a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
+++ b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
@@ -240,13 +240,7 @@
for (int j = 0; j < entry.transactions_size(); j++) {
// apply transactions
TransactionState transaction = parser.fromProto(entry.transactions(j));
- mFlinger.setTransactionState(transaction.frameTimelineInfo, transaction.states,
- transaction.displays, transaction.flags,
- transaction.applyToken, transaction.inputWindowCommands,
- transaction.desiredPresentTime,
- transaction.isAutoTimestamp, {},
- transaction.hasListenerCallbacks,
- transaction.listenerCallbacks, transaction.id);
+ mFlinger.setTransactionStateInternal(transaction);
}
const auto frameTime = TimePoint::fromNs(entry.elapsed_realtime_nanos());
diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h
index 3cbfe81..f1ef31d 100644
--- a/services/surfaceflinger/TransactionState.h
+++ b/services/surfaceflinger/TransactionState.h
@@ -20,17 +20,26 @@
#include <memory>
#include <mutex>
#include <vector>
+#include "renderengine/ExternalTexture.h"
#include <gui/LayerState.h>
#include <system/window.h>
namespace android {
+// Extends the client side composer state by resolving buffer cache ids.
+class ResolvedComposerState : public ComposerState {
+public:
+ ResolvedComposerState() = default;
+ ResolvedComposerState(ComposerState&& source) { state = std::move(source.state); }
+ std::shared_ptr<renderengine::ExternalTexture> externalTexture;
+};
+
struct TransactionState {
TransactionState() = default;
TransactionState(const FrameTimelineInfo& frameTimelineInfo,
- const Vector<ComposerState>& composerStates,
+ std::vector<ResolvedComposerState>& composerStates,
const Vector<DisplayState>& displayStates, uint32_t transactionFlags,
const sp<IBinder>& applyToken, const InputWindowCommands& inputWindowCommands,
int64_t desiredPresentTime, bool isAutoTimestamp,
@@ -38,7 +47,7 @@
bool hasListenerCallbacks, std::vector<ListenerCallbacks> listenerCallbacks,
int originPid, int originUid, uint64_t transactionId)
: frameTimelineInfo(frameTimelineInfo),
- states(composerStates),
+ states(std::move(composerStates)),
displays(displayStates),
flags(transactionFlags),
applyToken(applyToken),
@@ -57,18 +66,20 @@
// Invokes `void(const layer_state_t&)` visitor for matching layers.
template <typename Visitor>
void traverseStatesWithBuffers(Visitor&& visitor) const {
- for (const auto& [state] : states) {
- if (state.hasBufferChanges() && state.hasValidBuffer() && state.surface) {
- visitor(state);
+ for (const auto& state : states) {
+ if (state.state.hasBufferChanges() && state.state.hasValidBuffer() &&
+ state.state.surface) {
+ visitor(state.state);
}
}
}
template <typename Visitor>
void traverseStatesWithBuffersWhileTrue(Visitor&& visitor) const {
- for (const auto& [state] : states) {
- if (state.hasBufferChanges() && state.hasValidBuffer() && state.surface) {
- if (!visitor(state)) return;
+ for (const auto& state : states) {
+ if (state.state.hasBufferChanges() && state.state.hasValidBuffer() &&
+ state.state.surface) {
+ if (!visitor(state.state)) return;
}
}
}
@@ -79,8 +90,8 @@
bool isFrameActive() const {
if (!displays.empty()) return true;
- for (const auto& [state] : states) {
- if (state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) {
+ for (const auto& state : states) {
+ if (state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) {
return true;
}
}
@@ -89,7 +100,7 @@
}
FrameTimelineInfo frameTimelineInfo;
- Vector<ComposerState> states;
+ std::vector<ResolvedComposerState> states;
Vector<DisplayState> displays;
uint32_t flags;
sp<IBinder> applyToken;
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index e555867..cc0b012 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -736,12 +736,14 @@
return mFlinger->mTransactionHandler.mPendingTransactionQueues;
}
- auto setTransactionState(
- const FrameTimelineInfo &frameTimelineInfo, const Vector<ComposerState> &states,
- const Vector<DisplayState> &displays, uint32_t flags, const sp<IBinder> &applyToken,
- const InputWindowCommands &inputWindowCommands, int64_t desiredPresentTime,
- bool isAutoTimestamp, const client_cache_t &uncacheBuffer, bool hasListenerCallbacks,
- std::vector<ListenerCallbacks> &listenerCallbacks, uint64_t transactionId) {
+ auto setTransactionState(const FrameTimelineInfo &frameTimelineInfo,
+ Vector<ComposerState> &states, const Vector<DisplayState> &displays,
+ uint32_t flags, const sp<IBinder> &applyToken,
+ const InputWindowCommands &inputWindowCommands,
+ int64_t desiredPresentTime, bool isAutoTimestamp,
+ const client_cache_t &uncacheBuffer, bool hasListenerCallbacks,
+ std::vector<ListenerCallbacks> &listenerCallbacks,
+ uint64_t transactionId) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffer, hasListenerCallbacks,
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 7f471bc..935d953 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -427,18 +427,24 @@
return mFlinger->mTransactionHandler.mPendingTransactionCount.load();
}
- auto setTransactionState(
- const FrameTimelineInfo& frameTimelineInfo, const Vector<ComposerState>& states,
- const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
- const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
- bool isAutoTimestamp, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
- std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
+ auto setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
+ Vector<ComposerState>& states, const Vector<DisplayState>& displays,
+ uint32_t flags, const sp<IBinder>& applyToken,
+ const InputWindowCommands& inputWindowCommands,
+ int64_t desiredPresentTime, bool isAutoTimestamp,
+ const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
+ std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffer, hasListenerCallbacks,
listenerCallbacks, transactionId);
}
+ auto setTransactionStateInternal(TransactionState& transaction) {
+ return mFlinger->mTransactionHandler.queueTransaction(std::move(transaction));
+ }
+
auto flushTransactionQueues() {
return FTL_FAKE_GUARD(kMainThreadContext, mFlinger->flushTransactionQueues(kVsyncId));
}
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 9888f00..488d4a9 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -32,6 +32,7 @@
#include "FrontEnd/TransactionHandler.h"
#include "TestableSurfaceFlinger.h"
+#include "TransactionState.h"
#include "mock/MockEventThread.h"
#include "mock/MockVsyncController.h"
@@ -359,13 +360,23 @@
EXPECT_TRUE(mFlinger.getTransactionQueue().isEmpty());
EXPECT_EQ(0u, mFlinger.getPendingTransactionQueue().size());
- for (const auto& transaction : transactions) {
- mFlinger.setTransactionState(transaction.frameTimelineInfo, transaction.states,
- transaction.displays, transaction.flags,
- transaction.applyToken, transaction.inputWindowCommands,
- transaction.desiredPresentTime,
- transaction.isAutoTimestamp, transaction.uncacheBuffer,
- mHasListenerCallbacks, mCallbacks, transaction.id);
+ for (auto transaction : transactions) {
+ std::vector<ResolvedComposerState> resolvedStates;
+ resolvedStates.reserve(transaction.states.size());
+ for (auto& state : transaction.states) {
+ resolvedStates.emplace_back(std::move(state));
+ }
+
+ TransactionState transactionState(transaction.frameTimelineInfo, resolvedStates,
+ transaction.displays, transaction.flags,
+ transaction.applyToken,
+ transaction.inputWindowCommands,
+ transaction.desiredPresentTime,
+ transaction.isAutoTimestamp,
+ transaction.uncacheBuffer, systemTime(), 0,
+ mHasListenerCallbacks, mCallbacks, getpid(),
+ static_cast<int>(getuid()), transaction.id);
+ mFlinger.setTransactionStateInternal(transactionState);
}
mFlinger.flushTransactionQueues();
EXPECT_TRUE(mFlinger.getTransactionQueue().isEmpty());
diff --git a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
index 14e1aac..b6427c0 100644
--- a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
@@ -46,14 +46,14 @@
size_t layerCount = 2;
t1.states.reserve(layerCount);
for (uint32_t i = 0; i < layerCount; i++) {
- ComposerState s;
+ ResolvedComposerState s;
if (i == 1) {
layer.parentSurfaceControlForChild =
sp<SurfaceControl>::make(SurfaceComposerClient::getDefault(), layerHandle, 42,
"#42");
}
s.state = layer;
- t1.states.add(s);
+ t1.states.emplace_back(s);
}
size_t displayCount = 2;
diff --git a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
index 2dbcfbd..482c3a8 100644
--- a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
@@ -112,16 +112,16 @@
{
TransactionState transaction;
transaction.id = 50;
- ComposerState layerState;
+ ResolvedComposerState layerState;
layerState.state.surface = fakeLayerHandle;
layerState.state.what = layer_state_t::eLayerChanged;
layerState.state.z = 42;
- transaction.states.add(layerState);
- ComposerState childState;
+ transaction.states.emplace_back(layerState);
+ ResolvedComposerState childState;
childState.state.surface = fakeChildLayerHandle;
childState.state.what = layer_state_t::eLayerChanged;
childState.state.z = 43;
- transaction.states.add(childState);
+ transaction.states.emplace_back(childState);
mTracing.addQueuedTransaction(transaction);
std::vector<TransactionState> transactions;
@@ -138,12 +138,12 @@
{
TransactionState transaction;
transaction.id = 51;
- ComposerState layerState;
+ ResolvedComposerState layerState;
layerState.state.surface = fakeLayerHandle;
layerState.state.what = layer_state_t::eLayerChanged | layer_state_t::ePositionChanged;
layerState.state.z = 41;
layerState.state.x = 22;
- transaction.states.add(layerState);
+ transaction.states.emplace_back(layerState);
mTracing.addQueuedTransaction(transaction);
std::vector<TransactionState> transactions;
@@ -247,16 +247,16 @@
{
TransactionState transaction;
transaction.id = 50;
- ComposerState layerState;
+ ResolvedComposerState layerState;
layerState.state.surface = fakeLayerHandle;
layerState.state.what = layer_state_t::eLayerChanged;
layerState.state.z = 42;
- transaction.states.add(layerState);
- ComposerState mirrorState;
+ transaction.states.emplace_back(layerState);
+ ResolvedComposerState mirrorState;
mirrorState.state.surface = fakeMirrorLayerHandle;
mirrorState.state.what = layer_state_t::eLayerChanged;
mirrorState.state.z = 43;
- transaction.states.add(mirrorState);
+ transaction.states.emplace_back(mirrorState);
mTracing.addQueuedTransaction(transaction);
std::vector<TransactionState> transactions;