diff options
3 files changed, 143 insertions, 0 deletions
diff --git a/services/core/java/com/android/server/wm/BLASTSync.md b/services/core/java/com/android/server/wm/BLASTSync.md index 2f39d6db385e..dbb28d4d5ae3 100644 --- a/services/core/java/com/android/server/wm/BLASTSync.md +++ b/services/core/java/com/android/server/wm/BLASTSync.md @@ -106,3 +106,88 @@ to the client via ClientTransaction), we haven't even incremented the seqId yet, at the same time as the state? We solve this by pushing all client communication through a handler thread that has to acquire the lock. This ensures we uphold requirement 2. += Transaction ordering = + +Applying transactions from different process, and in to different server side transaction queues +raises various questions about transaction ordering. There are two tricky questions in this +domain that we address here: + 1. The ordering of Transactions from a single BLASTBufferQueue wrt to eachother + 2. The ordering of non synced WM updates to syncable state, wrt a BLASTSyncEngine + transaction + +== Ordering of Transactions in a single BBQ == + +We can see if sync is never involved, there are never any real questions about ordering. +Even using one-way setTransactionState, the calls are from a single thread to a single +interface on another, and will be ordered. When we hand out transactions for sync +is where issues can start to arise. Obviously if we apply another transaction +immediately after handing out the sync transaction it could arrive first, and this would +cause an ordering issue. It's also possible that the sync transaction arrives before the +transaction applied before it (since setTransactionState is one way from BBQ). Even if +the transactions are applied in the right order, it's possible for them to be +commited out of sync, as the BBQ and SyncConsumer may be using different apply tokens +resulting in the transaction being placed in different server side queues. + +To solve these issues, we use a scheme involving "barrier transactions". We show how +this scheme handles every permutation of (Sync, NotSync). + +1. NotSync, NotSync: This is the trivial case. As long as both buffers are submitted from + the same thread, they will arrive in SF in order, and SF will place them in the same + queue (since there is a shared apply token), which it will process in order. +2. Sync, NotSync: For sync transactions we register a commit callback, and require it to + be fired before applying the next transaction. Since the commit callback is only + fired when the transaction has latched on the server side, any transaction applied + later, must occur later. +3. Sync, Sync: Ordering of consecutive sync transactions is delegated to the sync + consumer +4. NotSync, Sync: This is the trickiest case, as we don't want to set a commit callback + on not sync transactions (as this would incur an unacceptable performance overhead). + Further complicating matters, we want to apply them with one-way binder, since + this is the hot path for all graphical updates. To solve this we use a + "setBufferHasBarrier" system. Sync transactions (before they are handed out) + are tagged with a barrier, referring to the frame number of the last + non sync transaction. SurfaceFlinger will ensure the correct ordering + by stalling transactions in the queue until barriers are fulfilled. This barrier + primitive is dangerous, because it could result in deadlocking, but its ok in + this scenario since only BBQ uses its apply token. + +We can see from this that all frames are in order. + +== Ordering of WM updates to syncable state == + +A second interesting question, is about the ordering of WM updates to syncable-state. In +the WM we frequently write code like + getPendingTransaction/getSyncTransaction().show(mSurfaceControl). +In normal operation, getSyncTransaction and getPendingTransaction both refer to the same +transaction which is the displaycontent pendingtransaction, applied by the WindowManager. +During sync, each syncing container will instead use its mSyncTransaction, which will +eventually be merged to be applied by the WindowManager. We can see that we have a congruent +"problem" to BLASTBufferQueue transaction ordering. Namely, earlier state updates which were +given to SysUI could end up being applied later than state updates which were made later but +directly applied by the WM. We can break this down in to two cases: +=== getPendingTransaction() and getSyncTransaction() ordering === +It's possible for code like this to occur: +getSyncTransaction().OP1 +getPendingTransaction().OP2 +applyPendingTransaction +applySyncTransaction + +in this case the change in OP2 was made later, but occurs first. We define this case as +intended behavior, and say there is no ordering guarantee between the pending +and sync transactions. This means the pending transaction is only useful in +some marginal cases and should probably be considered a deprecated primitive. +=== getSyncTransaction() and getSyncTransaction() ordering === +However, we take great care to ensure usage of getSyncTransaction() reflects +the ordering the developer would expect. BLASTSyncEngine will register +a commit callback on all transactions it hands out. In the interval between +receiving this commit callback and sending out the transaction, we may have other +data enter getSyncTransaction. If we returned the pending transaction +(as we do in normal time), then we could create ordering issues, since the pending +transactions ordering is undefined. Instead we continue to return the sync transaction +during this interval. If no second sync has started by the time we receive +the commit callback, then we directly apply this left over data in the sync transaction +guaranteed it will be ordered correctly, and return to using the pending +transaction. If a second sync has started, then we just allow the data +to persist in the mSyncTransaction, potentially to be overwritten +by the new sync. It will eventually apply with SysUI's apply token and +ordering will be maintained. diff --git a/services/core/java/com/android/server/wm/BLASTSyncEngine.java b/services/core/java/com/android/server/wm/BLASTSyncEngine.java index feaa10dd94a0..67eb9c8911a7 100644 --- a/services/core/java/com/android/server/wm/BLASTSyncEngine.java +++ b/services/core/java/com/android/server/wm/BLASTSyncEngine.java @@ -19,6 +19,7 @@ package com.android.server.wm; import static android.os.Trace.TRACE_TAG_WINDOW_MANAGER; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_SYNC_ENGINE; +import static com.android.server.wm.WindowManagerService.H.WINDOW_STATE_BLAST_SYNC_TIMEOUT; import android.annotation.NonNull; import android.os.Trace; @@ -147,6 +148,33 @@ class BLASTSyncEngine { for (WindowContainer wc : mRootMembers) { wc.finishSync(merged, false /* cancel */); } + + final ArraySet<WindowContainer> wcAwaitingCommit = new ArraySet<>(); + for (WindowContainer wc : mRootMembers) { + wc.waitForSyncTransactionCommit(wcAwaitingCommit); + } + final Runnable callback = new Runnable() { + // Can run a second time if the action completes after the timeout. + boolean ran = false; + public void run() { + synchronized (mWm.mGlobalLock) { + if (ran) { + return; + } + mWm.mH.removeCallbacks(this); + ran = true; + SurfaceControl.Transaction t = new SurfaceControl.Transaction(); + for (WindowContainer wc : wcAwaitingCommit) { + wc.onSyncTransactionCommitted(t); + } + t.apply(); + wcAwaitingCommit.clear(); + } + } + }; + merged.addTransactionCommittedListener((r) -> { r.run(); }, callback::run); + mWm.mH.postDelayed(callback, WINDOW_STATE_BLAST_SYNC_TIMEOUT); + Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "onTransactionReady"); mListener.onTransactionReady(mSyncId, merged); Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER); diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 4dbcea1e4751..f5af2b4e29fa 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -236,6 +236,8 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< */ private boolean mCommittedReparentToAnimationLeash; + private int mSyncTransactionCommitCallbackDepth = 0; + /** Interface for {@link #isAnimating} to check which cases for the container is animating. */ public interface AnimationFlags { /** @@ -2688,6 +2690,9 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< * {@link #getPendingTransaction()} */ public Transaction getSyncTransaction() { + if (mSyncTransactionCommitCallbackDepth > 0) { + return mSyncTransaction; + } if (mSyncState != SYNC_STATE_NONE) { return mSyncTransaction; } @@ -3909,4 +3914,29 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< p.updateOverlayInsetsState(originalChange); } } + + void waitForSyncTransactionCommit(ArraySet<WindowContainer> wcAwaitingCommit) { + if (wcAwaitingCommit.contains(this)) { + return; + } + mSyncTransactionCommitCallbackDepth++; + wcAwaitingCommit.add(this); + + for (int i = mChildren.size() - 1; i >= 0; --i) { + mChildren.get(i).waitForSyncTransactionCommit(wcAwaitingCommit); + } + } + + void onSyncTransactionCommitted(SurfaceControl.Transaction t) { + mSyncTransactionCommitCallbackDepth--; + if (mSyncTransactionCommitCallbackDepth > 0) { + return; + } + if (mSyncState != SYNC_STATE_NONE) { + return; + } + + t.merge(mSyncTransaction); + } + } |