From c60451d0940e3018df262f54ac7dedeed40e5a91 Mon Sep 17 00:00:00 2001 From: Ahan Wu Date: Thu, 10 Feb 2022 16:28:27 +0800 Subject: [PATCH] Defer the instrument of AOD transition per doze amount When the doze changes, StatusBar#updateDozing will be invoked at the first frame, usually make the following one be regarded as janky, but it is unperceptible by the users, so ignore it. Bug: 201987244 Test: Capture the trace manually Change-Id: Iac739d23eb0fdc988bd52076bdccca4665e28d18 --- .../android/internal/jank/FrameTracker.java | 40 ++++++++++++++----- .../internal/jank/InteractionJankMonitor.java | 26 +++++++++++- .../StatusBarStateControllerImpl.java | 13 +++++- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/core/java/com/android/internal/jank/FrameTracker.java b/core/java/com/android/internal/jank/FrameTracker.java index e1a67d861bcb9..5046389373828 100644 --- a/core/java/com/android/internal/jank/FrameTracker.java +++ b/core/java/com/android/internal/jank/FrameTracker.java @@ -98,6 +98,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener private final Handler mHandler; private final ChoreographerWrapper mChoreographer; private final Object mLock = InteractionJankMonitor.getInstance().getLock(); + private final boolean mDeferMonitoring; @VisibleForTesting public final boolean mSurfaceOnly; @@ -153,6 +154,7 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener mHandler = handler; mChoreographer = choreographer; mSurfaceControlWrapper = surfaceControlWrapper; + mDeferMonitoring = config.shouldDeferMonitor(); // HWUI instrumentation init. mRendererWrapper = mSurfaceOnly ? null : renderer; @@ -228,12 +230,25 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener */ public void begin() { synchronized (mLock) { - mBeginVsyncId = mChoreographer.getVsyncId() + 1; + final long currentVsync = mChoreographer.getVsyncId(); + // In normal case, we should begin at the next frame, + // the id of the next frame is not simply increased by 1, + // but we can exclude the current frame at least. + mBeginVsyncId = mDeferMonitoring ? currentVsync + 1 : currentVsync; if (DEBUG) { - Log.d(TAG, "begin: " + mSession.getName() + ", begin=" + mBeginVsyncId); + Log.d(TAG, "begin: " + mSession.getName() + ", begin=" + mBeginVsyncId + + ", defer=" + mDeferMonitoring); } if (mSurfaceControl != null) { - postTraceStartMarker(); + if (mDeferMonitoring) { + // Normal case, we begin the instrument from the very beginning, + // except the first frame. + postTraceStartMarker(); + } else { + // If we don't begin the instrument from the very beginning, + // there is no need to skip the frame where the begin invocation happens. + beginInternal(); + } mSurfaceControlWrapper.addJankStatsListener(this, mSurfaceControl); } if (!mSurfaceOnly) { @@ -247,15 +262,18 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener */ @VisibleForTesting public void postTraceStartMarker() { - mChoreographer.mChoreographer.postCallback(Choreographer.CALLBACK_INPUT, () -> { - synchronized (mLock) { - if (mCancelled || mEndVsyncId != INVALID_ID) { - return; - } - mTracingStarted = true; - Trace.beginAsyncSection(mSession.getName(), (int) mBeginVsyncId); + mChoreographer.mChoreographer.postCallback( + Choreographer.CALLBACK_INPUT, this::beginInternal, null); + } + + private void beginInternal() { + synchronized (mLock) { + if (mCancelled || mEndVsyncId != INVALID_ID) { + return; } - }, null); + mTracingStarted = true; + Trace.beginAsyncSection(mSession.getName(), (int) mBeginVsyncId); + } } /** diff --git a/core/java/com/android/internal/jank/InteractionJankMonitor.java b/core/java/com/android/internal/jank/InteractionJankMonitor.java index 5a66e9aec3e7c..3746bfdae7812 100644 --- a/core/java/com/android/internal/jank/InteractionJankMonitor.java +++ b/core/java/com/android/internal/jank/InteractionJankMonitor.java @@ -717,6 +717,7 @@ public class InteractionJankMonitor { private final boolean mSurfaceOnly; private final SurfaceControl mSurfaceControl; private final @CujType int mCujType; + private final boolean mDeferMonitor; /** * A builder for building Configuration. {@link #setView(View)} is essential @@ -733,6 +734,7 @@ public class InteractionJankMonitor { private boolean mAttrSurfaceOnly; private SurfaceControl mAttrSurfaceControl; private @CujType int mAttrCujType; + private boolean mAttrDeferMonitor = true; /** * Creates a builder which instruments only surface. @@ -822,6 +824,16 @@ public class InteractionJankMonitor { return this; } + /** + * Indicates if the instrument should be deferred to the next frame. + * @param defer true if the instrument should be deferred to the next frame. + * @return builder + */ + public Builder setDeferMonitorForAnimationStart(boolean defer) { + mAttrDeferMonitor = defer; + return this; + } + /** * Builds the {@link Configuration} instance * @return the instance of {@link Configuration} @@ -830,12 +842,14 @@ public class InteractionJankMonitor { public Configuration build() throws IllegalArgumentException { return new Configuration( mAttrCujType, mAttrView, mAttrTag, mAttrTimeout, - mAttrSurfaceOnly, mAttrContext, mAttrSurfaceControl); + mAttrSurfaceOnly, mAttrContext, mAttrSurfaceControl, + mAttrDeferMonitor); } } private Configuration(@CujType int cuj, View view, String tag, long timeout, - boolean surfaceOnly, Context context, SurfaceControl surfaceControl) { + boolean surfaceOnly, Context context, SurfaceControl surfaceControl, + boolean deferMonitor) { mCujType = cuj; mTag = tag; mTimeout = timeout; @@ -845,6 +859,7 @@ public class InteractionJankMonitor { ? context : (view != null ? view.getContext().getApplicationContext() : null); mSurfaceControl = surfaceControl; + mDeferMonitor = deferMonitor; validate(); } @@ -901,6 +916,13 @@ public class InteractionJankMonitor { Context getContext() { return mContext; } + + /** + * @return true if the monitoring should be deferred to the next frame, false otherwise. + */ + public boolean shouldDeferMonitor() { + return mDeferMonitor; + } } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java index fcbe17957d999..02870a3820564 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java @@ -32,6 +32,7 @@ import android.os.Trace; import android.text.format.DateFormat; import android.util.FloatProperty; import android.util.Log; +import android.view.Choreographer; import android.view.InsetsFlags; import android.view.InsetsVisibilities; import android.view.View; @@ -44,6 +45,7 @@ import androidx.annotation.NonNull; import com.android.internal.annotations.GuardedBy; import com.android.internal.jank.InteractionJankMonitor; +import com.android.internal.jank.InteractionJankMonitor.Configuration; import com.android.internal.logging.UiEventLogger; import com.android.systemui.DejankUtils; import com.android.systemui.Dumpable; @@ -352,8 +354,17 @@ public class StatusBarStateControllerImpl implements } private void beginInteractionJankMonitor() { + final boolean shouldPost = + (mIsDozing && mDozeAmount == 0) || (!mIsDozing && mDozeAmount == 1); if (mInteractionJankMonitor != null && mView != null && mView.isAttachedToWindow()) { - mInteractionJankMonitor.begin(mView, getCujType()); + if (shouldPost) { + Choreographer.getInstance().postCallback( + Choreographer.CALLBACK_ANIMATION, this::beginInteractionJankMonitor, null); + } else { + Configuration.Builder builder = Configuration.Builder.withView(getCujType(), mView) + .setDeferMonitorForAnimationStart(false); + mInteractionJankMonitor.begin(builder); + } } }