From b9876970422a640fd764b83862d615eb5b693d8e Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Thu, 26 May 2022 16:40:33 -0700 Subject: [PATCH] Drop sync buffers for subsequent sync requests When making subsequent draw requests for the same window, drop any existing buffers since they will be replaced by the new one. Sometimes the "old" buffer will still be "in-flight" (either drawing still or in binder). So, also drop incoming buffers for seqIds that are earlier than the latest prepareSync. Additionally, immediately apply any applyWithNextDraw that were requested before the sync, but arrive after the sync started -- otherwise they can get put on pending (which waits for sync apply) and thus also block the buffer queue. Bug: 233625646 Test: run tests and check for ANRs Change-Id: I156a9a73eea8346dc241b5d782a863b99127ca9f --- data/etc/services.core.protolog.json | 24 ++++---- .../server/wm/WindowManagerService.java | 8 +-- .../com/android/server/wm/WindowState.java | 60 ++++++++++++++++--- 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/data/etc/services.core.protolog.json b/data/etc/services.core.protolog.json index f8da95d04411d..6d8020369391e 100644 --- a/data/etc/services.core.protolog.json +++ b/data/etc/services.core.protolog.json @@ -475,12 +475,6 @@ "group": "WM_DEBUG_ADD_REMOVE", "at": "com\/android\/server\/wm\/ResetTargetTaskHelper.java" }, - "-1635750891": { - "message": "Received remote change for Display[%d], applied: [%dx%d, rot = %d]", - "level": "VERBOSE", - "group": "WM_DEBUG_CONFIGURATION", - "at": "com\/android\/server\/wm\/RemoteDisplayChangeController.java" - }, "-1633115609": { "message": "Key dispatch not paused for screen off", "level": "VERBOSE", @@ -1711,6 +1705,12 @@ "group": "WM_DEBUG_STATES", "at": "com\/android\/server\/wm\/RootWindowContainer.java" }, + "-417730399": { + "message": "Preparing to sync a window that was already in the sync, so try dropping buffer. win=%s", + "level": "DEBUG", + "group": "WM_DEBUG_SYNC_ENGINE", + "at": "com\/android\/server\/wm\/WindowState.java" + }, "-415865166": { "message": "findFocusedWindow: Found new focus @ %s", "level": "VERBOSE", @@ -2137,6 +2137,12 @@ "group": "WM_DEBUG_RECENTS_ANIMATIONS", "at": "com\/android\/server\/wm\/RecentsAnimation.java" }, + "-4263657": { + "message": "Got a buffer for request id=%d but latest request is id=%d. Since the buffer is out-of-date, drop it. win=%s", + "level": "DEBUG", + "group": "WM_DEBUG_SYNC_ENGINE", + "at": "com\/android\/server\/wm\/WindowState.java" + }, "3593205": { "message": "commitVisibility: %s: visible=%b mVisibleRequested=%b", "level": "VERBOSE", @@ -3877,12 +3883,6 @@ "group": "WM_DEBUG_ORIENTATION", "at": "com\/android\/server\/wm\/WindowStateAnimator.java" }, - "1764619787": { - "message": "Remote change for Display[%d]: timeout reached", - "level": "VERBOSE", - "group": "WM_DEBUG_CONFIGURATION", - "at": "com\/android\/server\/wm\/RemoteDisplayChangeController.java" - }, "1774661765": { "message": "Devices still not ready after waiting %d milliseconds before attempting to detect safe mode.", "level": "WARN", diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 125d55072a599..92091bc747662 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -122,7 +122,6 @@ import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_ALL; import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION; import static com.android.server.wm.WindowContainer.AnimationFlags.CHILDREN; import static com.android.server.wm.WindowContainer.AnimationFlags.TRANSITION; -import static com.android.server.wm.WindowContainer.SYNC_STATE_NONE; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_DISPLAY; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_INPUT_METHOD; @@ -2252,7 +2251,7 @@ public class WindowManagerService extends IWindowManager.Stub return 0; } - if (win.cancelAndRedraw()) { + if (win.cancelAndRedraw() && win.mPrepareSyncSeqId <= win.mLastSeqIdSentToRelayout) { result |= RELAYOUT_RES_CANCEL_AND_REDRAW; } @@ -2558,11 +2557,6 @@ public class WindowManagerService extends IWindowManager.Stub win.mLastSeqIdSentToRelayout = win.mSyncSeqId; outSyncIdBundle.putInt("seqid", win.mSyncSeqId); - // Only mark mAlreadyRequestedSync if there's an explicit sync request, and not if - // we're syncing due to mDrawHandlers - if (win.mSyncState != SYNC_STATE_NONE) { - win.mAlreadyRequestedSync = true; - } } else { outSyncIdBundle.putInt("seqid", -1); } diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 6728e63d055fd..46091d842c2ad 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -115,6 +115,7 @@ import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_FOCUS_LIGHT; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_ORIENTATION; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_RESIZE; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_STARTING_WINDOW; +import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_SYNC_ENGINE; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_WINDOW_INSETS; import static com.android.server.policy.WindowManagerPolicy.FINISH_LAYOUT_REDO_WALLPAPER; import static com.android.server.policy.WindowManagerPolicy.TRANSIT_ENTER; @@ -391,7 +392,9 @@ class WindowState extends WindowContainer implements WindowManagerP */ int mSyncSeqId = 0; int mLastSeqIdSentToRelayout = 0; - boolean mAlreadyRequestedSync; + + /** The last syncId associated with a prepareSync or 0 when no sync is active. */ + int mPrepareSyncSeqId = 0; /** * {@code true} when the client was still drawing for sync when the sync-set was finished or @@ -4421,7 +4424,7 @@ class WindowState extends WindowContainer implements WindowManagerP } } - pw.println(prefix + "mAlreadyRequestedSync=" + mAlreadyRequestedSync); + pw.println(prefix + "mPrepareSyncSeqId=" + mPrepareSyncSeqId); } @Override @@ -5913,6 +5916,13 @@ class WindowState extends WindowContainer implements WindowManagerP return mWinAnimator.getSurfaceControl(); } + /** Drops a buffer for this window's view-root from a transaction */ + private void dropBufferFrom(Transaction t) { + SurfaceControl viewSurface = getClientViewRootSurface(); + if (viewSurface == null) return; + t.setBuffer(viewSurface, (android.hardware.HardwareBuffer) null); + } + @Override boolean prepareSync() { if (!mDrawHandlers.isEmpty()) { @@ -5928,7 +5938,18 @@ class WindowState extends WindowContainer implements WindowManagerP // to draw even if the children draw first or don't need to sync, so we start // in WAITING state rather than READY. mSyncState = SYNC_STATE_WAITING_FOR_DRAW; + + if (mPrepareSyncSeqId > 0) { + // another prepareSync during existing sync (eg. reparented), so pre-emptively + // drop buffer (if exists). If the buffer hasn't been received yet, it will be + // dropped in finishDrawing. + ProtoLog.d(WM_DEBUG_SYNC_ENGINE, "Preparing to sync a window that was already in the" + + " sync, so try dropping buffer. win=%s", this); + dropBufferFrom(mSyncTransaction); + } + mSyncSeqId++; + mPrepareSyncSeqId = mSyncSeqId; requestRedrawForSync(); return true; } @@ -5949,7 +5970,13 @@ class WindowState extends WindowContainer implements WindowManagerP if (mSyncState == SYNC_STATE_WAITING_FOR_DRAW && mRedrawForSyncReported) { mClientWasDrawingForSync = true; } - mAlreadyRequestedSync = false; + mPrepareSyncSeqId = 0; + if (cancel) { + // This is leaving sync so any buffers left in the sync have a chance of + // being applied out-of-order and can also block the buffer queue for this + // window. To prevent this, drop the buffer. + dropBufferFrom(mSyncTransaction); + } super.finishSync(outMergedTransaction, cancel); } @@ -5971,6 +5998,17 @@ class WindowState extends WindowContainer implements WindowManagerP .notifyStartingWindowDrawn(mActivityRecord); } + final boolean syncActive = mPrepareSyncSeqId > 0; + final boolean syncStillPending = syncActive && mPrepareSyncSeqId > syncSeqId; + if (syncStillPending && postDrawTransaction != null) { + ProtoLog.d(WM_DEBUG_SYNC_ENGINE, "Got a buffer for request id=%d but latest request is" + + " id=%d. Since the buffer is out-of-date, drop it. win=%s", syncSeqId, + mPrepareSyncSeqId, this); + // sync is waiting for a newer seqId, so this buffer is obsolete and can be dropped + // to free up the buffer queue. + dropBufferFrom(postDrawTransaction); + } + final boolean hasSyncHandlers = executeDrawHandlers(postDrawTransaction, syncSeqId); boolean skipLayout = false; @@ -5983,10 +6021,15 @@ class WindowState extends WindowContainer implements WindowManagerP // Layout is not needed because the window will be hidden by the fade leash. postDrawTransaction = null; skipLayout = true; - } else if (onSyncFinishedDrawing() && postDrawTransaction != null) { - mSyncTransaction.merge(postDrawTransaction); - // Consume the transaction because the sync group will merge it. - postDrawTransaction = null; + } else if (syncActive) { + if (!syncStillPending) { + onSyncFinishedDrawing(); + } + if (postDrawTransaction != null) { + mSyncTransaction.merge(postDrawTransaction); + // Consume the transaction because the sync group will merge it. + postDrawTransaction = null; + } } final boolean layoutNeeded = @@ -6218,6 +6261,7 @@ class WindowState extends WindowContainer implements WindowManagerP } public boolean cancelAndRedraw() { - return mSyncState != SYNC_STATE_NONE && mAlreadyRequestedSync; + // Cancel any draw requests during a sync. + return mPrepareSyncSeqId > 0; } }