blast: in order no-op transaction callbacks
Transactions callbacks were being sent as soon as they were ready
instead of in order. To fix this, keep a deque of Transaction
callbacks and do not send a callback until all the callbacks
before it have been sent.
Bug: 128519264
Test: Transaction_test
Change-Id: Ia363b3aca85bc1cd71d0fd915de79b44f786e09f
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index ce88d7b..74cd4f1 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -76,7 +76,11 @@
}
status_t TransactionStats::writeToParcel(Parcel* output) const {
- status_t err = output->writeInt64(latchTime);
+ status_t err = output->writeInt64Vector(callbackIds);
+ if (err != NO_ERROR) {
+ return err;
+ }
+ err = output->writeInt64(latchTime);
if (err != NO_ERROR) {
return err;
}
@@ -96,7 +100,11 @@
}
status_t TransactionStats::readFromParcel(const Parcel* input) {
- status_t err = input->readInt64(&latchTime);
+ status_t err = input->readInt64Vector(&callbackIds);
+ if (err != NO_ERROR) {
+ return err;
+ }
+ err = input->readInt64(&latchTime);
if (err != NO_ERROR) {
return err;
}
@@ -120,16 +128,11 @@
if (err != NO_ERROR) {
return err;
}
-
- for (const auto& [callbackIds, stats] : transactionStats) {
+ for (const auto& stats : transactionStats) {
err = output->writeParcelable(stats);
if (err != NO_ERROR) {
return err;
}
- err = output->writeInt64Vector(callbackIds);
- if (err != NO_ERROR) {
- return err;
- }
}
return NO_ERROR;
}
@@ -139,18 +142,11 @@
for (int i = 0; i < transactionStats_size; i++) {
TransactionStats stats;
- std::vector<CallbackId> callbackIds;
-
status_t err = input->readParcelable(&stats);
if (err != NO_ERROR) {
return err;
}
- err = input->readInt64Vector(&callbackIds);
- if (err != NO_ERROR) {
- return err;
- }
-
- transactionStats.emplace(callbackIds, stats);
+ transactionStats.push_back(stats);
}
return NO_ERROR;
}
@@ -159,11 +155,8 @@
const std::unordered_set<CallbackId>& callbackIds) {
ListenerStats listenerStats;
listenerStats.listener = listener;
- TransactionStats transactionStats;
- listenerStats.transactionStats.emplace(std::piecewise_construct,
- std::forward_as_tuple(callbackIds.begin(),
- callbackIds.end()),
- std::forward_as_tuple(transactionStats));
+ listenerStats.transactionStats.emplace_back(callbackIds);
+
return listenerStats;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index c627898..83cf40c 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -203,15 +203,15 @@
* that could possibly exist for the callbacks.
*/
std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;
- for (const auto& [callbackIds, transactionStats] : listenerStats.transactionStats) {
- for (auto callbackId : callbackIds) {
+ for (const auto& transactionStats : listenerStats.transactionStats) {
+ for (auto callbackId : transactionStats.callbackIds) {
auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
surfaceControls.insert(callbackSurfaceControls.begin(), callbackSurfaceControls.end());
}
}
- for (const auto& [callbackIds, transactionStats] : listenerStats.transactionStats) {
- for (auto callbackId : callbackIds) {
+ for (const auto& transactionStats : listenerStats.transactionStats) {
+ for (auto callbackId : transactionStats.callbackIds) {
auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
if (!callbackFunction) {
ALOGE("cannot call null callback function, skipping");
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index 29ab026..774ad46 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -34,18 +34,6 @@
using CallbackId = int64_t;
-struct CallbackIdsHash {
- // CallbackId vectors have several properties that let us get away with this simple hash.
- // 1) CallbackIds are never 0 so if something has gone wrong and our CallbackId vector is
- // empty we can still hash 0.
- // 2) CallbackId vectors for the same listener either are identical or contain none of the
- // same members. It is sufficient to just check the first CallbackId in the vectors. If
- // they match, they are the same. If they do not match, they are not the same.
- std::size_t operator()(const std::vector<CallbackId> callbackIds) const {
- return std::hash<CallbackId>{}((callbackIds.size() == 0) ? 0 : callbackIds.front());
- }
-};
-
class SurfaceStats : public Parcelable {
public:
status_t writeToParcel(Parcel* output) const override;
@@ -65,6 +53,15 @@
status_t writeToParcel(Parcel* output) const override;
status_t readFromParcel(const Parcel* input) override;
+ TransactionStats() = default;
+ TransactionStats(const std::vector<CallbackId>& ids) : callbackIds(ids) {}
+ TransactionStats(const std::unordered_set<CallbackId>& ids)
+ : callbackIds(ids.begin(), ids.end()) {}
+ TransactionStats(const std::vector<CallbackId>& ids, nsecs_t latch, const sp<Fence>& present,
+ const std::vector<SurfaceStats>& surfaces)
+ : callbackIds(ids), latchTime(latch), presentFence(present), surfaceStats(surfaces) {}
+
+ std::vector<CallbackId> callbackIds;
nsecs_t latchTime = -1;
sp<Fence> presentFence = nullptr;
std::vector<SurfaceStats> surfaceStats;
@@ -79,7 +76,7 @@
const std::unordered_set<CallbackId>& callbackIds);
sp<ITransactionCompletedListener> listener;
- std::unordered_map<std::vector<CallbackId>, TransactionStats, CallbackIdsHash> transactionStats;
+ std::vector<TransactionStats> transactionStats;
};
class ITransactionCompletedListener : public IInterface {