Issue metadata
Sign in to add a comment
|
11.8% regression in blink_perf.svg at 517978:518030 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Nov 22 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8962230298193055440
,
Nov 22 2017
=== Auto-CCing suspected CL author ikilpatrick@chromium.org === Hi ikilpatrick@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 : Ian Kilpatrick Commit : 98b46470f307a68c8183d628237d7b361af96244 Date : Tue Nov 21 01:22:24 2017 Subject: Add secure_context_mode_ to CSSParserContext. Bisect Details Configuration: android_webview_nexus6_aosp_perf_bisect Benchmark : blink_perf.svg Metric : Bamboo/Bamboo Change : 10.52% | 1454.81766667 -> 1607.86816667 Revision Result N chromium@517977 1454.82 +- 16.7504 6 good chromium@518004 1460.36 +- 16.0688 6 good chromium@518011 1456.55 +- 15.2178 6 good chromium@518014 1450.75 +- 11.3951 6 good chromium@518015 1449.48 +- 13.2347 6 good chromium@518016 1634.91 +- 19.5079 6 bad <-- chromium@518017 1632.9 +- 14.1436 6 bad chromium@518030 1607.87 +- 14.7934 6 bad Please refer to the following doc on diagnosing blink_perf regressions: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/benchmark_harnesses/blink_perf.md To Run This Test src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.svg More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8962230298193055440 For feedback, file a bug with component Speed>Bisection
,
Nov 23 2017
Issue 787767 has been merged into this issue.
,
Nov 23 2017
Issue 787769 has been merged into this issue.
,
Nov 24 2017
Issue 787855 has been merged into this issue.
,
Nov 24 2017
Issue 788327 has been merged into this issue.
,
Nov 24 2017
Issue 788363 has been merged into this issue.
,
Nov 24 2017
Issue 788378 has been merged into this issue.
,
Nov 24 2017
Issue 788348 has been merged into this issue.
,
Nov 25 2017
Issue 788380 has been merged into this issue.
,
Jan 5 2018
ikilpatrick: Can you please take a look? There seem to be a lot of performance regressions caused by this CL. Adding reviewers in case they have ideas.
,
Jan 25 2018
Sorry - this slipped off my radar. The context for this change is: https://groups.google.com/a/chromium.org/d/msg/blink-dev/Jex3idOld48/VUYks4U4BAAJ Basically as we decided for now to ship the CSS Paint API only on secure contexts, we needed to wire an additional bit (secure_context_mode_) into the CSS Parser. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/parser/CSSParserContext.h?q=cssparsercontext&sq=package:chromium&l=132 This is so that things like "background: paint(foo)" fails to parse on insecure contexts. Checking this bit itself is cheap, and only happens in a few cases (at the moment) but the performance regression come in as we need to pass in the right the document (or more generally the execution context) to determine if its secure or not. I.e. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/CSSGroupingRule.cpp?type=cs&l=69 The regression is coming from the bindings code, which now has to look up what the correct execution context is, to pass into the function. E.g. https://chromium-review.googlesource.com/c/chromium/src/+/759112/16/third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl The IsSecureContext check on ExecutionContext itself is _now_ cheap (it wasn't when this patch first landed, hence the improvement after a few days). This was fixed in: https://chromium.googlesource.com/chromium/src/+/9371bdbfccefb2e3eead2c017ef2c9c924dbc06c @haraken might have thoughts on how we can get rid of some overhead, i.e. lazily looking up an execution context from FunctionCallbackInfo??? But this seems like a bad maintenance burden.
,
May 15 2018
Any updates here?
,
Aug 27
Marking as fixed due to: https://chromium.googlesource.com/chromium/src/+/9371bdbfccefb2e3eead2c017ef2c9c924dbc06c |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 22 2017