Issue metadata
Sign in to add a comment
|
10% regression in blink_perf.css at 441275:441328 |
||||||||||||||||||||||
Issue descriptionLinks below.
,
Jan 9 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8990905873106299680
,
Jan 10 2017
=== Auto-CCing suspected CL author timloh@chromium.org === Hi timloh@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 : timloh Commit : ed0f6121435448aa316c90be1468398ab2ef71b4 Date : Wed Jan 04 03:49:42 2017 Subject: Disallow setting invalid values for registered properties via CSSOM Bisect Details Configuration: android_nexus7_perf_bisect Benchmark : blink_perf.css Metric : CSSPropertySetterGetterMethods/CSSPropertySetterGetterMethods Change : 7.01% | 416.92268723 -> 387.709572622 Revision Result N chromium@441274 416.923 +- 65.4449 30 good chromium@441301 421.369 +- 28.809 30 good chromium@441308 425.976 +- 44.3575 30 good chromium@441312 417.459 +- 30.1937 30 good chromium@441313 378.139 +- 23.7062 30 bad <-- chromium@441314 379.962 +- 21.6213 30 bad chromium@441315 389.93 +- 23.3286 30 bad chromium@441328 387.71 +- 25.611 30 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/8990905873106299680 Is this bisect wrong? https://chromeperf.appspot.com/bad_bisect?try_job_id=5904120818434048 | 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 Tests>AutoBisect. Thank you!
,
Jan 10 2017
Bisect looks accurate. Maybe the extra cost of bindings for the execution context plus passing it around? :/
,
Jan 10 2017
Are there measurable tradeoffs for your CL? If we could justify this with an improvement in performance in some other area, we can negotiate. Also adding Tim in case he has ideas.
,
Jan 10 2017
I might just revert this for now and then investigate :/
,
Jan 11 2017
Was sheriffing today/yesterday but will investigate and revert or try to fix tomorrow.
,
Jan 12 2017
Patch to fix is up for review https://codereview.chromium.org/2625303002/, perf bot suggests it fixed the regression.
,
Jan 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521 commit 2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521 Author: timloh <timloh@chromium.org> Date: Thu Jan 12 09:00:32 2017 Fix CSSPropertySetterGetterMethods regression A recent patch crrev.com/2607403002 introduced a 10% regression in setters and getters for CSSOM style objects. This patch fixes this by removing the additional binding to access the registry, instead using already existing members to access it. BUG= 679511 Review-Url: https://codereview.chromium.org/2625303002 Cr-Commit-Position: refs/heads/master@{#443188} [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/bindings/core/v8/custom/V8CSSStyleDeclarationCustom.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.h [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/CSSStyleDeclaration.h [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/PropertySetCSSStyleDeclaration.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/css/PropertySetCSSStyleDeclaration.h [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/editing/VisiblePositionTest.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/editing/commands/RemoveCSSPropertyCommand.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp [modify] https://crrev.com/2dcda31b3f6d9e8cf588f0c233d9c8c7cb9b4521/third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp
,
Jan 12 2017
Thanks!
,
Jan 12 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by benhenry@google.com
, Jan 9 2017