From ccfbcb81c633aeb3d1dae26d51d95d47458ab61e Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Fri, 13 Mar 2020 21:41:58 +0800 Subject: [PATCH] Clean unnecessary lock of WindowAnimator#animate Reduce the invocation of thread priority booster and monitor-enter/exit. - The prohibition of lock was introduced in commits 9b19fd4 and 836dac4, but since commit 32fd84a that enabled lock free app animations on surface animation thread, the original WindowAnimator#animate is safe to be put in WM global lock. - All invocations of open/closeSurfaceTransaction are inside WM global lock. Bug: 139522754 Test: atest WindowTest Change-Id: Idb11b4e5bc3b982341d86cad33f6c23253d0d0d9 --- .../com/android/server/wm/WindowAnimator.java | 160 ++++++++---------- .../server/wm/WindowManagerService.java | 10 +- 2 files changed, 75 insertions(+), 95 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowAnimator.java b/services/core/java/com/android/server/wm/WindowAnimator.java index a60db94ad1e36..0b11dd28e5953 100644 --- a/services/core/java/com/android/server/wm/WindowAnimator.java +++ b/services/core/java/com/android/server/wm/WindowAnimator.java @@ -96,8 +96,8 @@ public class WindowAnimator { mAnimationFrameCallback = frameTimeNs -> { synchronized (mService.mGlobalLock) { mAnimationFrameCallbackScheduled = false; + animate(frameTimeNs); } - animate(frameTimeNs); }; } @@ -115,110 +115,94 @@ public class WindowAnimator { mInitialized = true; } - /** - * DO NOT HOLD THE WINDOW MANAGER LOCK WHILE CALLING THIS METHOD. Reason: the method closes - * an animation transaction, that might be blocking until the next sf-vsync, so we want to make - * sure other threads can make progress if this happens. - */ private void animate(long frameTimeNs) { - - synchronized (mService.mGlobalLock) { - if (!mInitialized) { - return; - } - - // Schedule next frame already such that back-pressure happens continuously - scheduleAnimation(); + if (!mInitialized) { + return; } - synchronized (mService.mGlobalLock) { - mCurrentTime = frameTimeNs / TimeUtils.NANOS_PER_MS; - mBulkUpdateParams = SET_ORIENTATION_CHANGE_COMPLETE; - if (DEBUG_WINDOW_TRACE) { - Slog.i(TAG, "!!! animate: entry time=" + mCurrentTime); + // Schedule next frame already such that back-pressure happens continuously. + scheduleAnimation(); + + mCurrentTime = frameTimeNs / TimeUtils.NANOS_PER_MS; + mBulkUpdateParams = SET_ORIENTATION_CHANGE_COMPLETE; + if (DEBUG_WINDOW_TRACE) { + Slog.i(TAG, "!!! animate: entry time=" + mCurrentTime); + } + + ProtoLog.i(WM_SHOW_TRANSACTIONS, ">>> OPEN TRANSACTION animate"); + mService.openSurfaceTransaction(); + try { + final AccessibilityController accessibilityController = + mService.mAccessibilityController; + final int numDisplays = mDisplayContentsAnimators.size(); + for (int i = 0; i < numDisplays; i++) { + final int displayId = mDisplayContentsAnimators.keyAt(i); + final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); + // Update animations of all applications, including those associated with + // exiting/removed apps. + dc.updateWindowsForAnimator(); + dc.prepareSurfaces(); } - ProtoLog.i(WM_SHOW_TRANSACTIONS, ">>> OPEN TRANSACTION animate"); - mService.openSurfaceTransaction(); - try { - final AccessibilityController accessibilityController = - mService.mAccessibilityController; - final int numDisplays = mDisplayContentsAnimators.size(); - for (int i = 0; i < numDisplays; i++) { - final int displayId = mDisplayContentsAnimators.keyAt(i); - final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); - // Update animations of all applications, including those - // associated with exiting/removed apps - dc.updateWindowsForAnimator(); - dc.prepareSurfaces(); + for (int i = 0; i < numDisplays; i++) { + final int displayId = mDisplayContentsAnimators.keyAt(i); + final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); + + dc.checkAppWindowsReadyToShow(); + if (accessibilityController != null) { + accessibilityController.drawMagnifiedRegionBorderIfNeededLocked(displayId, + mTransaction); } - - for (int i = 0; i < numDisplays; i++) { - final int displayId = mDisplayContentsAnimators.keyAt(i); - final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); - - dc.checkAppWindowsReadyToShow(); - if (accessibilityController != null) { - accessibilityController.drawMagnifiedRegionBorderIfNeededLocked(displayId, - mTransaction); - } - } - - cancelAnimation(); - - if (mService.mWatermark != null) { - mService.mWatermark.drawIfNeeded(); - } - - SurfaceControl.mergeToGlobalTransaction(mTransaction); - } catch (RuntimeException e) { - Slog.wtf(TAG, "Unhandled exception in Window Manager", e); - } finally { - mService.closeSurfaceTransaction("WindowAnimator"); - ProtoLog.i(WM_SHOW_TRANSACTIONS, "<<< CLOSE TRANSACTION animate"); } - boolean hasPendingLayoutChanges = mService.mRoot.hasPendingLayoutChanges(this); - boolean doRequest = false; - if (mBulkUpdateParams != 0) { - doRequest = mService.mRoot.copyAnimToLayoutParams(); + cancelAnimation(); + + if (mService.mWatermark != null) { + mService.mWatermark.drawIfNeeded(); } - if (hasPendingLayoutChanges || doRequest) { - mService.mWindowPlacerLocked.requestTraversal(); - } + SurfaceControl.mergeToGlobalTransaction(mTransaction); + } catch (RuntimeException e) { + Slog.wtf(TAG, "Unhandled exception in Window Manager", e); + } finally { + mService.closeSurfaceTransaction("WindowAnimator"); + ProtoLog.i(WM_SHOW_TRANSACTIONS, "<<< CLOSE TRANSACTION animate"); + } - final boolean rootAnimating = mService.mRoot.isAnimating(TRANSITION | CHILDREN); - if (rootAnimating && !mLastRootAnimating) { + final boolean hasPendingLayoutChanges = mService.mRoot.hasPendingLayoutChanges(this); + final boolean doRequest = mBulkUpdateParams != 0 && mService.mRoot.copyAnimToLayoutParams(); + if (hasPendingLayoutChanges || doRequest) { + mService.mWindowPlacerLocked.requestTraversal(); + } - // Usually app transitions but quite a load onto the system already (with all the - // things happening in app), so pause task snapshot persisting to not increase the - // load. - mService.mTaskSnapshotController.setPersisterPaused(true); - Trace.asyncTraceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0); - } - if (!rootAnimating && mLastRootAnimating) { - mService.mWindowPlacerLocked.requestTraversal(); - mService.mTaskSnapshotController.setPersisterPaused(false); - Trace.asyncTraceEnd(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0); - } + final boolean rootAnimating = mService.mRoot.isAnimating(TRANSITION | CHILDREN); + if (rootAnimating && !mLastRootAnimating) { + // Usually app transitions but quite a load onto the system already (with all the things + // happening in app), so pause task snapshot persisting to not increase the load. + mService.mTaskSnapshotController.setPersisterPaused(true); + Trace.asyncTraceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0); + } + if (!rootAnimating && mLastRootAnimating) { + mService.mWindowPlacerLocked.requestTraversal(); + mService.mTaskSnapshotController.setPersisterPaused(false); + Trace.asyncTraceEnd(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0); + } - mLastRootAnimating = rootAnimating; + mLastRootAnimating = rootAnimating; - if (mRemoveReplacedWindows) { - mService.mRoot.removeReplacedWindows(); - mRemoveReplacedWindows = false; - } + if (mRemoveReplacedWindows) { + mService.mRoot.removeReplacedWindows(); + mRemoveReplacedWindows = false; + } - mService.destroyPreservedSurfaceLocked(); + mService.destroyPreservedSurfaceLocked(); - executeAfterPrepareSurfacesRunnables(); + executeAfterPrepareSurfacesRunnables(); - if (DEBUG_WINDOW_TRACE) { - Slog.i(TAG, "!!! animate: exit" - + " mBulkUpdateParams=" + Integer.toHexString(mBulkUpdateParams) - + " hasPendingLayoutChanges=" + hasPendingLayoutChanges); - } + if (DEBUG_WINDOW_TRACE) { + Slog.i(TAG, "!!! animate: exit" + + " mBulkUpdateParams=" + Integer.toHexString(mBulkUpdateParams) + + " hasPendingLayoutChanges=" + hasPendingLayoutChanges); } } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 3f4f629b52924..5726ff533313e 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -1007,9 +1007,7 @@ public class WindowManagerService extends IWindowManager.Stub void openSurfaceTransaction() { try { Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "openSurfaceTransaction"); - synchronized (mGlobalLock) { - SurfaceControl.openTransaction(); - } + SurfaceControl.openTransaction(); } finally { Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER); } @@ -1022,10 +1020,8 @@ public class WindowManagerService extends IWindowManager.Stub void closeSurfaceTransaction(String where) { try { Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "closeSurfaceTransaction"); - synchronized (mGlobalLock) { - SurfaceControl.closeTransaction(); - mWindowTracing.logState(where); - } + SurfaceControl.closeTransaction(); + mWindowTracing.logState(where); } finally { Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER); }