Issue metadata
Sign in to add a comment
|
17.2% regression in blink_perf.paint at 457730:457783 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 24 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8984285307169843424
,
Mar 24 2017
=== Auto-CCing suspected CL author nhiroki@chromium.org === Hi nhiroki@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 : nhiroki Commit : 1d85fa7d0be347292a5805ec87ca3d2d10a8c57f Date : Fri Mar 17 12:24:56 2017 Subject: Bindings: Remove DOMWrapperWorld::fromWorldId() for cleanup Bisect Details Configuration: android_nexus5_perf_bisect Benchmark : blink_perf.paint Metric : complex-content-slow-scroll/complex-content-slow-scroll Change : 16.02% | 38.1773333333 -> 44.2944166667 Revision Result N chromium@457729 38.1773 +- 1.49483 6 good chromium@457736 37.4615 +- 1.7369 6 good chromium@457738 37.6928 +- 1.76342 6 good chromium@457739 43.353 +- 1.95136 6 bad <-- chromium@457740 43.347 +- 1.62071 6 bad chromium@457743 43.0798 +- 0.775813 6 bad chromium@457756 44.2807 +- 3.27388 6 bad chromium@457783 44.2944 +- 2.77377 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/8984285307169843424 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=6406144605552640 | 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!
,
Mar 27 2017
I'll take a look... CC to wangxianzhu@ who is the author of the test: https://chromium.googlesource.com/chromium/src/+/fd7fc207884807c1b5365c191cf6d5cb55e33f4c
,
Mar 27 2017
Note that my culprit CL is just for refactoring, so I'll revert it if it's difficult to fix.
,
Mar 27 2017
Hm..., probably changing the hash traits for WTF::HashMap and/or adding the main world into the world map changed the performance? I'll try to revert that part.
,
Mar 27 2017
I'm not sure but let's try to revert it.
,
Mar 27 2017
,
Mar 27 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8983994804957104400
,
Mar 27 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : nhiroki Commit : 1d85fa7d0be347292a5805ec87ca3d2d10a8c57f Date : Fri Mar 17 12:24:56 2017 Subject: Bindings: Remove DOMWrapperWorld::fromWorldId() for cleanup Bisect Details Configuration: android_nexus5_perf_bisect Benchmark : blink_perf.paint Metric : complex-content-slow-scroll/complex-content-slow-scroll Change : 14.83% | 38.273 -> 43.9481666667 Revision Result N chromium@457710 38.273 +- 1.58094 6 good chromium@457729 38.0792 +- 0.946489 6 good chromium@457738 38.1372 +- 0.832841 6 good chromium@457739 43.6574 +- 3.03112 6 bad <-- chromium@457740 43.8799 +- 2.74143 6 bad chromium@457741 42.6599 +- 2.33508 6 bad chromium@457743 42.78 +- 1.29046 6 bad chromium@457747 42.9478 +- 0.784935 6 bad chromium@457783 43.9482 +- 1.84136 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/8983994804957104400 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5811138224193536 | 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!
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e5d3fc45d06833189606b0ad750b9dee95ef50a commit 3e5d3fc45d06833189606b0ad750b9dee95ef50a Author: nhiroki <nhiroki@chromium.org> Date: Wed Mar 29 05:53:18 2017 Bindings: Manage the main world separately from other worlds This CL speculatively reverts a part of the CL[1]. The CL[1] pushed the main world into the world map for cleanup, but that CL seems to cause a performance regression on a perf test (see issue 704778 ) probably because of changing the hashmap traits and/or the world map management. [1] https://codereview.chromium.org/2753013003/ BUG= 697622 , 697629 , 704778 Review-Url: https://codereview.chromium.org/2777763003 Cr-Commit-Position: refs/heads/master@{#460294} [modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorldTest.cpp
,
Mar 29 2017
A possible fix was landed. I'll monitor the graph for a while...
,
Mar 29 2017
After the patch, results of the perf test returned to the original level. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jasontiller@chromium.org
, Mar 24 2017