=== Auto-CCing suspected CL author chongz@chromium.org ===
Hi chongz@chromium.org, the bisect results pointed to your CL, please take a look at the
results.
=== BISECT JOB RESULTS ===
Perf regression found with culprit
Suspected Commit
Author : chongz
Commit : 5a7f45dab8372cb7b80ee24dfb137a7ced2049ea
Date : Wed Mar 22 23:13:14 2017
Subject: Enable VsyncAlignedInput to field trial
Bisect Details
Configuration: android_nexus5X_perf_bisect
Benchmark : smoothness.top_25_smooth
Metric : first_gesture_scroll_update_latency/http___techcrunch.com
Change : 135.88% | 12.8131666667 -> 30.2231666667
Revision Result N
chromium@458906 12.8132 +- 0.76457 6 good
chromium@458915 12.7273 +- 1.91149 6 good
chromium@458920 13.274 +- 2.33544 6 good
chromium@458921 30.169 +- 3.04538 6 bad <--
chromium@458922 29.8082 +- 2.34259 6 bad
chromium@458924 30.4765 +- 2.85826 6 bad
chromium@458942 30.723 +- 3.07344 6 bad
chromium@458978 30.2232 +- 3.02183 6 bad
To Run This Test
src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...techcrunch.com smoothness.top_25_smooth
Debug Info
https://chromeperf.appspot.com/buildbucket_job_status/8984291138537333472
Is this bisect wrong?
https://chromeperf.appspot.com/bad_bisect?try_job_id=5239024823828480
| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
| X | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection. Thank you!
This regression is expected. (See CL description)
Reasons for the delay:
The synthetic input controller sends |TouchMove| right before |BeginFrame|, which
means the ACK will come back later and the generated |GestureScrollUpdate| will fall
into next |BeginFrame|.
This is fine since the actual scroll result won't be rendered until next frame with or
without this patch.
Before CL:
The matrix measures when the |GestureScrollUpdate| got ACKed.
After CL:
The matrix measures when the |GestureScrollUpdate| got rendered by cc.
Discussed with tdresser@ and here are some possible solutions. Will keep investigating on which one we should use. (Maybe 3?)
1. Accept the latency
2. Don't queue GestureScrollUpdate caused by blocking events
* How to check? Touch vs Wheel?
3. Add real-life test cases so the extra latency would be roughly half a frame
4. Add a delay when processing Gesture Events, e.g. Process them 10ms after receiving VSync signal
* This is for the concern that Gesture Events missed BeginFrame might actually be able to catch GPUSwap.
Re #2: We'd need to add an extra bit on the gestures, indicating if they came from a blocking or non-blocking event. The gestures aren't dispatched until they're acked, and the ack indicates if they're blocking or not.
I think a combination of #3 and #2 sounds good for now. Improving the tests is good either way, and #2 is a nice baby step towards shipping this for all events.
=== BISECT JOB RESULTS ===
Perf regression found with culprit
Suspected Commit
Author : chongz
Commit : 5a7f45dab8372cb7b80ee24dfb137a7ced2049ea
Date : Wed Mar 22 23:13:14 2017
Subject: Enable VsyncAlignedInput to field trial
Bisect Details
Configuration: mac_retina_perf_bisect
Benchmark : smoothness.tough_scrolling_cases
Metric : mean_pixels_checkerboarded/text_constant_full_page_raster_60000_pixels_per_second
Change : 712.66% | 11.7818333333 -> 95.746
Revision Result N
chromium@458826 11.7818 +- 9.71105 6 good
chromium@458899 14.8065 +- 11.4286 6 good
chromium@458918 10.6058 +- 13.6799 6 good
chromium@458920 15.6405 +- 13.9584 6 good
chromium@458921 95.7043 +- 1.60753 6 bad <--
chromium@458923 95.224 +- 2.71684 6 bad
chromium@458927 95.7562 +- 1.35899 6 bad
chromium@458936 95.8557 +- 1.78812 6 bad
chromium@458972 95.746 +- 3.13823 6 bad
To Run This Test
src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=text.constant.full.page.raster.60000.pixels.per.second smoothness.tough_scrolling_cases
Debug Info
https://chromeperf.appspot.com/buildbucket_job_status/8983660499699115504
Is this bisect wrong?
https://chromeperf.appspot.com/bad_bisect?try_job_id=5334547983499264
| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
| X | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection. Thank you!
=== BISECT JOB RESULTS ===
Perf regression found with culprit
Suspected Commit
Author : chongz
Commit : 5a7f45dab8372cb7b80ee24dfb137a7ced2049ea
Date : Wed Mar 22 23:13:14 2017
Subject: Enable VsyncAlignedInput to field trial
Bisect Details
Configuration: mac_retina_perf_bisect
Benchmark : smoothness.tough_scrolling_cases
Metric : mean_pixels_checkerboarded/mean_pixels_checkerboarded
Change : 786.36% | 4.1673875 -> 36.9382208333
Revision Result N
chromium@458826 4.16739 +- 0.740542 6 good
chromium@458899 3.6985 +- 1.2594 6 good
chromium@458918 3.73524 +- 0.936102 6 good
chromium@458920 3.69328 +- 1.37851 6 good
chromium@458921 36.9771 +- 0.392737 6 bad <--
chromium@458923 36.9431 +- 0.512035 6 bad
chromium@458927 36.8661 +- 0.417049 6 bad
chromium@458936 36.9327 +- 0.332757 6 bad
chromium@458972 36.9382 +- 0.272775 6 bad
To Run This Test
src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests smoothness.tough_scrolling_cases
Debug Info
https://chromeperf.appspot.com/buildbucket_job_status/8983660485948251232
Is this bisect wrong?
https://chromeperf.appspot.com/bad_bisect?try_job_id=6630266971357184
| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
| X | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection. Thank you!
The checkerboarding regression is definitely not good, since one of the long term goals of this project is to reduce checkerboarding.
Dana or Enne, any thoughts?
I think this may be because we should be using a constant offset from vsync, instead of the vsync signal itself?
I don't understand how we'd get more checkerboarding by changing things in the compositor further from the frame deadline, unless this is happening to batch everything and deliver it at the deadline instead of at the start of the next frame?
We should be delivering input to the compositor based on the BeginImplFrame happening, that's when the compositor starts doing math, preparing tiles, etc. I don't think it should need to worry anything about vsyncs.
GestureEvents being processed are based on BeginImplFrame occurring.
However the touchmove is being generated on the BeginFrame signal in the browser and it (the touchmove) must be ack'd before the gesture scroll update is release from the browser.
I presume we have something like this:
Old Way:
Begin Frame
Touch Move IPC sent
Begin Frame IPC sent
Touch Move handled
Touch Move ACK
Gesture Scroll Update IPC sent
Gesture Scroll Update handled
Frame deadline/Swap (latency recorded)
Now we have:
Begin Frame
Touch Move IPC sent
Begin Frame IPC sent
Touch Move handled
Touch Move ACK
Gesture Scroll Update IPC sent
Frame deadline/Swap (latency *not* recorded)
BeginFrame
Gesture Scroll Update handled
Frame deadline/Swap (latency recorded)
Can we not grab traces of this so we aren't speculating what happens?
I don't understand why checker-boarding increases but I think some detailed traces we all can look at is likely the best scenario in understanding what is happening.
I would expect latency to increase when not delivering input events whenever they arrive. I would not expect an increase in checkerboarding.
I don't really have a sense of why this would be happening, so I think some traces and debugging would need to be done to figure out more, sorry.
I just reverted my CL in https://crrev.com/2791663002, wangxianzhu@ can you help verify if it's still blocking your change?
Thanks!
--
I will try to find the root cause first before re-landing the Field Trial.
Comment 1 by jasontiller@chromium.org
, Mar 23 2017