Changeset 198584 in webkit


Ignore:
Timestamp:
Mar 23, 2016 7:16:17 AM (8 years ago)
Author:
Antti Koivisto
Message:

Share style by sharing RenderStyle substructures not the object itself
https://bugs.webkit.org/show_bug.cgi?id=155787

Reviewed by Anreas Kling.

The current approach where we share RenderStyle objects between elements leads to lot of awkward and bug-prone code.
Most of the RenderStyle consists of shareable substructures. It is better to just share those.

With this patch we create shared styles with RenderStyle::clone(). Sharing is traced as state in Style::SharingResolver
instead of relying on RenderStyle equality to locate potential sharing cousins.

  • rendering/style/StyleRareNonInheritedData.cpp:

(WebCore::StyleRareNonInheritedData::operator==):

m_altText was missing from operator==
This was exposed by TreeResolver::resolveElement change, tested by fast/css/alt-inherit-initial.html

  • style/StyleSharingResolver.cpp:

(WebCore::Style::elementHasDirectionAuto):
(WebCore::Style::SharingResolver::resolve):

Save share results to a map.

(WebCore::Style::SharingResolver::findSibling):
(WebCore::Style::SharingResolver::locateCousinList):

Instead of traversing we can now just do a hash lookup to locate a candidate cousin list.
There is no need for recursion anymore, the map covers sharing beyond immediate siblings too.
Remove most tests here as they have been already covered when sharing occured.

(WebCore::Style::canShareStyleWithControl):

  • style/StyleSharingResolver.h:
  • style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::styleForElement):
(WebCore::Style::TreeResolver::resolveElement):

No need to do forced setting anymore just to support style sharing.

Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r198583 r198584  
     12016-03-23  Antti Koivisto  <antti@apple.com>
     2
     3        Share style by sharing RenderStyle substructures not the object itself
     4        https://bugs.webkit.org/show_bug.cgi?id=155787
     5
     6        Reviewed by Anreas Kling.
     7
     8        The current approach where we share RenderStyle objects between elements leads to lot of awkward and bug-prone code.
     9        Most of the RenderStyle consists of shareable substructures. It is better to just share those.
     10
     11        With this patch we create shared styles with RenderStyle::clone(). Sharing is traced as state in Style::SharingResolver
     12        instead of relying on RenderStyle equality to locate potential sharing cousins.
     13
     14        * rendering/style/StyleRareNonInheritedData.cpp:
     15        (WebCore::StyleRareNonInheritedData::operator==):
     16
     17            m_altText was missing from operator==
     18            This was exposed by TreeResolver::resolveElement change, tested by fast/css/alt-inherit-initial.html
     19
     20        * style/StyleSharingResolver.cpp:
     21        (WebCore::Style::elementHasDirectionAuto):
     22        (WebCore::Style::SharingResolver::resolve):
     23
     24            Save share results to a map.
     25
     26        (WebCore::Style::SharingResolver::findSibling):
     27        (WebCore::Style::SharingResolver::locateCousinList):
     28
     29            Instead of traversing we can now just do a hash lookup to locate a candidate cousin list.
     30            There is no need for recursion anymore, the map covers sharing beyond immediate siblings too.
     31            Remove most tests here as they have been already covered when sharing occured.
     32
     33        (WebCore::Style::canShareStyleWithControl):
     34        * style/StyleSharingResolver.h:
     35        * style/StyleTreeResolver.cpp:
     36        (WebCore::Style::TreeResolver::styleForElement):
     37        (WebCore::Style::TreeResolver::resolveElement):
     38
     39            No need to do forced setting anymore just to support style sharing.
     40
    1412016-03-23  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
    242
  • trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp

    r197617 r198584  
    245245        && contentDataEquivalent(o)
    246246        && arePointingToEqualData(m_counterDirectives, o.m_counterDirectives)
     247        && m_altText == o.m_altText
    247248        && arePointingToEqualData(m_boxShadow, o.m_boxShadow)
    248249        && arePointingToEqualData(m_willChange, o.m_willChange)
  • trunk/Source/WebCore/style/StyleSharingResolver.cpp

    r197764 r198584  
    4343
    4444static const unsigned cStyleSearchThreshold = 10;
    45 static const unsigned cStyleSearchLevelThreshold = 10;
    4645
    4746struct SharingResolver::Context {
     
    6968}
    7069
    71 const Element* SharingResolver::resolve(const Element& searchElement) const
     70RefPtr<RenderStyle> SharingResolver::resolve(const Element& searchElement)
    7271{
    7372    if (!is<StyledElement>(searchElement))
     
    104103    // Check previous siblings and their cousins.
    105104    unsigned count = 0;
    106     unsigned visitedNodeCount = 0;
    107105    StyledElement* shareElement = nullptr;
    108106    Node* cousinList = element.previousSibling();
     
    111109        if (shareElement)
    112110            break;
    113         cousinList = locateCousinList(cousinList->parentElement(), visitedNodeCount);
     111        cousinList = locateCousinList(cousinList->parentElement());
    114112    }
    115113
     
    128126        return nullptr;
    129127
    130     return shareElement;
     128    m_elementsSharingStyle.add(&element, shareElement);
     129
     130    return RenderStyle::clone(shareElement->renderStyle());
    131131}
    132132
     
    144144}
    145145
    146 Node* SharingResolver::locateCousinList(Element* parent, unsigned& visitedNodeCount) const
    147 {
    148     if (visitedNodeCount >= cStyleSearchThreshold * cStyleSearchLevelThreshold)
    149         return nullptr;
    150     if (!is<StyledElement>(parent))
    151         return nullptr;
    152     auto& styledParent = downcast<StyledElement>(*parent);
    153     if (styledParent.inlineStyle())
    154         return nullptr;
    155     if (is<SVGElement>(styledParent) && downcast<SVGElement>(styledParent).animatedSMILStyleProperties())
    156         return nullptr;
    157     if (styledParent.hasID() && m_ruleSets.features().idsInRules.contains(styledParent.idForStyleResolution().impl()))
    158         return nullptr;
    159 
    160     RenderStyle* parentStyle = styledParent.renderStyle();
    161     unsigned subcount = 0;
    162     Node* thisCousin = &styledParent;
    163     Node* currentNode = styledParent.previousSibling();
    164 
    165     // Reserve the tries for this level. This effectively makes sure that the algorithm
    166     // will never go deeper than cStyleSearchLevelThreshold levels into recursion.
    167     visitedNodeCount += cStyleSearchThreshold;
    168     while (thisCousin) {
    169         for (; currentNode; currentNode = currentNode->previousSibling()) {
    170             if (++subcount > cStyleSearchThreshold)
    171                 return nullptr;
    172             if (!is<Element>(*currentNode))
    173                 continue;
    174             auto& currentElement = downcast<Element>(*currentNode);
    175             if (currentElement.renderStyle() != parentStyle)
    176                 continue;
    177             if (!currentElement.lastChild())
    178                 continue;
    179             if (!parentElementPreventsSharing(currentElement)) {
    180                 // Adjust for unused reserved tries.
    181                 visitedNodeCount -= cStyleSearchThreshold - subcount;
    182                 return currentNode->lastChild();
    183             }
     146Node* SharingResolver::locateCousinList(const Element* parent) const
     147{
     148    const unsigned maximumSearchCount = 10;
     149    for (unsigned count = 0; count < maximumSearchCount; ++count) {
     150        auto* elementSharingParentStyle = m_elementsSharingStyle.get(parent);
     151        if (!elementSharingParentStyle)
     152            return nullptr;
     153        if (!parentElementPreventsSharing(*elementSharingParentStyle)) {
     154            if (auto* cousin = elementSharingParentStyle->lastChild())
     155                return cousin;
    184156        }
    185         currentNode = locateCousinList(thisCousin->parentElement(), visitedNodeCount);
    186         thisCousin = currentNode;
     157        parent = elementSharingParentStyle;
    187158    }
    188159
  • trunk/Source/WebCore/style/StyleSharingResolver.h

    r196031 r198584  
    2727#define StyleSharingResolver_h
    2828
     29#include <wtf/HashMap.h>
     30
    2931namespace WebCore {
    3032
     
    3335class Element;
    3436class Node;
     37class RenderStyle;
    3538class RuleSet;
    3639class SelectorFilter;
     
    4447    SharingResolver(const Document&, const DocumentRuleSets&, const SelectorFilter&);
    4548
    46     const Element* resolve(const Element&) const;
     49    RefPtr<RenderStyle> resolve(const Element&);
    4750
    4851private:
     
    5053
    5154    StyledElement* findSibling(const Context&, Node*, unsigned& count) const;
    52     Node* locateCousinList(Element* parent, unsigned& visitedNodeCount) const;
     55    Node* locateCousinList(const Element* parent) const;
    5356    bool canShareStyleWithElement(const Context&, const StyledElement& candidateElement) const;
    5457    bool styleSharingCandidateMatchesRuleSet(const StyledElement&, const RuleSet*) const;
     
    5962    const DocumentRuleSets& m_ruleSets;
    6063    const SelectorFilter& m_selectorFilter;
     64
     65    HashMap<const Element*, const Element*> m_elementsSharingStyle;
    6166};
    6267
  • trunk/Source/WebCore/style/StyleTreeResolver.cpp

    r198044 r198584  
    187187    }
    188188
    189     if (auto* sharingElement = scope().sharingResolver.resolve(element))
    190         return *sharingElement->renderStyle();
     189    if (auto style = scope().sharingResolver.resolve(element))
     190        return *style;
    191191
    192192    auto elementStyle = scope().styleResolver.styleForElement(element, &inheritedStyle, MatchAllRules, nullptr, &scope().selectorFilter);
     
    698698        if (localChange != NoChange || pseudoStyleCacheIsInvalid(renderer, newStyle.get()) || (parent().change == Force && renderer->requiresForcedStyleRecalcPropagation()) || current.styleChangeType() == SyntheticStyleChange)
    699699            renderer->setAnimatableStyle(*newStyle, current.styleChangeType() == SyntheticStyleChange ? StyleDifferenceRecompositeLayer : StyleDifferenceEqual);
    700         else if (current.needsStyleRecalc()) {
    701             // Although no change occurred, we use the new style so that the cousin style sharing code won't get
    702             // fooled into believing this style is the same.
    703             renderer->setStyleInternal(*newStyle);
    704         }
    705700    }
    706701
Note: See TracChangeset for help on using the changeset viewer.