Track transaction merges through transaction trace
Bug: 233371599
Bug: 278663063
Test: dump a transaction trace and ensure transactions have the list of merged transaction ids in the proto
Change-Id: I3edf8f1443a2573ef2086f221ceeef57172ecdbe
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index dfe8c72..b87b36f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4474,7 +4474,8 @@
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp,
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
- const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
+ const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
+ const std::vector<uint64_t>& mergedTransactionIds) {
ATRACE_CALL();
IPCThreadState* ipc = IPCThreadState::self();
@@ -4563,7 +4564,8 @@
listenerCallbacks,
originPid,
originUid,
- transactionId};
+ transactionId,
+ mergedTransactionIds};
if (mTransactionTracing) {
mTransactionTracing->addQueuedTransaction(state);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index d92ec7a..45a82ed 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -508,15 +508,13 @@
}
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
- status_t setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& state, const Vector<DisplayState>& displays,
- uint32_t flags, const sp<IBinder>& applyToken,
- InputWindowCommands inputWindowCommands,
- int64_t desiredPresentTime, bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
- bool hasListenerCallbacks,
- const std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) override;
+ status_t setTransactionState(
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
+ const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
+ InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
+ bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) override;
void bootFinished();
virtual status_t getSupportedFrameTimestamps(std::vector<FrameEvent>* outSupported) const;
sp<IDisplayEventConnection> createDisplayEventConnection(
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 57d927b..0694180 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -69,6 +69,13 @@
for (auto& displayState : t.displays) {
proto.mutable_display_changes()->Add(std::move(toProto(displayState)));
}
+
+ proto.mutable_merged_transaction_ids()->Reserve(
+ static_cast<int32_t>(t.mergedTransactionIds.size()));
+ for (auto& mergedTransactionId : t.mergedTransactionIds) {
+ proto.mutable_merged_transaction_ids()->Add(mergedTransactionId);
+ }
+
return proto;
}
diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h
index 62a7dfd..7132a59 100644
--- a/services/surfaceflinger/TransactionState.h
+++ b/services/surfaceflinger/TransactionState.h
@@ -56,7 +56,8 @@
int64_t desiredPresentTime, bool isAutoTimestamp,
std::vector<uint64_t> uncacheBufferIds, int64_t postTime,
bool hasListenerCallbacks, std::vector<ListenerCallbacks> listenerCallbacks,
- int originPid, int originUid, uint64_t transactionId)
+ int originPid, int originUid, uint64_t transactionId,
+ std::vector<uint64_t> mergedTransactionIds)
: frameTimelineInfo(frameTimelineInfo),
states(std::move(composerStates)),
displays(displayStates),
@@ -71,7 +72,8 @@
listenerCallbacks(listenerCallbacks),
originPid(originPid),
originUid(originUid),
- id(transactionId) {}
+ id(transactionId),
+ mergedTransactionIds(std::move(mergedTransactionIds)) {}
// Invokes `void(const layer_state_t&)` visitor for matching layers.
template <typename Visitor>
@@ -131,6 +133,7 @@
int originUid;
uint64_t id;
bool sentFenceTimeoutWarning = false;
+ std::vector<uint64_t> mergedTransactionIds;
};
} // namespace android
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
index ce4d18f..80943b5 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
@@ -185,11 +185,12 @@
bool hasListenerCallbacks = mFdp.ConsumeBool();
std::vector<ListenerCallbacks> listenerCallbacks{};
uint64_t transactionId = mFdp.ConsumeIntegral<uint64_t>();
+ std::vector<uint64_t> mergedTransactionIds{};
mTestableFlinger.setTransactionState(FrameTimelineInfo{}, states, displays, flags, applyToken,
InputWindowCommands{}, desiredPresentTime, isAutoTimestamp,
- {}, hasListenerCallbacks, listenerCallbacks,
- transactionId);
+ {}, hasListenerCallbacks, listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
void SurfaceFlingerFuzzer::setDisplayStateLocked() {
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index 4d13aca..da5ec48 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -737,19 +737,18 @@
return mFlinger->mTransactionHandler.mPendingTransactionQueues;
}
- 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 std::vector<client_cache_t>& uncacheBuffers,
- 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 std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffers, hasListenerCallbacks,
- listenerCallbacks, transactionId);
+ listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
auto flushTransactionQueues() {
diff --git a/services/surfaceflinger/layerproto/transactions.proto b/services/surfaceflinger/layerproto/transactions.proto
index 2c4eb10..b0cee9b 100644
--- a/services/surfaceflinger/layerproto/transactions.proto
+++ b/services/surfaceflinger/layerproto/transactions.proto
@@ -100,6 +100,7 @@
uint64 transaction_id = 6;
repeated LayerState layer_changes = 7;
repeated DisplayState display_changes = 8;
+ repeated uint64 merged_transaction_ids = 9;
}
// Keep insync with layer_state_t
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index a189c00..8566064 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -466,19 +466,18 @@
return mFlinger->mTransactionHandler.mPendingTransactionCount.load();
}
- 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 std::vector<client_cache_t>& uncacheBuffers,
- 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 std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffers, hasListenerCallbacks,
- listenerCallbacks, transactionId);
+ listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
auto setTransactionStateInternal(TransactionState& transaction) {
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 6a641b3..afb8efb 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -73,6 +73,7 @@
FrameTimelineInfo frameTimelineInfo;
std::vector<client_cache_t> uncacheBuffers;
uint64_t id = static_cast<uint64_t>(-1);
+ std::vector<uint64_t> mergedTransactionIds;
static_assert(0xffffffffffffffff == static_cast<uint64_t>(-1));
};
@@ -108,7 +109,7 @@
transaction.applyToken, transaction.inputWindowCommands,
transaction.desiredPresentTime, transaction.isAutoTimestamp,
transaction.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transaction.id);
+ transaction.id, transaction.mergedTransactionIds);
// If transaction is synchronous, SF applyTransactionState should time out (5s) wating for
// SF to commit the transaction. If this is animation, it should not time out waiting.
@@ -135,7 +136,7 @@
transaction.applyToken, transaction.inputWindowCommands,
transaction.desiredPresentTime, transaction.isAutoTimestamp,
transaction.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transaction.id);
+ transaction.id, transaction.mergedTransactionIds);
nsecs_t returnedTime = systemTime();
EXPECT_LE(returnedTime, applicationSentTime + TRANSACTION_TIMEOUT);
@@ -166,7 +167,7 @@
transactionA.applyToken, transactionA.inputWindowCommands,
transactionA.desiredPresentTime, transactionA.isAutoTimestamp,
transactionA.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transactionA.id);
+ transactionA.id, transactionA.mergedTransactionIds);
// This thread should not have been blocked by the above transaction
// (5s is the timeout period that applyTransactionState waits for SF to
@@ -181,7 +182,7 @@
transactionB.applyToken, transactionB.inputWindowCommands,
transactionB.desiredPresentTime, transactionB.isAutoTimestamp,
transactionB.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transactionB.id);
+ transactionB.id, transactionB.mergedTransactionIds);
// this thread should have been blocked by the above transaction
// if this is an animation, this thread should be blocked for 5s
@@ -218,7 +219,8 @@
transactionA.displays, transactionA.flags, transactionA.applyToken,
transactionA.inputWindowCommands, transactionA.desiredPresentTime,
transactionA.isAutoTimestamp, transactionA.uncacheBuffers,
- mHasListenerCallbacks, mCallbacks, transactionA.id);
+ mHasListenerCallbacks, mCallbacks, transactionA.id,
+ transactionA.mergedTransactionIds);
auto& transactionQueue = mFlinger.getTransactionQueue();
ASSERT_FALSE(transactionQueue.isEmpty());
@@ -238,7 +240,8 @@
transactionA.displays, transactionA.flags, transactionA.applyToken,
transactionA.inputWindowCommands, transactionA.desiredPresentTime,
transactionA.isAutoTimestamp, transactionA.uncacheBuffers,
- mHasListenerCallbacks, mCallbacks, transactionA.id);
+ mHasListenerCallbacks, mCallbacks, transactionA.id,
+ transactionA.mergedTransactionIds);
auto& transactionQueue = mFlinger.getTransactionQueue();
ASSERT_FALSE(transactionQueue.isEmpty());
@@ -251,7 +254,8 @@
mFlinger.setTransactionState(empty.frameTimelineInfo, empty.states, empty.displays, empty.flags,
empty.applyToken, empty.inputWindowCommands,
empty.desiredPresentTime, empty.isAutoTimestamp,
- empty.uncacheBuffers, mHasListenerCallbacks, mCallbacks, empty.id);
+ empty.uncacheBuffers, mHasListenerCallbacks, mCallbacks, empty.id,
+ empty.mergedTransactionIds);
// flush transaction queue should flush as desiredPresentTime has
// passed
@@ -388,7 +392,8 @@
transaction.desiredPresentTime,
transaction.isAutoTimestamp, {}, systemTime(),
mHasListenerCallbacks, mCallbacks, getpid(),
- static_cast<int>(getuid()), transaction.id);
+ static_cast<int>(getuid()), transaction.id,
+ transaction.mergedTransactionIds);
mFlinger.setTransactionStateInternal(transactionState);
}
mFlinger.flushTransactionQueues();
@@ -1083,4 +1088,137 @@
EXPECT_EQ(transactionsReadyToBeApplied.front().id, 42u);
}
+TEST(TransactionHandlerTest, TransactionsKeepTrackOfDirectMerges) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3, transaction4;
+
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+ EXPECT_NE(transaction2Id, transaction3Id);
+
+ transaction1.merge(std::move(transaction2));
+ transaction1.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 2u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction3Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[1], transaction2Id);
+}
+
+TEST(TransactionHandlerTest, TransactionsKeepTrackOfIndirectMerges) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3, transaction4;
+
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+ uint64_t transaction4Id = transaction4.getId();
+ EXPECT_NE(transaction2Id, transaction3Id);
+ EXPECT_NE(transaction2Id, transaction4Id);
+ EXPECT_NE(transaction3Id, transaction4Id);
+
+ transaction4.merge(std::move(transaction2));
+ transaction4.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction4.getMergedTransactionIds().size(), 2u);
+ EXPECT_EQ(transaction4.getMergedTransactionIds()[0], transaction3Id);
+ EXPECT_EQ(transaction4.getMergedTransactionIds()[1], transaction2Id);
+
+ transaction1.merge(std::move(transaction4));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 3u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction4Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[1], transaction3Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[2], transaction2Id);
+}
+
+TEST(TransactionHandlerTest, TransactionMergesAreCleared) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3;
+
+ transaction1.merge(std::move(transaction2));
+ transaction1.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 2u);
+
+ transaction1.clear();
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().empty(), true);
+}
+
+TEST(TransactionHandlerTest, TransactionMergesAreCapped) {
+ SurfaceComposerClient::Transaction transaction;
+ std::vector<uint64_t> mergedTransactionIds;
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds.push_back(transactionToMerge.getId());
+ transaction.merge(std::move(transactionToMerge));
+ }
+
+ // Keeps latest 10 merges in order of merge recency
+ EXPECT_EQ(transaction.getMergedTransactionIds().size(), 10u);
+ for (uint i = 0; i < 10u; i++) {
+ EXPECT_EQ(transaction.getMergedTransactionIds()[i],
+ mergedTransactionIds[mergedTransactionIds.size() - 1 - i]);
+ }
+}
+
+TEST(TransactionHandlerTest, KeepsMergesFromMoreRecentMerge) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3;
+ std::vector<uint64_t> mergedTransactionIds1, mergedTransactionIds2, mergedTransactionIds3;
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds1.push_back(transactionToMerge.getId());
+ transaction1.merge(std::move(transactionToMerge));
+ }
+
+ for (uint i = 0; i < 5u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds2.push_back(transactionToMerge.getId());
+ transaction2.merge(std::move(transactionToMerge));
+ }
+
+ transaction1.merge(std::move(transaction2));
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction2Id);
+ for (uint i = 0; i < 5u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 1u],
+ mergedTransactionIds2[mergedTransactionIds2.size() - 1 - i]);
+ }
+ for (uint i = 0; i < 4u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 6u],
+ mergedTransactionIds1[mergedTransactionIds1.size() - 1 - i]);
+ }
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds3.push_back(transactionToMerge.getId());
+ transaction3.merge(std::move(transactionToMerge));
+ }
+
+ transaction1.merge(std::move(transaction3));
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction3Id);
+ for (uint i = 0; i < 9u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 1],
+ mergedTransactionIds3[mergedTransactionIds3.size() - 1 - i]);
+ }
+}
+
+TEST(TransactionHandlerTest, CanAddTransactionWithFullMergedIds) {
+ SurfaceComposerClient::Transaction transaction1, transaction2;
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ transaction1.merge(std::move(transactionToMerge));
+ }
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+
+ auto transaction1Id = transaction1.getId();
+ transaction2.merge(std::move(transaction1));
+ EXPECT_EQ(transaction2.getMergedTransactionIds().size(), 10u);
+ auto mergedTransactionIds = transaction2.getMergedTransactionIds();
+ EXPECT_TRUE(std::count(mergedTransactionIds.begin(), mergedTransactionIds.end(),
+ transaction1Id) > 0);
+}
+
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
index 82aac7e..92411a7 100644
--- a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
@@ -67,8 +67,14 @@
ASSERT_EQ(actualProto.transactions().size(),
static_cast<int32_t>(expectedTransactions.size()));
for (uint32_t i = 0; i < expectedTransactions.size(); i++) {
- EXPECT_EQ(actualProto.transactions(static_cast<int32_t>(i)).pid(),
- expectedTransactions[i].originPid);
+ const auto expectedTransaction = expectedTransactions[i];
+ const auto protoTransaction = actualProto.transactions(static_cast<int32_t>(i));
+ EXPECT_EQ(protoTransaction.transaction_id(), expectedTransaction.id);
+ EXPECT_EQ(protoTransaction.pid(), expectedTransaction.originPid);
+ for (uint32_t i = 0; i < expectedTransaction.mergedTransactionIds.size(); i++) {
+ EXPECT_EQ(protoTransaction.merged_transaction_ids(static_cast<int32_t>(i)),
+ expectedTransaction.mergedTransactionIds[i]);
+ }
}
}
@@ -92,6 +98,7 @@
TransactionState transaction;
transaction.id = i;
transaction.originPid = static_cast<int32_t>(i);
+ transaction.mergedTransactionIds = std::vector<uint64_t>{i + 100, i + 102};
transactions.emplace_back(transaction);
mTracing.addQueuedTransaction(transaction);
}