New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 855626 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

30% regression in smoothness.tough_scrolling_cases at 565414:565493

Project Member Reported by ajuma@chromium.org, Jun 22 2018

Issue description

mean_pixels_checkerboarded regressed on the android-webview-nexus6 and android-webview-nexus5X bots:

https://chromeperf.appspot.com/report?sid=3757be325a81f91a0721d12a079e97eb71414da4b9cad71ad69f088f773395dc&start_rev=562778&end_rev=568778

Triggering bisects.

 
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jun 26 2018

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14eb9a17240000

The swarming task expired. The bots are likely overloaded, dead, or misconfigured.
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Jun 26 2018

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14461c73240000

The swarming task expired. The bots are likely overloaded, dead, or misconfigured.
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Jun 29 2018

Cc: acondor@chromium.org
Owner: acondor@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1585c633240000

VR: Ensuring consistency of events when transitioning to 2D UI by acondor@chromium.org
https://chromium.googlesource.com/chromium/src/+/0d7abffcc3f8d9f81bd3dea8a2aea9b70bbb390b
27.94 → 34.85 (+6.905)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Jun 30 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/132a7367240000

VR: Ensuring consistency of events when transitioning to 2D UI by acondor@chromium.org
https://chromium.googlesource.com/chromium/src/+/0d7abffcc3f8d9f81bd3dea8a2aea9b70bbb390b
18.28 → 23.34 (+5.065)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 11 by 42576172...@developer.gserviceaccount.com, Jun 30 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14d8eebf240000

VR: Ensuring consistency of events when transitioning to 2D UI by acondor@chromium.org
https://chromium.googlesource.com/chromium/src/+/0d7abffcc3f8d9f81bd3dea8a2aea9b70bbb390b
18.26 → 23.35 (+5.085)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/15d21cd8a40000

Buildbucket says the build completed successfully, but Pinpoint can't find the isolate hash.
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/168b64bca40000

Buildbucket says the build completed successfully, but Pinpoint can't find the isolate hash.
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14913a6ca40000

Buildbucket says the build completed successfully, but Pinpoint can't find the isolate hash.
Labels: -Pri-3 M-69 Pri-1
Components: Internals>Compositing
The bisect result seems a bit suspicious, but if it's correct, I wonder if the synthetic-event changes in that CL are affecting the way the smoothness benchmark works.
I'm trying to test by just reverting the changes in the synthesizer, but I haven't had success running the benchmark (https://github.com/catapult-project/catapult/issues/4538)
I agree that the bisect result seems suspicious, as the changes don't change any behavior.
This CL https://chromium-review.googlesource.com/c/chromium/src/+/1126330 seems to bring back the mean_pixels_checkerboarded benchmark back to ~2.5% I'm only removing the argument from the synthesizer and hardcoding 0.

Any thoughts ajuma@?
It looks like that ends up changing the source argument we pass to MotionEvent.obtain. Reading through https://developer.android.com/reference/android/view/MotionEvent.html#device-types, maybe that changes the interpretation of the event? So maybe that changes the speed at which we're scrolling in the test?
That is not the case. The tests end up using the SyntheticGestureTarget, which uses 0 as source. So, the CL shouldn't change any behavior. Only the VR browser uses a different source, but the involved tests do not run in VR.
Maybe another direction would be to try minimizing https://chromium-review.googlesource.com/c/chromium/src/+/1126330 to the smallest change that fixes the benchmark? That might help for understanding what's going on.
TBH, this is the smallest change possible. But it already is equivalent to reverting the change for VR. On the other hand, I only introduced the type to remove some warnings in logcat for the missing source.
Status: Started (was: Assigned)
Adding a default source brings the perf back https://pinpoint-dot-chromeperf.appspot.com/job/12bde35fa40000
Project Member

Comment 27 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02cd150c24de8bdebb14d88c1a144b921a47756f

commit 02cd150c24de8bdebb14d88c1a144b921a47756f
Author: Aldo Culquicondor <acondor@chromium.org>
Date: Mon Aug 13 16:21:43 2018

Add default source to the MotionEventSynthetizer

Having an undefined source triggers warnings in the logs.
SOURCE_CLASS_POINTER is the minimal requirement for producing
touch events, and it is general enough. Passing the source as
an argument to `inject` was causing perf regressions.

BUG= 855626 ,  867438 

Change-Id: I05458c9c98776a2af99b570aba5760adfd0381d5
Reviewed-on: https://chromium-review.googlesource.com/1167207
Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582606}
[modify] https://crrev.com/02cd150c24de8bdebb14d88c1a144b921a47756f/chrome/android/java/src/org/chromium/chrome/browser/vr/AndroidUiGestureTarget.java
[modify] https://crrev.com/02cd150c24de8bdebb14d88c1a144b921a47756f/content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizerImpl.java
[modify] https://crrev.com/02cd150c24de8bdebb14d88c1a144b921a47756f/content/public/android/java/src/org/chromium/content/browser/SyntheticGestureTarget.java
[modify] https://crrev.com/02cd150c24de8bdebb14d88c1a144b921a47756f/content/public/android/java/src/org/chromium/content_public/browser/MotionEventSynthesizer.java

Status: Fixed (was: Started)

Sign in to add a comment