Issue metadata
Sign in to add a comment
|
10.2% regression in blink_perf.css at 464868:464873 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Apr 19 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8981888508982947744
,
Apr 19 2017
=== Auto-CCing suspected CL author leszeks@chromium.org === Hi leszeks@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 : Leszek Swirski Commit : 0010be5b23f5d72744dcf5b11113568e1916d2e9 Date : Thu Apr 13 13:41:26 2017 Subject: [compiler] Always use deopt count for disabling optimization Bisect Details Configuration: android_nexus6_perf_bisect Benchmark : blink_perf.css Metric : SelectorCountScaling/SelectorCountScaling Change : 11.76% | 21.2471897173 -> 18.7495559415 Revision Result N chromium@464867 21.2472 +- 0.333452 6 good chromium@464870 21.0376 +- 0.297957 6 good chromium@464871 21.0309 +- 0.418338 6 good chromium@464871,v8@d9f6dd896c 21.6359 +- 0.366762 6 good chromium@464871,v8@dc83caa6fb 21.6209 +- 0.406186 6 good chromium@464871,v8@c4b02905d8 21.882 +- 0.48591 6 good chromium@464871,v8@0010be5b23 19.1127 +- 0.387841 6 bad <-- chromium@464871,v8@38c3b71c26 18.7801 +- 0.677582 6 bad chromium@464871,v8@204989a5aa 18.72 +- 0.713421 6 bad chromium@464871,v8@e8cfc5b7ca 18.6583 +- 0.429928 6 bad chromium@464872 18.6539 +- 0.572252 6 bad chromium@464873 18.7496 +- 0.675128 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 blink_perf.css Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8981888508982947744 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6462193370923008 | 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!
,
Apr 20 2017
There's enough improvements to the same benchmark on other bots that I think this is WontFix material. WDYT?
,
Apr 20 2017
Well the other bots appear to be all desktop ones where this change was a perf win. I presume we care about android perf too.
,
Apr 20 2017
Good point -- looking at graphs of other mobile devices, it seems like they may be encountering a similar issue. I'll check if we're doing anything obviously wrong here, but I worry that it may just be a "you win some, you lose some" heuristic change.
,
Apr 24 2017
+rmcilroy
,
Apr 24 2017
This benchmark is pretty micro-benchmarky and is focused on CSS performance, so I agree with Leszek that this is probably just a heuristic change which causes this particular code sequence to end up optimizing the JS at a different point that it would otherwise (which happens to help desktop but hurt mobile for this particularly microbenchmark). It's worth checking we aren't doing anything seriously wrong, but if not then I think we can wontfix this (particularly since we don't see any regressions in more real-world examples on mobile).
,
Jul 27 2017
Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md We're looking for one of the following: 1. Justification via explanation 2. Plan to revert or fix 3. Angry rage throwing of equipment at my head Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it. Note: This was a bulk edit message and not very personal.
,
Jul 28 2017
This will go away with https://chromium-review.googlesource.com/c/588894/
,
Jul 28 2017
,
Aug 4 2017
Looks like the fix worked. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by alexclarke@chromium.org
, Apr 19 2017