Changeset 101524 in webkit


Ignore:
Timestamp:
Nov 30, 2011 9:25:17 AM (12 years ago)
Author:
Antti Koivisto
Message:

Reuse cached style fully if the parent inherited styles are equal
​https://bugs.webkit.org/show_bug.cgi?id=73421

Reviewed by Oliver Hunt.

The matched declaration cache currently restores the non-inherted properties from the cache
entry but still applies all inherited properties normally. In case the current parent
inherited style is equivalent to the cache entry's, also the inherited style can be reused
and no properties need to be applied. This is faster and saves memory (by sharing the
style substructures better).

The new optimized code path has a pretty good hit rate, >50% of all cases on many pages.

Loading the HTML5 spec this reduces style memory consumption by ~20% (5MB, ~2.5% of total) and
speeds up style applying by ~25% for ~0.4s (2-3%) gain in the spec loading benchmark.

  • css/CSSStyleSelector.cpp:

(WebCore::CSSStyleSelector::applyDeclaration):
(WebCore::CSSStyleSelector::applyDeclarations):

Remove the code that dynamically disables inherited only applying. We now don't allow
styles with explicitly inherited properties to be cached in the first place.


(WebCore::CSSStyleSelector::findFromMatchedDeclarationCache):

Return the full cache item.


(WebCore::CSSStyleSelector::addToMatchedDeclarationCache):

Also the parent style is now needed for the check for full sharing.


(WebCore::isCacheableInMatchedDeclarationCache):

Don't allow styles with explicitly inherited properties to be cached at all.


(WebCore::CSSStyleSelector::applyMatchedDeclarations):

If the parent inherited styles are equal reuse the cache entry fully and return without
doing anything else.


  • css/CSSStyleSelector.h:
  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::inheritedDataShared):

  • rendering/style/RenderStyle.h:


Add fast check for equal inherited properties.

Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r101520 r101524  
     12011-11-30  Antti Koivisto  <antti@apple.com>
     2
     3        Reuse cached style fully if the parent inherited styles are equal
     4        https://bugs.webkit.org/show_bug.cgi?id=73421
     5
     6        Reviewed by Oliver Hunt.
     7
     8        The matched declaration cache currently restores the non-inherted properties from the cache
     9        entry but still applies all inherited properties normally. In case the current parent
     10        inherited style is equivalent to the cache entry's, also the inherited style can be reused
     11        and no properties need to be applied. This is faster and saves memory (by sharing the
     12        style substructures better).
     13       
     14        The new optimized code path has a pretty good hit rate, >50% of all cases on many pages.
     15       
     16        Loading the HTML5 spec this reduces style memory consumption by ~20% (5MB, ~2.5% of total) and
     17        speeds up style applying by ~25% for ~0.4s (2-3%) gain in the spec loading benchmark.
     18       
     19        * css/CSSStyleSelector.cpp:
     20        (WebCore::CSSStyleSelector::applyDeclaration):
     21        (WebCore::CSSStyleSelector::applyDeclarations):
     22       
     23            Remove the code that dynamically disables inherited only applying. We now don't allow
     24            styles with explicitly inherited properties to be cached in the first place.
     25       
     26        (WebCore::CSSStyleSelector::findFromMatchedDeclarationCache):
     27       
     28            Return the full cache item.
     29       
     30        (WebCore::CSSStyleSelector::addToMatchedDeclarationCache):
     31       
     32            Also the parent style is now needed for the check for full sharing.
     33       
     34        (WebCore::isCacheableInMatchedDeclarationCache):
     35
     36            Don't allow styles with explicitly inherited properties to be cached at all.
     37       
     38        (WebCore::CSSStyleSelector::applyMatchedDeclarations):
     39       
     40            If the parent inherited styles are equal reuse the cache entry fully and return without
     41            doing anything else.
     42       
     43        * css/CSSStyleSelector.h:
     44        * rendering/style/RenderStyle.cpp:
     45        (WebCore::RenderStyle::inheritedDataShared):
     46        * rendering/style/RenderStyle.h:
     47       
     48            Add fast check for equal inherited properties.
     49
    1502011-11-30  Renata Hodovan  <reni@webkit.org>
    251

  • trunk/Source/WebCore/css/CSSStyleSelector.cpp

    r101499 r101524  
    21202120
    21212121template <bool applyFirst>
    2122 void CSSStyleSelector::applyDeclaration(CSSMutableStyleDeclaration* styleDeclaration, bool isImportant, bool& inheritedOnly)
     2122void CSSStyleSelector::applyDeclaration(CSSMutableStyleDeclaration* styleDeclaration, bool isImportant, bool inheritedOnly)
    21232123{
    21242124    CSSMutableStyleDeclaration::const_iterator end = styleDeclaration->end();
    … …  
    21282128            continue;
    21292129        if (inheritedOnly && !current.isInherited()) {
    2130             if (!current.value()->isInheritedValue())
    2131                 continue;
    21322130            // If the property value is explicitly inherited, we need to apply further non-inherited properties
    2133             // as they might override the value inherited here. This is really per-property but that is
    2134             // probably not worth optimizing for.
    2135             inheritedOnly = false;
     2131            // as they might override the value inherited here. For this reason we don't allow declarations with
     2132            // explicitly inherited properties to be cached.
     2133            ASSERT(!current.value()->isInheritedValue());
     2134            continue;
    21362135        }
    21372136        int property = current.id();
    … …  
    21572156
    21582157template <bool applyFirst>
    2159 void CSSStyleSelector::applyDeclarations(bool isImportant, int startIndex, int endIndex, bool& inheritedOnly)
     2158void CSSStyleSelector::applyDeclarations(bool isImportant, int startIndex, int endIndex, bool inheritedOnly)
    21602159{
    21612160    if (startIndex == -1)
    … …  
    22112210}
    22122211
    2213 const RenderStyle* CSSStyleSelector::findFromMatchedDeclarationCache(unsigned hash, const MatchResult& matchResult)
     2212const CSSStyleSelector::MatchedStyleDeclarationCacheItem* CSSStyleSelector::findFromMatchedDeclarationCache(unsigned hash, const MatchResult& matchResult)
    22142213{
    22152214    ASSERT(hash);
    … …  
    22302229    if (cacheItem.matchResult != matchResult)
    22312230        return 0;
    2232     return cacheItem.renderStyle.get();
    2233 }
    2234 
    2235 void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, unsigned hash, const MatchResult& matchResult)
     2231    return &cacheItem;
     2232}
     2233
     2234void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, const RenderStyle* parentStyle, unsigned hash, const MatchResult& matchResult)
    22362235{
    22372236    ASSERT(hash);
    … …  
    22402239    cacheItem.matchResult = matchResult;
    22412240    // Note that we don't cache the original RenderStyle instance. It may be further modified.
    2242     // The RenderStyle in the cache is really just a holder for the non-inherited substructures and never used as-is.
     2241    // The RenderStyle in the cache is really just a holder for the substructures and never used as-is.
    22432242    cacheItem.renderStyle = RenderStyle::clone(style);
     2243    cacheItem.parentRenderStyle = RenderStyle::clone(parentStyle);
    22442244    m_matchStyleDeclarationCache.add(hash, cacheItem);
    22452245}
    … …  
    22532253    if (style->zoom() != RenderStyle::initialZoom())
    22542254        return false;
     2255    // The cache assumes static knowledge about which properties are inherited.
     2256    if (parentStyle->hasExplicitlyInheritedProperties())
     2257        return false;
    22552258    return true;
    22562259}
    … …  
    22602263    unsigned cacheHash = matchResult.isCacheable ? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;
    22612264    bool applyInheritedOnly = false;
    2262     const RenderStyle* cachedStyle = 0;
    2263     if (cacheHash && (cachedStyle = findFromMatchedDeclarationCache(cacheHash, matchResult))) {
     2265    const MatchedStyleDeclarationCacheItem* cacheItem = 0;
     2266    if (cacheHash && (cacheItem = findFromMatchedDeclarationCache(cacheHash, matchResult))) {
    22642267        // We can build up the style by copying non-inherited properties from an earlier style object built using the same exact
    22652268        // style declarations. We then only need to apply the inherited properties, if any, as their values can depend on the
    22662269        // element context. This is fast and saves memory by reusing the style data structures.
    2267         m_style->copyNonInheritedFrom(cachedStyle);
     2270        m_style->copyNonInheritedFrom(cacheItem->renderStyle.get());
     2271        if (m_parentStyle->inheritedDataShared(cacheItem->parentRenderStyle.get())) {
     2272            EInsideLink linkStatus = m_style->insideLink();
     2273            // If the cache item parent style has identical inherited properties to the current parent style then the
     2274            // resulting style will be identical too. We copy the inherited properties over from the cache and are done.
     2275            m_style->inheritFrom(cacheItem->renderStyle.get());
     2276            // Unfortunately the link status is treated like an inherited property. We need to explicitly restore it.
     2277            m_style->setInsideLink(linkStatus);
     2278            return;
     2279        }
    22682280        applyInheritedOnly = true;
    22692281    }
    … …  
    22782290    applyDeclarations<true>(true, matchResult.firstUARule, matchResult.lastUARule, applyInheritedOnly);
    22792291
    2280     if (cachedStyle && cachedStyle->effectiveZoom() != m_style->effectiveZoom()) {
     2292    if (cacheItem && cacheItem->renderStyle->effectiveZoom() != m_style->effectiveZoom()) {
    22812293        m_fontDirty = true;
    22822294        applyInheritedOnly = false;
    … …  
    22912303
    22922304    // Many properties depend on the font. If it changes we just apply all properties.
    2293     if (cachedStyle && cachedStyle->fontDescription() != m_style->fontDescription())
     2305    if (cacheItem && cacheItem->renderStyle->fontDescription() != m_style->fontDescription())
    22942306        applyInheritedOnly = false;
    22952307
    … …  
    23142326    ASSERT(!m_fontDirty);
    23152327   
    2316     if (cachedStyle || !cacheHash)
     2328    if (cacheItem || !cacheHash)
    23172329        return;
    23182330    if (!isCacheableInMatchedDeclarationCache(m_style.get(), m_parentStyle))
    23192331        return;
    2320     addToMatchedDeclarationCache(m_style.get(), cacheHash, matchResult);
     2332    addToMatchedDeclarationCache(m_style.get(), m_parentStyle, cacheHash, matchResult);
    23212333}
    23222334

  • trunk/Source/WebCore/css/CSSStyleSelector.h

    r101499 r101524  
    266266    void applyMatchedDeclarations(const MatchResult&);
    267267    template <bool firstPass>
    268     void applyDeclarations(bool important, int startIndex, int endIndex, bool& inheritedOnly);
     268    void applyDeclarations(bool important, int startIndex, int endIndex, bool inheritedOnly);
    269269    template <bool firstPass>
    270     void applyDeclaration(CSSMutableStyleDeclaration*, bool isImportant, bool& inheritedOnly);
     270    void applyDeclaration(CSSMutableStyleDeclaration*, bool isImportant, bool inheritedOnly);
    271271
    272272    void matchPageRules(RuleSet*, bool isLeftPage, bool isFirstPage, const String& pageName);
    … …  
    350350    };
    351351    static unsigned computeDeclarationHash(MatchedStyleDeclaration*, unsigned size);
    352     const RenderStyle* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);   
    353     void addToMatchedDeclarationCache(const RenderStyle*, unsigned hash, const MatchResult&);
     352    struct MatchedStyleDeclarationCacheItem {
     353        Vector<MatchedStyleDeclaration> matchedStyleDeclarations;
     354        MatchResult matchResult;
     355        RefPtr<RenderStyle> renderStyle;
     356        RefPtr<RenderStyle> parentRenderStyle;
     357    };
     358    const MatchedStyleDeclarationCacheItem* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);
     359    void addToMatchedDeclarationCache(const RenderStyle*, const RenderStyle* parentStyle, unsigned hash, const MatchResult&);
    354360
    355361    // We collect the set of decls that match in |m_matchedDecls|. We then walk the
    … …  
    358364    // for any !important rules.
    359365    Vector<MatchedStyleDeclaration, 64> m_matchedDecls;
    360    
    361     struct MatchedStyleDeclarationCacheItem {
    362         Vector<MatchedStyleDeclaration> matchedStyleDeclarations;
    363         MatchResult matchResult;
    364         RefPtr<RenderStyle> renderStyle;
    365     };
     366
    366367    typedef HashMap<unsigned, MatchedStyleDeclarationCacheItem> MatchedStyleDeclarationCache;
    367368    MatchedStyleDeclarationCache m_matchStyleDeclarationCache;

  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r101288 r101524  
    320320#endif
    321321           || rareInheritedData != other->rareInheritedData;
     322}
     323
     324bool RenderStyle::inheritedDataShared(const RenderStyle* other) const
     325{
     326    // This is a fast check that only looks if the data structures are shared.
     327    return inherited_flags == other->inherited_flags
     328        && inherited.get() == other->inherited.get()
     329#if ENABLE(SVG)
     330        && m_svgStyle.get() == other->m_svgStyle.get()
     331#endif
     332        && rareInheritedData.get() == other->rareInheritedData.get();
    322333}
    323334

  • trunk/Source/WebCore/rendering/style/RenderStyle.h

    r101399 r101524  
    13441344
    13451345    bool inheritedNotEqual(const RenderStyle*) const;
     1346    bool inheritedDataShared(const RenderStyle*) const;
    13461347
    13471348    StyleDifference diff(const RenderStyle*, unsigned& changedContextSensitiveProperties) const;
Note: See TracChangeset for help on using the changeset viewer.