Issue metadata
Sign in to add a comment
|
19.5% regression in blink_perf.paint at 462371:462400 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Apr 19 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8981887889941166688
,
Apr 20 2017
=== Auto-CCing suspected CL author shend@chromium.org === Hi shend@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 : shend Commit : 25809e1270b9eb4103f1efcc089c781df8465e39 Date : Thu Apr 06 06:58:14 2017 Subject: Generate ComputedStyle::hasViewportUnits and hasRemUnits. Bisect Details Configuration: android_nexus7_perf_bisect Benchmark : blink_perf.paint Metric : large-table-background-change-with-invisible-collapsed-borders/large-table-background-change-with-invisible-collapsed-borders Change : 31.28% | 706.157833333 -> 927.05125 Revision Result N chromium@462370 706.158 +- 21.8691 6 good chromium@462374 711.395 +- 22.2861 6 good chromium@462376 715.675 +- 11.1529 6 good chromium@462377 912.787 +- 9.31231 6 bad <-- chromium@462378 907.774 +- 26.2508 6 bad chromium@462385 918.342 +- 45.6486 6 bad chromium@462400 927.051 +- 11.515 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.paint Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8981887889941166688 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6333394616582144 | 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
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8981813874618225136
,
Apr 20 2017
=== Auto-CCing suspected CL author scottmg@chromium.org === Hi scottmg@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 : scottmg Commit : ffcbc0b2bda32309a8e799e146b934ce9b556b82 Date : Wed Apr 12 22:53:42 2017 Subject: Force PlzNav and LoadingWithMojo on when using network service Bisect Details Configuration: android_nexus7_perf_bisect Benchmark : blink_perf.paint Metric : large-table-background-change-with-invisible-collapsed-borders/large-table-background-change-with-invisible-collapsed-borders Change : 22.84% | 936.542833333 -> 722.636916667 Revision Result N chromium@464165 936.543 +- 15.5878 6 good chromium@464181 910.002 +- 31.3424 6 good chromium@464185 906.321 +- 9.7155 6 good chromium@464186 920.795 +- 17.3946 6 good chromium@464187 726.578 +- 19.3117 6 bad <-- chromium@464189 734.038 +- 17.6249 6 bad chromium@464197 707.17 +- 12.6937 6 bad chromium@464229 722.637 +- 29.7758 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.paint Debug Info https://chromeperf.appspot.com/buildbucket_job_status/8981813874618225136 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5910920091402240 | 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
-scottmg Sorry, was just doing a separate bisect to help figure out what's going on.
,
Apr 27 2017
Ran perf bots on a revert patch: https://codereview.chromium.org/2838513002 We would gain a 5% speedup on Nexus 7 with the revert. Interestingly, if we keep the generated members (which are unused) and add back the handwritten members in ComputedStyle: https://codereview.chromium.org/2828253004#ps00001 we would get a 17% speedup. I think there are two factors at work here. When we generate hasViewport and hasRem, it causes the data layout to change for ComputedStyleBase. It just so happens that the new data layout: - Moves the other fields around in a way that makes the test faster. - But makes accesses to hasViewport and hasRem slower. This is just a hypothesis and more investigation would be needed. Personally, I don't think this is urgent, as the graph has since gone back down and the regression seems to be only on this test. A possible fix would be to allow the order of fields to be specified (in a JSON file), but this would conflict with the automatic packing that we're currently doing. @meade, WDYT?
,
Apr 27 2017
I'm curious about what made the graph go down again - I couldn't see any obvious culprits in the range other than V8 rolls. Do you know? But likewise I don't think this is urgent.
,
Apr 27 2017
I did a bisect on what made the graph go down, and it's in comment #5. Doesn't appear to be relevant...
,
May 3 2017
Closing since the graph has gone back down and the regression seems to have disappeared.
,
Feb 16 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/chromeos-admin/+/bc0d6d92201bd68cf6d780b00239f465181ce296 commit bc0d6d92201bd68cf6d780b00239f465181ce296 Author: Prathmesh Prabhu <pprabhu@chromium.org> Date: Fri Feb 16 22:09:17 2018 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by alexclarke@chromium.org
, Apr 19 2017