From 5a1eab34fcfd087b4cf6d113c14f98febc69b882 Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Mon, 8 May 2023 14:43:03 +0000 Subject: [PATCH] Only prevent overlapping SSG if a buffer is going to be synced In VRI, SSG are sometimes created as a way to notify WMS that a buffer has been drawn and not always to sync the buffer. In those cases, there's no need to prevent overlapping syncs since the buffers cannot be submitted out of order. Only add the safeguard SSG when there's a buffer getting synced to ensure multiple buffer syncs for the same VRI are submitted in order. Test: SurfaceSyncGroupTests SurfaceSyncGroupTest Bug: 279096620 Bug: 279790175 Change-Id: I77841296297717e413237c183738b9ec9642f1cf --- core/java/android/view/ViewRootImpl.java | 20 +++-- .../java/android/window/SurfaceSyncGroup.java | 79 +++++++++---------- 2 files changed, 52 insertions(+), 47 deletions(-) diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 153bfde07758d..cdc488e80ebb4 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -3772,6 +3772,16 @@ public final class ViewRootImpl implements ViewParent, createSyncIfNeeded(); notifyDrawStarted(isInWMSRequestedSync()); mDrewOnceForSync = true; + + // If the active SSG is also requesting to sync a buffer, the following needs to happen + // 1. Ensure we keep track of the number of active syncs to know when to disable RT + // RT animations that conflict with syncing a buffer. + // 2. Add a safeguard SSG to prevent multiple SSG that sync buffers from being submitted + // out of order. + if (mActiveSurfaceSyncGroup != null && mSyncBuffer) { + updateSyncInProgressCount(mActiveSurfaceSyncGroup); + safeguardOverlappingSyncs(mActiveSurfaceSyncGroup); + } } if (!isViewVisible) { @@ -3836,14 +3846,11 @@ public final class ViewRootImpl implements ViewParent, mWmsRequestSyncGroupState = WMS_SYNC_MERGED; reportDrawFinished(t, seqId); }); - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "create WMS Sync group=" + mWmsRequestSyncGroup.getName()); if (DEBUG_BLAST) { Log.d(mTag, "Setup new sync=" + mWmsRequestSyncGroup.getName()); } mWmsRequestSyncGroup.add(this, null /* runnable */); - Trace.traceEnd(Trace.TRACE_TAG_VIEW); } private void notifyContentCaptureEvents() { @@ -4504,6 +4511,9 @@ public final class ViewRootImpl implements ViewParent, Log.d(mTag, "reportDrawFinished"); } + if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { + Trace.instant(Trace.TRACE_TAG_VIEW, "reportDrawFinished " + mTag + " seqId=" + seqId); + } try { mWindowSession.finishDrawing(mWindow, t, seqId); } catch (RemoteException e) { @@ -11387,7 +11397,7 @@ public final class ViewRootImpl implements ViewParent, * ensure the latter SSG always waits for the former SSG's transaction to get to SF. */ private void safeguardOverlappingSyncs(SurfaceSyncGroup activeSurfaceSyncGroup) { - SurfaceSyncGroup safeguardSsg = new SurfaceSyncGroup("VRI-Safeguard"); + SurfaceSyncGroup safeguardSsg = new SurfaceSyncGroup("Safeguard-" + mTag); // Always disable timeout on the safeguard sync safeguardSsg.toggleTimeout(false /* enable */); synchronized (mPreviousSyncSafeguardLock) { @@ -11446,8 +11456,6 @@ public final class ViewRootImpl implements ViewParent, mHandler.post(runnable); } }); - safeguardOverlappingSyncs(mActiveSurfaceSyncGroup); - updateSyncInProgressCount(mActiveSurfaceSyncGroup); newSyncGroup = true; } diff --git a/core/java/android/window/SurfaceSyncGroup.java b/core/java/android/window/SurfaceSyncGroup.java index 18405676dbd84..eec05b94734b4 100644 --- a/core/java/android/window/SurfaceSyncGroup.java +++ b/core/java/android/window/SurfaceSyncGroup.java @@ -53,7 +53,6 @@ import java.util.function.Supplier; * This will also allow synchronization of surfaces across multiple processes. The caller can add * SurfaceControlViewHosts from another process to the SurfaceSyncGroup in a different process * and this clas will ensure all the surfaces are ready before applying everything together. - *

* see the SurfaceSyncGroup documentation *

*/ @@ -136,6 +135,7 @@ public final class SurfaceSyncGroup { @GuardedBy("mLock") private boolean mTimeoutDisabled; + private final String mTrackName; private static boolean isLocalBinder(IBinder binder) { return !(binder instanceof BinderProxy); @@ -192,6 +192,7 @@ public final class SurfaceSyncGroup { } mName = name + "#" + sCounter.getAndIncrement(); + mTrackName = "SurfaceSyncGroup " + name; mTransactionReadyConsumer = (transaction) -> { if (DEBUG && transaction != null) { @@ -199,9 +200,10 @@ public final class SurfaceSyncGroup { + mName); } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.instant(Trace.TRACE_TAG_VIEW, - "Final TransactionCallback with " + transaction + " for " + mName); + Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, + "Final TransactionCallback with " + transaction); } + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); transactionReadyConsumer.accept(transaction); synchronized (mLock) { // If there's a registered listener with WMS, that means we aren't actually complete @@ -213,7 +215,7 @@ public final class SurfaceSyncGroup { }; if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.instant(Trace.TRACE_TAG_VIEW, "new SurfaceSyncGroup " + mName); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, mName, hashCode()); } if (DEBUG) { @@ -257,7 +259,7 @@ public final class SurfaceSyncGroup { Log.d(TAG, "markSyncReady " + mName); } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, "markSyncReady " + mName); + Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, "markSyncReady"); } synchronized (mLock) { if (mHasWMSync) { @@ -269,9 +271,6 @@ public final class SurfaceSyncGroup { mSyncReady = true; checkIfSyncIsComplete(); } - if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); - } } /** @@ -399,14 +398,14 @@ public final class SurfaceSyncGroup { public boolean add(ISurfaceSyncGroup surfaceSyncGroup, boolean parentSyncGroupMerge, @Nullable Runnable runnable) { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "addToSync token=" + mToken.hashCode() + " parent=" + mName); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, + "addToSync token=" + mToken.hashCode(), hashCode()); } synchronized (mLock) { if (mSyncReady) { Log.w(TAG, "Trying to add to sync when already marked as ready " + mName); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return false; } @@ -419,7 +418,7 @@ public final class SurfaceSyncGroup { if (isLocalBinder(surfaceSyncGroup.asBinder())) { boolean didAddLocalSync = addLocalSync(surfaceSyncGroup, parentSyncGroupMerge); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return didAddLocalSync; } @@ -447,7 +446,7 @@ public final class SurfaceSyncGroup { mSurfaceSyncGroupCompletedListener)) { mSurfaceSyncGroupCompletedListener = null; if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return false; } @@ -459,13 +458,13 @@ public final class SurfaceSyncGroup { surfaceSyncGroup.onAddedToSyncGroup(mToken, parentSyncGroupMerge); } catch (RemoteException e) { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return false; } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return true; } @@ -510,15 +509,15 @@ public final class SurfaceSyncGroup { + ". Setting up Sync in WindowManager."); } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "addSyncToWm=" + token.hashCode() + " group=" + mName); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, + "addSyncToWm=" + token.hashCode(), hashCode()); } AddToSurfaceSyncGroupResult addToSyncGroupResult = new AddToSurfaceSyncGroupResult(); if (!WindowManagerGlobal.getWindowManagerService().addToSurfaceSyncGroup(token, parentSyncGroupMerge, surfaceSyncGroupCompletedListener, addToSyncGroupResult)) { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return false; } @@ -527,12 +526,12 @@ public final class SurfaceSyncGroup { addToSyncGroupResult.mTransactionReadyCallback); } catch (RemoteException e) { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return false; } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return true; } @@ -550,8 +549,8 @@ public final class SurfaceSyncGroup { } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "addLocalSync=" + childSurfaceSyncGroup.mName + " parent=" + mName); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, + "addLocalSync=" + childSurfaceSyncGroup.mName, hashCode()); } ITransactionReadyCallback callback = createTransactionReadyCallback(parentSyncGroupMerge); @@ -562,7 +561,7 @@ public final class SurfaceSyncGroup { childSurfaceSyncGroup.setTransactionCallbackFromParent(mISurfaceSyncGroup, callback); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return true; } @@ -574,9 +573,9 @@ public final class SurfaceSyncGroup { } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, "setTransactionCallbackFromParent " + mName + " callback=" - + transactionReadyCallback.hashCode()); + + transactionReadyCallback.hashCode(), hashCode()); } // Start the timeout when this SurfaceSyncGroup has been added to a parent SurfaceSyncGroup. @@ -617,9 +616,9 @@ public final class SurfaceSyncGroup { mParentSyncGroup = parentSyncGroup; mTransactionReadyConsumer = (transaction) -> { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "transactionReadyCallback " + mName + " callback=" - + transactionReadyCallback.hashCode()); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, + "Invoke transactionReadyCallback=" + + transactionReadyCallback.hashCode(), hashCode()); } lastCallback.accept(null); @@ -629,7 +628,7 @@ public final class SurfaceSyncGroup { transaction.apply(); } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } }; addedToSyncListener = mAddedToSyncListener; @@ -647,7 +646,7 @@ public final class SurfaceSyncGroup { addedToSyncListener.run(); } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } } @@ -669,8 +668,8 @@ public final class SurfaceSyncGroup { } if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.instant(Trace.TRACE_TAG_VIEW, - "checkIfSyncIsComplete " + mName + " mSyncReady=" + mSyncReady + Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, + "checkIfSyncIsComplete mSyncReady=" + mSyncReady + " mPendingSyncs=" + mPendingSyncs.size()); } @@ -726,9 +725,8 @@ public final class SurfaceSyncGroup { } mPendingSyncs.remove(this); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.instant(Trace.TRACE_TAG_VIEW, - "onTransactionReady group=" + mName + " callback=" - + hashCode()); + Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, + "onTransactionReady callback=" + hashCode()); } checkIfSyncIsComplete(); } @@ -743,8 +741,8 @@ public final class SurfaceSyncGroup { } mPendingSyncs.add(transactionReadyCallback); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.instant(Trace.TRACE_TAG_VIEW, - "createTransactionReadyCallback " + mName + " mPendingSyncs=" + Trace.instantForTrack(Trace.TRACE_TAG_VIEW, mTrackName, + "createTransactionReadyCallback mPendingSyncs=" + mPendingSyncs.size() + " transactionReady=" + transactionReadyCallback.hashCode()); } @@ -764,13 +762,12 @@ public final class SurfaceSyncGroup { public boolean onAddedToSyncGroup(IBinder parentSyncGroupToken, boolean parentSyncGroupMerge) { if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceBegin(Trace.TRACE_TAG_VIEW, - "onAddedToSyncGroup token=" + parentSyncGroupToken.hashCode() + " child=" - + mName); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_VIEW, mTrackName, + "onAddedToSyncGroup token=" + parentSyncGroupToken.hashCode(), hashCode()); } boolean didAdd = addSyncToWm(parentSyncGroupToken, parentSyncGroupMerge, null); if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) { - Trace.traceEnd(Trace.TRACE_TAG_VIEW); + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_VIEW, mTrackName, hashCode()); } return didAdd; }