From 26cbd718964d2a5474e97e5385e733452283cec4 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Tue, 22 Jan 2019 17:59:48 -0800 Subject: [PATCH] Post expensive ContentCapture calls to Choreographer's CALLBACK_COMMIT stage. onProvideContentCaptureStructure() doesn't take much for only 1 view, but it's called for all views the first time the view hiearchy is laid off, which could acccumulative delay the initial layout. Hence, we're postponing it to a later stage - it might still cost a lost frame (or more), but that jank cost would only happen after the 1st layout. Using this change, the impact on cold-dropcache-test dropped about 50% (from ~2ms to ~1ms). Bug: 123307965 Bug: 121039624 Test: atest ContentCaptureSession Test: atest google/perf/app-startup/benchmark-app-hermetic/cold-dropcache-test Change-Id: I68b98b2894d23309af90d87cc99280f133557252 --- core/java/android/view/View.java | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index f1dfc1c6072f..cd3decf4e981 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -9067,35 +9067,43 @@ public class View implements Drawable.Callback, KeyEvent.Callback, Log.v(CONTENT_CAPTURE_LOG_TAG, "Ignoring 'appeared' on " + this + ": laid=" + isLaidOut() + ", visibleToUser=" + isVisibleToUser() + ", visible=" + (getVisibility() == VISIBLE) - + ": alreadyNotifiedAppeared=" - + ((mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) != 0)); + + ": alreadyNotifiedAppeared=" + ((mPrivateFlags4 + & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) != 0) + + ", alreadyNotifiedDisappeared=" + ((mPrivateFlags4 + & PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED) != 0)); } return; } - // All good: notify it... - final ViewStructure structure = session.newViewStructure(this); - onProvideContentCaptureStructure(structure, /* flags= */ 0); - session.notifyViewAppeared(structure); - // ...and set the flags mPrivateFlags4 |= PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED; mPrivateFlags4 &= ~PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED; + + // The code below doesn't take much for a unique view, but it's called for all views + // the first time the view hiearchy is laid off, which could acccumulative delay the + // initial layout. Hence, we're postponing it to a later stage - it might still cost a + // lost frame (or more), but that jank cost would only happen after the 1st layout. + Choreographer.getInstance().postCallback(Choreographer.CALLBACK_COMMIT, () -> { + final ViewStructure structure = session.newViewStructure(this); + onProvideContentCaptureStructure(structure, /* flags= */ 0); + session.notifyViewAppeared(structure); + }, /* token= */ null); } else { if ((mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) == 0 || (mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED) != 0) { if (Log.isLoggable(CONTENT_CAPTURE_LOG_TAG, Log.VERBOSE)) { - Log.v(CONTENT_CAPTURE_LOG_TAG, "Ignoring 'disappeared' on " + this - + ": notifiedAppeared=" - + ((mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) != 0) + Log.v(CONTENT_CAPTURE_LOG_TAG, "Ignoring 'disappeared' on " + this + ": laid=" + + isLaidOut() + ", visibleToUser=" + isVisibleToUser() + + ", visible=" + (getVisibility() == VISIBLE) + + ": alreadyNotifiedAppeared=" + ((mPrivateFlags4 + & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) != 0) + ", alreadyNotifiedDisappeared=" + ((mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED) != 0)); } return; } - // All good: notify it... - session.notifyViewDisappeared(getAutofillId()); - // ...and set the flags mPrivateFlags4 |= PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED; mPrivateFlags4 &= ~PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED; + Choreographer.getInstance().postCallback(Choreographer.CALLBACK_COMMIT, + () -> session.notifyViewDisappeared(getAutofillId()), /* token= */ null); } } -- 2.11.0