Huge regression in power required to scroll on Mac didn't regress system_health.common_desktop |
||||||
Issue descriptionThis weekend, our infrastructure caught a huge (70%) regression in the power required to scroll on Mac. (Yay!) https://chromeperf.appspot.com/report?sid=983b62c109ccda5810e6ef5be8940f9bcd52c784267edd1c5d2cfc018e6331f8 Unfortunately, the regression was caught by battor.trivial_pages but not any of the system health benchmarks. (Boo!) https://chromeperf.appspot.com/report?sid=d0479aa292343a0743c24fdc7d0292f01eaadcc1c61a8906ca371a39065063cd Randy, could you look into why this might be? Are the system health benchmarks actually scrolling? We can tell just by playing around that apparently only the scrolling trivial_pages benchmarks actually saw the regression: https://chromeperf.appspot.com/report?sid=38753598b0fc828dbf274231ea0792c7f8ba427771256ec0a17754232a1c69a9
,
Nov 15 2016
,
Nov 15 2016
nednguyen@ and I talked about this with tdresser@ at dinner yesterday, and he suggested that it might have something to do with main thread scrolling versus compositor thread scrolling.
,
Nov 15 2016
I can confirm that these pages scroll when running locally on my linux machine: browse:news:cnn browse:news:flipboard <stopped looking after this because it was obvious after only a few pages that scrolling isn't uncommon in system health browsing stories> What I dont see them doing is scrolling then waiting. They scroll, then switch to the next action immediately. It typically looks like this for the news sites. <initial page load> <scroll down to link, if needed> <click link> <scroll down if needed> <scroll back up> <go back> <rinse and repeat> Running battor.trivial_pages locally with --story-filter="TrivialScrollingPageSharedPageState" I see that it is scrolling very slowly the entire time. These two behaviors are drastically different. Since there is very little down time from scrolling, and the scrolling for the news articles is much faster, I'm guessing this may be why we are seeing a difference in what they catch. I will get a CL up that I think will fix this gap and we can test locally if it helps.
,
Nov 15 2016
It also worths verifying Tim's theory about main thread scrolling vs compositor thread scrolling. The easy way to verify this is to enable whichever tracing categories that output events that represent main thread scrolling. Then you can hack the power metric to throw exception when it finds trace events that represents main thread scrolling. Finally, if you run the system health benchmark and the exception doesn't get thrown --> means out system health stories don't have main thread scroll.
,
Nov 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11a42166b7ca5f2ac4ff371110d41054a2d1dc3e commit 11a42166b7ca5f2ac4ff371110d41054a2d1dc3e Author: rnephew <rnephew@chromium.org> Date: Tue Nov 15 23:30:34 2016 [System Health] Change scrolling behaviour for news sites. They currently would wait 3 seconds, then scroll then go back to the main site. There was an issue where after scrolling, the scroll bar would constantly rewrite. This moves half the wait period to after the scrolling, so that it has time to sit idle after scrolling *** TO PERF SHERRIFS *** The change in default behaviour for the news test cases may cause regressions. These are likely not real regressions, and we just need to establish a new baseline. BUG= 665220 Review-Url: https://codereview.chromium.org/2510443002 Cr-Commit-Position: refs/heads/master@{#432288} [modify] https://crrev.com/11a42166b7ca5f2ac4ff371110d41054a2d1dc3e/tools/perf/page_sets/system_health/browsing_stories.py
,
Nov 16 2016
I can confirm that the trivial scrolling page uses main thread scrolling. Can't comment further without knowing more details about the actual theory.
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fbdd4cf5f8069eb223fc72e6298a97854abd0374 commit fbdd4cf5f8069eb223fc72e6298a97854abd0374 Author: rnephew <rnephew@chromium.org> Date: Wed Nov 16 18:03:33 2016 [System Health] Change from integer division to float division for News Stories A CL Yesterday changed the waiting pattern on news sites to do half before the scroll and half after. It is using int math instead of float math so it is waiting longer than it should. BUG= 665220 Review-Url: https://codereview.chromium.org/2505033002 Cr-Commit-Position: refs/heads/master@{#432551} [modify] https://crrev.com/fbdd4cf5f8069eb223fc72e6298a97854abd0374/tools/perf/page_sets/system_health/browsing_stories.py
,
Nov 17 2016
Charlie has a laptop with a BattOr and he said he would check if this change to the benchmark help catch this type of regression. Reassigning to him for this.
,
Nov 18 2016
Bisect running at https://codereview.chromium.org/2514003002.
,
Nov 18 2016
Looks like I did something wrong with that. Also it wans't a bisect, it was a tryjob.
,
Nov 18 2016
Here is the first tryjob results: http://storage.googleapis.com/chromium-telemetry/html-results/results-2016-11-18_14-37-20 I am starting another one with a change that makes it sit longer after scrolling. That might not be as useful though, since it will not have the extra length in the run before it. I'm not 100% sure how to interpret these results, the power metrics have changed quite a bit recently. The mousewheel_response energy seems to be drastically higher though. NAME ToT Patch css_animation:power 33.195 W 31.453 W idle:power 19.922 W 19.106 W load:energy 0.000 J 0.000 J mousewheel_animation:power 28.396 W 28.782 W mousewheel_response:energy 9.013 J 23.269 J scroll_response:energy 210.217 J 216.235 J story:power 20.551 W 19.700 W
,
Nov 22 2016
The mousewheel_response:energy seems promising. Can we validate whether this metric account for most regression in https://chromeperf.appspot.com/report?sid=983b62c109ccda5810e6ef5be8940f9bcd52c784267edd1c5d2cfc018e6331f8 ?
,
Jun 22 2017
I think we missed the boat on investigating this further, but I still think we should keep an eye on these types of failures to catch regressions in the future. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by charliea@chromium.org
, Nov 15 2016