From ed7993b5d147a6741d26fe0b16cc9fa5e34ceaee Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Tue, 28 Mar 2017 18:50:01 +0100 Subject: [PATCH] Introduce android.anim thread in system_server We create a new thread on which everything is running that directly impacts window animations, i.e. layout, anim tick and starting window creation. This is such that any work on android.display can not lead to jank in the window animation, specifically lock contention on activity manager lock that blocks callbacks from android.display into AM can not lead to window animation jank. Test: Run animation, take systrace, make sure animation is on android.anim Test: AppWindowContainerControllerTestTest: AppWindowContainerControllerTestss Fixes: 36792959 Change-Id: I5d41419a709b7984724e7053a3afdcc1ffe1aaa2 --- .../SurfaceFlingerVsyncChoreographer.java | 6 +- .../systemui/stackdivider/DividerView.java | 4 +- .../com/android/server/AnimationThread.java | 58 +++++++++++++++++++ .../com/android/server/DisplayThread.java | 5 +- .../server/display/DisplayManagerService.java | 15 ++--- .../wm/AppWindowContainerController.java | 2 +- .../com/android/server/wm/TaskPositioner.java | 3 +- .../com/android/server/wm/WindowAnimator.java | 31 ++++++++-- .../server/wm/WindowManagerService.java | 27 ++++----- .../server/wm/WindowSurfacePlacer.java | 12 +++- .../wm/AppWindowContainerControllerTests.java | 10 ++-- .../android/server/wm/WindowTestsBase.java | 3 +- 12 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 services/core/java/com/android/server/AnimationThread.java diff --git a/core/java/com/android/internal/view/SurfaceFlingerVsyncChoreographer.java b/core/java/com/android/internal/view/SurfaceFlingerVsyncChoreographer.java index e40090f63a762..924b3f704f5be 100644 --- a/core/java/com/android/internal/view/SurfaceFlingerVsyncChoreographer.java +++ b/core/java/com/android/internal/view/SurfaceFlingerVsyncChoreographer.java @@ -31,7 +31,7 @@ public class SurfaceFlingerVsyncChoreographer { private static final long ONE_S_IN_NS = ONE_MS_IN_NS * 1000; private final Handler mHandler; - private final Choreographer mChoreographer = Choreographer.getInstance(); + private final Choreographer mChoreographer; /** * The offset between vsync-app and vsync-surfaceflinger. See @@ -39,8 +39,10 @@ public class SurfaceFlingerVsyncChoreographer { */ private long mSurfaceFlingerOffsetMs; - public SurfaceFlingerVsyncChoreographer(Handler handler, Display display) { + public SurfaceFlingerVsyncChoreographer(Handler handler, Display display, + Choreographer choreographer) { mHandler = handler; + mChoreographer = choreographer; mSurfaceFlingerOffsetMs = calculateAppSurfaceFlingerVsyncOffsetMs(display); } diff --git a/packages/SystemUI/src/com/android/systemui/stackdivider/DividerView.java b/packages/SystemUI/src/com/android/systemui/stackdivider/DividerView.java index 0ee3e19171176..c48ecdbe838a1 100644 --- a/packages/SystemUI/src/com/android/systemui/stackdivider/DividerView.java +++ b/packages/SystemUI/src/com/android/systemui/stackdivider/DividerView.java @@ -33,6 +33,7 @@ import android.os.Bundle; import android.os.Handler; import android.os.Message; import android.util.AttributeSet; +import android.view.Choreographer; import android.view.Display; import android.view.DisplayInfo; import android.view.GestureDetector; @@ -312,7 +313,8 @@ public class DividerView extends FrameLayout implements OnTouchListener, protected void onAttachedToWindow() { super.onAttachedToWindow(); EventBus.getDefault().register(this); - mSfChoreographer = new SurfaceFlingerVsyncChoreographer(mHandler, getDisplay()); + mSfChoreographer = new SurfaceFlingerVsyncChoreographer(mHandler, getDisplay(), + Choreographer.getInstance()); } @Override diff --git a/services/core/java/com/android/server/AnimationThread.java b/services/core/java/com/android/server/AnimationThread.java new file mode 100644 index 0000000000000..08392b067a9a2 --- /dev/null +++ b/services/core/java/com/android/server/AnimationThread.java @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.server; + +import static android.os.Process.THREAD_PRIORITY_DISPLAY; + +import android.os.Handler; +import android.os.Trace; + +/** + * Thread for handling all window animations, or anything that's directly impacting animations like + * starting windows or traversals. + */ +public final class AnimationThread extends ServiceThread { + private static AnimationThread sInstance; + private static Handler sHandler; + + private AnimationThread() { + super("android.anim", THREAD_PRIORITY_DISPLAY, false /*allowIo*/); + } + + private static void ensureThreadLocked() { + if (sInstance == null) { + sInstance = new AnimationThread(); + sInstance.start(); + sInstance.getLooper().setTraceTag(Trace.TRACE_TAG_WINDOW_MANAGER); + sHandler = new Handler(sInstance.getLooper()); + } + } + + public static AnimationThread get() { + synchronized (AnimationThread.class) { + ensureThreadLocked(); + return sInstance; + } + } + + public static Handler getHandler() { + synchronized (AnimationThread.class) { + ensureThreadLocked(); + return sHandler; + } + } +} diff --git a/services/core/java/com/android/server/DisplayThread.java b/services/core/java/com/android/server/DisplayThread.java index 9ef02598c1d38..cad2a612a13d0 100644 --- a/services/core/java/com/android/server/DisplayThread.java +++ b/services/core/java/com/android/server/DisplayThread.java @@ -17,6 +17,7 @@ package com.android.server; import android.os.Handler; +import android.os.Process; import android.os.Trace; /** @@ -30,7 +31,9 @@ public final class DisplayThread extends ServiceThread { private static Handler sHandler; private DisplayThread() { - super("android.display", android.os.Process.THREAD_PRIORITY_DISPLAY, false /*allowIo*/); + // DisplayThread runs important stuff, but these are not as important as things running in + // AnimationThread. Thus, set the priority to one lower. + super("android.display", Process.THREAD_PRIORITY_DISPLAY + 1, false /*allowIo*/); } private static void ensureThreadLocked() { diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index d83676b58552f..208295847e24d 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -63,6 +63,7 @@ import android.view.DisplayInfo; import android.view.Surface; import android.view.WindowManagerInternal; +import com.android.server.AnimationThread; import com.android.server.DisplayThread; import com.android.server.LocalServices; import com.android.server.SystemService; @@ -257,13 +258,13 @@ public final class DisplayManagerService extends SystemService { } public void setupSchedulerPolicies() { - /* - * android.display is critical to user experience and we should - * make sure it is not in the default foregroup groups, add it to - * top-app to make sure it uses all the cores and scheduling - * settings for top-app when it runs. - */ - Process.setThreadGroupAndCpuset(DisplayThread.get().getThreadId(), Process.THREAD_GROUP_TOP_APP); + // android.display and android.anim is critical to user experience and we should make sure + // it is not in the default foregroup groups, add it to top-app to make sure it uses all the + // cores and scheduling settings for top-app when it runs. + Process.setThreadGroupAndCpuset(DisplayThread.get().getThreadId(), + Process.THREAD_GROUP_TOP_APP); + Process.setThreadGroupAndCpuset(AnimationThread.get().getThreadId(), + Process.THREAD_GROUP_TOP_APP); } @Override diff --git a/services/core/java/com/android/server/wm/AppWindowContainerController.java b/services/core/java/com/android/server/wm/AppWindowContainerController.java index 4b4be40880ee6..a39131cbaf039 100644 --- a/services/core/java/com/android/server/wm/AppWindowContainerController.java +++ b/services/core/java/com/android/server/wm/AppWindowContainerController.java @@ -554,7 +554,7 @@ public class AppWindowContainerController // want to process the message ASAP, before any other queued // messages. if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Enqueueing ADD_STARTING"); - mHandler.postAtFrontOfQueue(mAddStartingWindow); + mService.mAnimationHandler.postAtFrontOfQueue(mAddStartingWindow); } private boolean createSnapshot() { diff --git a/services/core/java/com/android/server/wm/TaskPositioner.java b/services/core/java/com/android/server/wm/TaskPositioner.java index 53c24e13e8593..0c68e2ca6d772 100644 --- a/services/core/java/com/android/server/wm/TaskPositioner.java +++ b/services/core/java/com/android/server/wm/TaskPositioner.java @@ -262,7 +262,8 @@ class TaskPositioner implements DimLayer.DimLayerUser { mService.mInputManager.registerInputChannel(mServerChannel, null); mInputEventReceiver = new WindowPositionerEventReceiver( - mClientChannel, mService.mH.getLooper(), mService.mChoreographer); + mClientChannel, mService.mAnimationHandler.getLooper(), + mService.mAnimator.getChoreographer()); mDragApplicationHandle = new InputApplicationHandle(null); mDragApplicationHandle.name = TAG; diff --git a/services/core/java/com/android/server/wm/WindowAnimator.java b/services/core/java/com/android/server/wm/WindowAnimator.java index 57eaa2b2ad8d9..1367a06eb1109 100644 --- a/services/core/java/com/android/server/wm/WindowAnimator.java +++ b/services/core/java/com/android/server/wm/WindowAnimator.java @@ -25,7 +25,6 @@ import static com.android.server.wm.WindowSurfacePlacer.SET_ORIENTATION_CHANGE_C import static com.android.server.wm.WindowSurfacePlacer.SET_UPDATE_ROTATION; import android.content.Context; -import android.os.Handler; import android.os.Trace; import android.util.Slog; import android.util.SparseArray; @@ -35,7 +34,7 @@ import android.view.SurfaceControl; import android.view.WindowManagerPolicy; import com.android.internal.view.SurfaceFlingerVsyncChoreographer; -import com.android.server.DisplayThread; +import com.android.server.AnimationThread; import java.io.PrintWriter; @@ -87,20 +86,25 @@ public class WindowAnimator { private final Runnable mAnimationTick; private final SurfaceFlingerVsyncChoreographer mSfChoreographer; + private Choreographer mChoreographer; + private boolean mAnimationScheduled; + + WindowAnimator(final WindowManagerService service) { mService = service; mContext = service.mContext; mPolicy = service.mPolicy; mWindowPlacerLocked = service.mWindowPlacerLocked; - final Handler handler = DisplayThread.getHandler(); + AnimationThread.getHandler().runWithScissors( + () -> mChoreographer = Choreographer.getInstance(), 0 /* timeout */); // TODO: Multi-display: If displays have different vsync tick, have a separate tick per // display. - mSfChoreographer = new SurfaceFlingerVsyncChoreographer(handler, - mService.getDefaultDisplayContentLocked().getDisplay()); + mSfChoreographer = new SurfaceFlingerVsyncChoreographer(AnimationThread.getHandler(), + mService.getDefaultDisplayContentLocked().getDisplay(), mChoreographer); mAnimationTick = () -> { synchronized (mService.mWindowMap) { - mService.mAnimationScheduled = false; + mAnimationScheduled = false; animateLocked(mCurrentFrameTime); } }; @@ -366,6 +370,13 @@ public class WindowAnimator { mRemoveReplacedWindows = true; } + void scheduleAnimation() { + if (!mAnimationScheduled) { + mAnimationScheduled = true; + mChoreographer.postFrameCallback(mAnimationFrameCallback); + } + } + private class DisplayContentsAnimator { ScreenRotationAnimation mScreenRotationAnimation = null; } @@ -374,6 +385,14 @@ public class WindowAnimator { return mAnimating; } + boolean isAnimationScheduled() { + return mAnimationScheduled; + } + + Choreographer getChoreographer() { + return mChoreographer; + } + void setAnimating(boolean animating) { mAnimating = animating; } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 1be051270e161..1963c93498474 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -31,6 +31,7 @@ import static android.os.Process.SHELL_UID; import static android.os.Process.SYSTEM_UID; import static android.os.Process.THREAD_PRIORITY_DISPLAY; import static android.os.Process.myPid; +import static android.os.Process.myTid; import static android.os.UserHandle.USER_NULL; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.WindowManager.DOCKED_INVALID; @@ -147,6 +148,7 @@ import android.os.ParcelFileDescriptor; import android.os.PowerManager; import android.os.PowerManagerInternal; import android.os.PowerSaveState; +import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; import android.os.StrictMode; @@ -169,7 +171,6 @@ import android.util.SparseIntArray; import android.util.TimeUtils; import android.util.TypedValue; import android.view.AppTransitionAnimationSpec; -import android.view.Choreographer; import android.view.Display; import android.view.DisplayInfo; import android.view.Gravity; @@ -218,6 +219,7 @@ import com.android.internal.view.IInputContext; import com.android.internal.view.IInputMethodClient; import com.android.internal.view.IInputMethodManager; import com.android.internal.view.WindowManagerPolicyThread; +import com.android.server.AnimationThread; import com.android.server.DisplayThread; import com.android.server.EventLogTags; import com.android.server.FgThread; @@ -601,7 +603,12 @@ public class WindowManagerService extends IWindowManager.Stub final H mH = new H(); - final Choreographer mChoreographer = Choreographer.getInstance(); + /** + * Handler for things to run that have direct impact on an animation, i.e. animation tick, + * layout, starting window creation, whereas {@link H} runs things that are still important, but + * not as critical. + */ + final Handler mAnimationHandler = new Handler(AnimationThread.getHandler().getLooper()); WindowState mCurrentFocus = null; WindowState mLastFocus = null; @@ -708,8 +715,6 @@ public class WindowManagerService extends IWindowManager.Stub // For frozen screen animations. private int mExitAnimId, mEnterAnimId; - boolean mAnimationScheduled; - /** Skip repeated AppWindowTokens initialization. Note that AppWindowsToken's version of this * is a long initialized to Long.MIN_VALUE so that it doesn't match this value on startup. */ int mTransactionSequence; @@ -4645,7 +4650,6 @@ public class WindowManagerService extends IWindowManager.Stub final class H extends android.os.Handler { public static final int REPORT_FOCUS_CHANGE = 2; public static final int REPORT_LOSING_FOCUS = 3; - public static final int DO_TRAVERSAL = 4; public static final int WINDOW_FREEZE_TIMEOUT = 11; public static final int APP_TRANSITION_TIMEOUT = 13; @@ -4775,12 +4779,6 @@ public class WindowManagerService extends IWindowManager.Stub } } break; - case DO_TRAVERSAL: { - synchronized(mWindowMap) { - mWindowPlacerLocked.performSurfacePlacement(); - } - } break; - case WINDOW_FREEZE_TIMEOUT: { // TODO(multidisplay): Can non-default displays rotate? synchronized (mWindowMap) { @@ -4849,7 +4847,7 @@ public class WindowManagerService extends IWindowManager.Stub synchronized (mWindowMap) { // Since we're holding both mWindowMap and mAnimator we don't need to // hold mAnimator.mLayoutToAnim. - if (mAnimator.isAnimating() || mAnimationScheduled) { + if (mAnimator.isAnimating() || mAnimator.isAnimationScheduled()) { // If we are animating, don't do the gc now but // delay a bit so we don't interrupt the animation. sendEmptyMessageDelayed(H.FORCE_GC, 2000); @@ -5744,10 +5742,7 @@ public class WindowManagerService extends IWindowManager.Stub /** Note that Locked in this case is on mLayoutToAnim */ void scheduleAnimationLocked() { - if (!mAnimationScheduled) { - mAnimationScheduled = true; - mChoreographer.postFrameCallback(mAnimator.mAnimationFrameCallback); - } + mAnimator.scheduleAnimation(); } // TODO: Move to DisplayContent diff --git a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java index ee2d5de710c14..ddd1ca5c1aecd 100644 --- a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java +++ b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java @@ -30,7 +30,6 @@ import static com.android.server.wm.WindowManagerDebugConfig.SHOW_LIGHT_TRANSACT import static com.android.server.wm.WindowManagerDebugConfig.SHOW_TRANSACTIONS; import static com.android.server.wm.WindowManagerDebugConfig.TAG_WITH_CLASS_NAME; import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM; -import static com.android.server.wm.WindowManagerService.H.DO_TRAVERSAL; import static com.android.server.wm.WindowManagerService.H.NOTIFY_APP_TRANSITION_STARTING; import static com.android.server.wm.WindowManagerService.H.REPORT_WINDOWS_CHANGE; import static com.android.server.wm.WindowManagerService.LAYOUT_REPEAT_THRESHOLD; @@ -97,9 +96,16 @@ class WindowSurfacePlacer { private final ArrayList mPendingDestroyingSurfaces = new ArrayList<>(); private final SparseIntArray mTempTransitionReasons = new SparseIntArray(); + private final Runnable mPerformSurfacePlacement; + public WindowSurfacePlacer(WindowManagerService service) { mService = service; mWallpaperControllerLocked = mService.mRoot.mWallpaperController; + mPerformSurfacePlacement = () -> { + synchronized (mService.mWindowMap) { + performSurfacePlacement(); + } + }; } /** @@ -131,7 +137,7 @@ class WindowSurfacePlacer { do { mTraversalScheduled = false; performSurfacePlacementLoop(); - mService.mH.removeMessages(DO_TRAVERSAL); + mService.mAnimationHandler.removeCallbacks(mPerformSurfacePlacement); loopCount--; } while (mTraversalScheduled && loopCount > 0); mService.mRoot.mWallpaperActionPending = false; @@ -735,7 +741,7 @@ class WindowSurfacePlacer { void requestTraversal() { if (!mTraversalScheduled) { mTraversalScheduled = true; - mService.mH.sendEmptyMessage(DO_TRAVERSAL); + mService.mAnimationHandler.post(mPerformSurfacePlacement); } } diff --git a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java index 25004de606768..3c8bf206b8a6b 100644 --- a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java @@ -98,11 +98,11 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { createAppWindowController(); controller.addStartingWindow(InstrumentationRegistry.getContext().getPackageName(), android.R.style.Theme, null, "Test", 0, 0, 0, 0, null, true, true, false); - waitUntilHandlerIdle(); + waitUntilHandlersIdle(); final AppWindowToken atoken = controller.getAppWindowToken(); assertHasStartingWindow(atoken); controller.removeStartingWindow(); - waitUntilHandlerIdle(); + waitUntilHandlersIdle(); assertNoStartingWindow(atoken); } @@ -114,11 +114,11 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { createAppWindowController(); controller1.addStartingWindow(InstrumentationRegistry.getContext().getPackageName(), android.R.style.Theme, null, "Test", 0, 0, 0, 0, null, true, true, false); - waitUntilHandlerIdle(); + waitUntilHandlersIdle(); controller2.addStartingWindow(InstrumentationRegistry.getContext().getPackageName(), android.R.style.Theme, null, "Test", 0, 0, 0, 0, controller1.mToken.asBinder(), true, true, false); - waitUntilHandlerIdle(); + waitUntilHandlersIdle(); assertNoStartingWindow(controller1.getAppWindowToken()); assertHasStartingWindow(controller2.getAppWindowToken()); } @@ -138,7 +138,7 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { }); controller1.addStartingWindow(InstrumentationRegistry.getContext().getPackageName(), android.R.style.Theme, null, "Test", 0, 0, 0, 0, null, true, true, false); - waitUntilHandlerIdle(); + waitUntilHandlersIdle(); assertNoStartingWindow(controller1.getAppWindowToken()); assertHasStartingWindow(controller2.getAppWindowToken()); } diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java index eaf4ac4baf4f3..218af7356e30a 100644 --- a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java @@ -167,8 +167,9 @@ class WindowTestsBase { /** * Waits until the main handler for WM has processed all messages. */ - void waitUntilHandlerIdle() { + void waitUntilHandlersIdle() { sWm.mH.runWithScissors(() -> { }, 0); + sWm.mAnimationHandler.runWithScissors(() -> { }, 0); } private static WindowToken createWindowToken(DisplayContent dc, int stackId, int type) {