Issue metadata
Sign in to add a comment
|
1.1%-35.6% regression in system_health.memory_mobile at 439070:439094 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Dec 19 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8992862572356136864
,
Dec 19 2016
=== PERF REGRESSION === === Auto-CCing suspected CL author rune@opera.com === Hi rune@opera.com, the bisect results pointed to your CL, please take a look at the results. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Reland: Collect active stylesheets and and apply asynchronously. Author : rune Commit description: This CL enables asynchronously updating the lists of active stylesheets applying any style changes using rule set invalidations. This means we more often avoid full style recalcs when we add or remove stylesheets from the document as well as when the evaluation of media queries change. In general, we now alway compare new and old stylesheets by comparing their rulesets and schedule style invalidations for removed and added rulesets. When media queries changes, we used to give completely in and recalculate all style once we discovered a media query changed its evaluation. With this patch, we clear rule sets for sheets which contain media queries which means we will invalidate rules for the sets before and after the query change. This can be further refined by only clearing rule sets when the sheets has a media query which actually did change evaluation, and also just schedule invalidations for rules which are inside @media rules. TreeScopeStyleSheetCollectionTest.cpp is removed as it is replaced by ActiveStyleSheetsTest.cpp which landed earlier. updateActiveStyle() has been added a few places where ensureStyleResolver() previously caused active stylesheets to be up-to- date. ensureStyleResolver() is now merely a method which creates the StyleResolver if necessary and returns it. There are some cleanups and code removal which needs to be done after this CL, but I have left those out to make this CL as small as possible. For instance resolverChanged(), which synchronously updated the active stylesheets, has an empty implementation instead of including a lot of removals in this CL. The code for lazy-appending stylesheets in StyleResolver is still there, but not in use. R=meade@chromium.org BUG= 567021 Committed: https://crrev.com/9fb5b60edfb769134733009f9447bad3eaf347b0 Review-Url: https://codereview.chromium.org/2557533005 Cr-Original-Commit-Position: refs/heads/master@{#438148} Cr-Commit-Position: refs/heads/master@{#439092} Commit : 90d4ea3d543f0031769b3aacac2d3e084b95fb7d Date : Fri Dec 16 12:25:03 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@439069 14023851 74215.9 6 good chromium@439082 14043136 49449.8 6 good chromium@439088 14036139 53374.2 6 good chromium@439091 14067883 140085 6 good chromium@439092 19308373 141656 6 bad <-- chromium@439093 19257856 201313 6 bad chromium@439094 19269803 115266 6 bad Bisect job ran on: android_nexus6_perf_bisect Bug ID: 675533 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.tools.maps system_health.memory_mobile Test Metric: memory:chrome:all_processes:reported_by_os:system_memory:java_heap:proportional_resident_size_avg/load_tools/load_tools_maps Relative Change: 37.41% Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2837 Job details: https://chromeperf.appspot.com/buildbucket_job_status/8992862572356136864 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5213930744446976 | 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!
,
Dec 19 2016
,
Dec 20 2016
,
Dec 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a865f6c19c00f0ba9e021c4871b53d2e115a2b00 commit a865f6c19c00f0ba9e021c4871b53d2e115a2b00 Author: rune <rune@opera.com> Date: Tue Dec 20 23:52:11 2016 Clear CSSGlobalRuleSet on StyleEngine::didDetach(). This could free up memory sooner. Found while investigating 675533, but not confirmed that this fixes that issue. BUG= 675533 Review-Url: https://codereview.chromium.org/2589243002 Cr-Commit-Position: refs/heads/master@{#439935} [modify] https://crrev.com/a865f6c19c00f0ba9e021c4871b53d2e115a2b00/third_party/WebKit/Source/core/css/CSSGlobalRuleSet.cpp [modify] https://crrev.com/a865f6c19c00f0ba9e021c4871b53d2e115a2b00/third_party/WebKit/Source/core/css/CSSGlobalRuleSet.h [modify] https://crrev.com/a865f6c19c00f0ba9e021c4871b53d2e115a2b00/third_party/WebKit/Source/core/dom/StyleEngine.cpp
,
Dec 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b0f03812c4f8ca50a0cc07caaca0c8eafbb2862a commit b0f03812c4f8ca50a0cc07caaca0c8eafbb2862a Author: rune <rune@opera.com> Date: Wed Dec 21 01:55:16 2016 Clear active tree scopes on StyleEngine::didDetach(). clearResolver() is not only called on didDetach(). Make it private and name it clearResolvers to reflect that it clears scoped resolvers as well. The comments related to style resolver reconstructruction is removed as that does not happen anymore. Clearing m_treeBoundaryCrossingScopes is moved into didDetach() which was a more natural place. Clear active and dirty tree scopes in didDetach to not unnecessarily hang on to any memory associated with them. These changes were done investigating issue 675533 , but won't necessarily fix anything for that issue. BUG= 567021 , 675533 Review-Url: https://codereview.chromium.org/2593643002 Cr-Commit-Position: refs/heads/master@{#439973} [modify] https://crrev.com/b0f03812c4f8ca50a0cc07caaca0c8eafbb2862a/third_party/WebKit/Source/core/dom/StyleEngine.cpp [modify] https://crrev.com/b0f03812c4f8ca50a0cc07caaca0c8eafbb2862a/third_party/WebKit/Source/core/dom/StyleEngine.h
,
Aug 16 2017
rune: Any more work planned on this bug, or should we close?
,
Aug 16 2017
I was not able to figure out how to reproduce and I never understood how my change could affect the java heap size. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by jasontiller@chromium.org
, Dec 19 2016