Make inheritance faster |
||||||||||
Issue descriptionWhen any inherited property changes stylePropagationDiff will return Inherit. This causes the engine to recalc the style for the entire subtree until we find a node where the stylePropagationDiff is NoChange. When there’s internally inherited properties this is a bit more complex and we continue to compute style down the tree. Unfortunately this means that if you change a property like pointer-events: none high up in the tree to disable mouse interaction we end up doing a recalc style of most of the page. ex. <body> <div id=”container”> … thousand elements here … </div> <div id=”dialog”></div> The page might do $(“container”).pointerEvents = “none” while the #dialog is displayed, but this causes the thousands of elements inside #container to go through a style recalc. Worse the StyleResolver itself has no concept of an Inherit style change so as we propagate this change down the tree we run selector matching and lots of other work again too. Optimally for properties like pointer-events we’d just walk the entire subtree applying pointer-events: none to every level of the tree and updating the LayoutObjects instead of going through the full selector matching, cascade, update step. Design doc: https://docs.google.com/document/d/1wiHZXkBbL1mz2ZIxNqWMSbJbmKJdw_yuDgh8oSKwOpM/edit?ts=5747fd77&pref=2&pli=1
,
Jul 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c65d0c0a618449f58c3824992f14c458cd2b5830 commit c65d0c0a618449f58c3824992f14c458cd2b5830 Author: sashab <sashab@chromium.org> Date: Wed Jul 06 06:51:56 2016 Clean up more naming in ComputedStyle Split this into two patches because the first one was getting too big. First patch: crrev.com/2115803002 Some more small naming fixes in ComputedStyle. - Renamed 'inherited' to 'm_styleInheritedData' - Renamed 'inherited_data' and 'noninherited_data' to 'm_inheritedData' and 'm_nonInheritedData' - Some other small fields changed to start with m BUG=622138 Review-Url: https://codereview.chromium.org/2114873002 Cr-Commit-Position: refs/heads/master@{#403858} [modify] https://crrev.com/c65d0c0a618449f58c3824992f14c458cd2b5830/third_party/WebKit/Source/core/style/ComputedStyle.cpp [modify] https://crrev.com/c65d0c0a618449f58c3824992f14c458cd2b5830/third_party/WebKit/Source/core/style/ComputedStyle.h
,
Aug 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f24dba9f04dd093aac4298378c671ecd44d0fe97 commit f24dba9f04dd093aac4298378c671ecd44d0fe97 Author: sashab <sashab@chromium.org> Date: Tue Aug 02 05:44:14 2016 Add a fast-path for independent inherited properties Add a fast-path for inherited properties which do not depend on and do not affect any other properties on ComputedStyle. When these properties are modified in a parent element, set them directly on ComputedStyle and skip doing a full recalc for elements only affected by this change. Also implemented two of these properties: visibility and pointer-events, storing an extra 2 bits per ComputedStyle. This increases the size of ComputedStyle by 1 byte on Windows and some Android builds (due to aligned fields), which increases the memory usage for a standard page with ~1000 elements by up to 1kb (although potentially up to 4/8kb on 32/64 bit builds due to packing, although this depends on the allocator implementation details) but realistically less since style sharing only creates one ComputedStyle object for each unique style. Benchmarks show a speed increase of up to 2x for setting these properties on the root element of a typical web page (Facebook, Twitter, Pinterest, Amazon, Wikipedia) and letting the change propagate directly onto the child ComputedStyle objects, rather than doing a full style recalc. Initial Benchmarks: https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=1597242813 Follow-up Benchmarks: https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=918856082 BUG=622138 Review-Url: https://codereview.chromium.org/2117143003 Cr-Commit-Position: refs/heads/master@{#409143} [add] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-multiple-properties.html [add] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path.html [add] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/LayoutTests/fast/css/invalidation/non-independent-inheritance-identical-computed-styles.html [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/build/scripts/css_properties.py [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/build/scripts/make_style_builder.py [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/css/CSSProperties.in [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.cpp [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.h [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/dom/Element.h [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/dom/Node.h [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/style/ComputedStyle.cpp [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/style/ComputedStyle.h [modify] https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
,
Aug 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf9f7aeb812949577490b55e40dd36c8eca7b8f9 commit bf9f7aeb812949577490b55e40dd36c8eca7b8f9 Author: rune <rune@opera.com> Date: Fri Aug 05 11:22:13 2016 Revert of Add a fast-path for independent inherited properties (patchset #13 id:240001 of https://codereview.chromium.org/2117143003/ ) Reason for revert: Caused issues 634254 and 633859 . Original issue's description: > Add a fast-path for independent inherited properties > > Add a fast-path for inherited properties which do not depend on and do > not affect any other properties on ComputedStyle. When these properties > are modified in a parent element, set them directly on ComputedStyle and > skip doing a full recalc for elements only affected by this change. > > Also implemented two of these properties: visibility and pointer-events, > storing an extra 2 bits per ComputedStyle. This increases the size of > ComputedStyle by 1 byte on Windows and some Android builds (due to > aligned fields), which increases the memory usage for a standard page > with ~1000 elements by up to 1kb (although potentially up to 4/8kb on > 32/64 bit builds due to packing, although this depends on the allocator > implementation details) but realistically less since style sharing only > creates one ComputedStyle object for each unique style. > > Benchmarks show a speed increase of up to 2x for setting these > properties on the root element of a typical web page (Facebook, Twitter, > Pinterest, Amazon, Wikipedia) and letting the change propagate directly > onto the child ComputedStyle objects, rather than doing a full style > recalc. > > Initial Benchmarks: > https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=1597242813 > > Follow-up Benchmarks: > https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=918856082 > > BUG=622138 > > Committed: https://crrev.com/f24dba9f04dd093aac4298378c671ecd44d0fe97 > Cr-Commit-Position: refs/heads/master@{#409143} TBR=esprehn@chromium.org,meade@chromium.org,timloh@chromium.org,sashab@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=622138 Review-Url: https://codereview.chromium.org/2213223004 Cr-Commit-Position: refs/heads/master@{#410030} [delete] https://crrev.com/752d6e61e682886b2347b8626be9be879a536994/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-multiple-properties.html [delete] https://crrev.com/752d6e61e682886b2347b8626be9be879a536994/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path.html [delete] https://crrev.com/752d6e61e682886b2347b8626be9be879a536994/third_party/WebKit/LayoutTests/fast/css/invalidation/non-independent-inheritance-identical-computed-styles.html [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/build/scripts/css_properties.py [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/build/scripts/make_style_builder.py [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/css/CSSProperties.in [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.cpp [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.h [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/dom/Element.h [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/dom/Node.h [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/style/ComputedStyle.cpp [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/style/ComputedStyle.h [modify] https://crrev.com/bf9f7aeb812949577490b55e40dd36c8eca7b8f9/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
,
Aug 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0c57905368c76aba662dbda3b4e13cb9bd454bf commit d0c57905368c76aba662dbda3b4e13cb9bd454bf Author: sashab <sashab@chromium.org> Date: Mon Aug 22 04:52:21 2016 Add a fast-path for independent inherited properties (Note: This patch was reverted in crrev.com/2213223004 because of issues crbug.com/633859 and crbug.com/634254 . These issues were caused from setting isInherited to false instead of true for the StyleBuilder generated inherit functions, and a slotting bug from skipping child recalc in HTMLSlotElement and InsertionPoints, which have both now been fixed in this patch & have tests added.) Add a fast-path for inherited properties which do not depend on and do not affect any other properties on ComputedStyle. When these properties are modified in a parent element, set them directly on ComputedStyle and skip doing a full recalc for elements only affected by this change. Also implemented two of these properties: visibility and pointer-events, storing an extra 2 bits per ComputedStyle. This increases the size of ComputedStyle by 1 byte on Windows and some Android builds (due to aligned fields), which increases the memory usage for a standard page with ~1000 elements by up to 1kb (although potentially up to 4/8kb on 32/64 bit builds due to packing, although this depends on the allocator implementation details) but realistically less since style sharing only creates one ComputedStyle object for each unique style. Benchmarks show a speed increase of up to 2x for setting these properties on the root element of a typical web page (Facebook, Twitter, Pinterest, Amazon, Wikipedia) and letting the change propagate directly onto the child ComputedStyle objects, rather than doing a full style recalc. Initial Benchmarks: https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=1597242813 Follow-up Benchmarks: https://docs.google.com/spreadsheets/d/1mUuJEs8cPWyNTR7tQw27oxq6fDTvWiAwgatf_g--B4w/edit#gid=918856082 BUG=622138 Review-Url: https://codereview.chromium.org/2220873002 Cr-Commit-Position: refs/heads/master@{#413406} [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-inherit-keyword.html [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-initial-keyword.html [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-multiple-properties.html [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path-slots.html [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path.html [add] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/LayoutTests/fast/css/invalidation/non-independent-inheritance-identical-computed-styles.html [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/build/scripts/css_properties.py [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/build/scripts/make_style_builder.py [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/css/CSSProperties.in [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.cpp [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/css/resolver/StyleResolverStats.h [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/dom/Element.h [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/dom/Node.h [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/style/ComputedStyle.cpp [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/style/ComputedStyle.h [modify] https://crrev.com/d0c57905368c76aba662dbda3b4e13cb9bd454bf/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
,
Oct 11 2016
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd928558b52c794c649fc064a82151bd9171ee65 commit bd928558b52c794c649fc064a82151bd9171ee65 Author: napper <napper@chromium.org> Date: Thu Dec 01 05:14:14 2016 Made CSS white-space property use IndependentInherit Made CSS white-space property use IndependentInherit, which is part of the process of making property inheritance faster. This change adds 1 bit to the size of ComputedStyle but significantly reduces the time for a recalc for independent-only changes involving whitespace. Tested using: third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path.html BUG=622138 Review-Url: https://codereview.chromium.org/2538983002 Cr-Commit-Position: refs/heads/master@{#435559} [modify] https://crrev.com/bd928558b52c794c649fc064a82151bd9171ee65/third_party/WebKit/LayoutTests/fast/css/invalidation/independent-inheritance-fast-path.html [modify] https://crrev.com/bd928558b52c794c649fc064a82151bd9171ee65/third_party/WebKit/Source/core/css/CSSProperties.in [modify] https://crrev.com/bd928558b52c794c649fc064a82151bd9171ee65/third_party/WebKit/Source/core/style/ComputedStyle.cpp [modify] https://crrev.com/bd928558b52c794c649fc064a82151bd9171ee65/third_party/WebKit/Source/core/style/ComputedStyle.h
,
Jan 18 2017
Should this still be assigned to sashab?
,
Jan 18 2017
Assigning to ktyliu@ and moving myself to CC. ktyliu and I discussed offline that when he's ready to start work on this, the in-progress patch to remove style recalcs for independently inherited properties changed through variables is here -- https://codereview.chromium.org/2106073005 This is not a super straightforward patch, so I'll be working with him to help him understand it and complete it. I consider variables the 'generic' case for independent inheritance so follow-up patches for this should be simpler. :)
,
Feb 12 2017
,
Feb 13 2017
With Elliott's comments on http://crbug.com/664066 that favor walking up the tree, should we adopt this generally? That is, always walk up the tree if style is inherited and then we won't have to propagate styles down unless required for layout/render
,
Feb 20 2017
I'd be all for an investigatory project where we remove cache-like fields and replace them with treewalks and see what the impact on perf/mem is like. Another thing we could look at (maybe later) is a dynamic LRU cache for elements, it would be interesting to see what properties are most commonly stored in there (e.g. maybe font-size, width/height, and some others give us the best bang for buck to store, others are often set higher up in the page or change frequently). This might be easier once CStyle is generated, but if you want to try it for only a few fields, we could definitely start that now.
,
Mar 3 2017
I don't think you want to always walk up the tree. It depends on the usage of the property. For a property where we rarely need to know the answer, and only for one element at a time, walking up the tree makes sense. For a value where we often need to know the answer, and we often need to know it for many elements in the tree it might make more sense to propagate it down. Also values that others depend on during computation require passing the value down, otherwise we're n^2 in the depth of the tree during computation.
,
Mar 5 2017
,
Mar 8 2017
,
Apr 18 2017
,
Dec 6 2017
,
Dec 6
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 12
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by bugdroid1@chromium.org
, Jul 6 2016