Prevent waiting synchronous case if transaction applied first
Currently, transaction would be queued and apply in the main thread,
if a given transaction would wish to apply synchronously. We may wait
it in binder thread until the transaction has been applied, but if
'applyTransactionState' comes first, it would cause redundant waiting.
To prevent this, we use the CountDownLatch in transaction to wait the
synchronous state, it will wait until transactions applied and committed.
Bug: 181480169
Test: libsurfaceflinger_unittest SurfaceFlinger_test libgui_test
Change-Id: I1fe56296810cdeadeae5b37ae9b4bd00d9e689c3
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 593855e..227d523 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3041,9 +3041,8 @@
void SurfaceFlinger::commitTransaction() {
commitTransactionLocked();
- mTransactionPending = false;
+ signalSynchronousTransactions();
mAnimTransactionPending = false;
- mTransactionCV.broadcast();
}
void SurfaceFlinger::commitTransactionLocked() {
@@ -3312,7 +3311,7 @@
auto& [applyToken, transactionQueue] = *it;
while (!transactionQueue.empty()) {
- const auto& transaction = transactionQueue.front();
+ auto& transaction = transactionQueue.front();
if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
transaction.isAutoTimestamp,
transaction.desiredPresentTime,
@@ -3320,7 +3319,7 @@
setTransactionFlags(eTransactionFlushNeeded);
break;
}
- transactions.push_back(transaction);
+ transactions.emplace_back(std::move(transaction));
transactionQueue.pop();
}
@@ -3337,7 +3336,7 @@
// Case 2: push to pending when there exist a pending queue.
// Case 3: others are ready to apply.
while (!mTransactionQueue.empty()) {
- const auto& transaction = mTransactionQueue.front();
+ auto& transaction = mTransactionQueue.front();
bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
mPendingTransactionQueues.end();
if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
@@ -3345,9 +3344,9 @@
transaction.desiredPresentTime,
transaction.states, pendingBuffers) ||
pendingTransactions) {
- mPendingTransactionQueues[transaction.applyToken].push(transaction);
+ mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));
} else {
- transactions.push_back(transaction);
+ transactions.emplace_back(std::move(transaction));
}
mTransactionQueue.pop();
ATRACE_INT("TransactionQueue", mTransactionQueue.size());
@@ -3363,6 +3362,10 @@
transaction.postTime, transaction.permissions,
transaction.hasListenerCallbacks, transaction.listenerCallbacks,
transaction.originPid, transaction.originUid, transaction.id);
+ if (transaction.transactionCommittedSignal) {
+ mTransactionCommittedSignals.emplace_back(
+ std::move(transaction.transactionCommittedSignal));
+ }
}
}
}
@@ -3430,7 +3433,7 @@
return ready;
}
-void SurfaceFlinger::queueTransaction(TransactionState state) {
+void SurfaceFlinger::queueTransaction(TransactionState& state) {
Mutex::Autolock _l(mQueueLock);
// If its TransactionQueue already has a pending TransactionState or if it is pending
@@ -3450,20 +3453,15 @@
}
}
+ // Generate a CountDownLatch pending state if this is a synchronous transaction.
+ if ((state.flags & eSynchronous) || state.inputWindowCommands.syncInputWindows) {
+ state.transactionCommittedSignal = std::make_shared<CountDownLatch>(
+ (state.inputWindowCommands.syncInputWindows ? 2 : 1));
+ }
+
mTransactionQueue.emplace(state);
ATRACE_INT("TransactionQueue", mTransactionQueue.size());
- // TODO(b/159125966): Remove eEarlyWakeup completely as no client should use this flag
- if (state.flags & eEarlyWakeup) {
- ALOGW("eEarlyWakeup is deprecated. Use eExplicitEarlyWakeup[Start|End]");
- }
-
- if (!(state.permissions & Permission::ACCESS_SURFACE_FLINGER) &&
- (state.flags & (eExplicitEarlyWakeupStart | eExplicitEarlyWakeupEnd))) {
- ALOGE("Only WindowManager is allowed to use eExplicitEarlyWakeup[Start|End] flags");
- state.flags &= ~(eExplicitEarlyWakeupStart | eExplicitEarlyWakeupEnd);
- }
-
const auto schedule = [](uint32_t flags) {
if (flags & eEarlyWakeup) return TransactionSchedule::Early;
if (flags & eExplicitEarlyWakeupEnd) return TransactionSchedule::EarlyEnd;
@@ -3474,28 +3472,23 @@
setTransactionFlags(eTransactionFlushNeeded, schedule);
}
-void SurfaceFlinger::waitForSynchronousTransaction(bool synchronous, bool syncInput) {
- Mutex::Autolock _l(mStateLock);
- if (synchronous) {
- mTransactionPending = true;
+void SurfaceFlinger::waitForSynchronousTransaction(
+ const CountDownLatch& transactionCommittedSignal) {
+ // applyTransactionState is called on the main SF thread. While a given process may wish
+ // to wait on synchronous transactions, the main SF thread should apply the transaction and
+ // set the value to notify this after committed.
+ if (!transactionCommittedSignal.wait_until(std::chrono::seconds(5))) {
+ ALOGE("setTransactionState timed out!");
}
- if (syncInput) {
- mPendingSyncInputWindows = true;
- }
+}
- // applyTransactionState can be called by either the main SF thread or by
- // another process through setTransactionState. While a given process may wish
- // to wait on synchronous transactions, the main SF thread should never
- // be blocked. Therefore, we only wait if isMainThread is false.
- while (mTransactionPending || mPendingSyncInputWindows) {
- status_t err = mTransactionCV.waitRelative(mStateLock, s2ns(5));
- if (CC_UNLIKELY(err != NO_ERROR)) {
- // just in case something goes wrong in SF, return to the
- // called after a few seconds.
- ALOGW_IF(err == TIMED_OUT, "setTransactionState timed out!");
- mTransactionPending = false;
- mPendingSyncInputWindows = false;
- break;
+void SurfaceFlinger::signalSynchronousTransactions() {
+ for (auto it = mTransactionCommittedSignals.begin();
+ it != mTransactionCommittedSignals.end();) {
+ if ((*it)->countDown() == 0) {
+ it = mTransactionCommittedSignals.erase(it);
+ } else {
+ it++;
}
}
}
@@ -3524,21 +3517,35 @@
permissions |= Permission::ROTATE_SURFACE_FLINGER;
}
+ // TODO(b/159125966): Remove eEarlyWakeup completely as no client should use this flag
+ if (flags & eEarlyWakeup) {
+ ALOGW("eEarlyWakeup is deprecated. Use eExplicitEarlyWakeup[Start|End]");
+ }
+
+ if (!(permissions & Permission::ACCESS_SURFACE_FLINGER) &&
+ (flags & (eExplicitEarlyWakeupStart | eExplicitEarlyWakeupEnd))) {
+ ALOGE("Only WindowManager is allowed to use eExplicitEarlyWakeup[Start|End] flags");
+ flags &= ~(eExplicitEarlyWakeupStart | eExplicitEarlyWakeupEnd);
+ }
+
const int64_t postTime = systemTime();
IPCThreadState* ipc = IPCThreadState::self();
const int originPid = ipc->getCallingPid();
const int originUid = ipc->getCallingUid();
+ TransactionState state{frameTimelineInfo, states,
+ displays, flags,
+ applyToken, inputWindowCommands,
+ desiredPresentTime, isAutoTimestamp,
+ uncacheBuffer, postTime,
+ permissions, hasListenerCallbacks,
+ listenerCallbacks, originPid,
+ originUid, transactionId};
+ queueTransaction(state);
- queueTransaction({frameTimelineInfo, states, displays, flags, applyToken, inputWindowCommands,
- desiredPresentTime, isAutoTimestamp, uncacheBuffer, postTime, permissions,
- hasListenerCallbacks, listenerCallbacks, originPid, originUid,
- transactionId});
-
- const bool synchronous = flags & eSynchronous;
- const bool syncInput = inputWindowCommands.syncInputWindows;
- if (synchronous || syncInput) {
- waitForSynchronousTransaction(synchronous, syncInput);
+ // Check the pending state to make sure the transaction is synchronous.
+ if (state.transactionCommittedSignal) {
+ waitForSynchronousTransaction(*state.transactionCommittedSignal);
}
return NO_ERROR;
@@ -6078,10 +6085,7 @@
void SurfaceFlinger::setInputWindowsFinished() {
Mutex::Autolock _l(mStateLock);
-
- mPendingSyncInputWindows = false;
-
- mTransactionCV.broadcast();
+ signalSynchronousTransactions();
}
// ---------------------------------------------------------------------------