Changeset 58273 in webkit


Ignore:
Timestamp:
Apr 26, 2010 5:09:58 PM (14 years ago)
Author:
mjs@apple.com
Message:

2010-04-26 Maciej Stachowiak <​mjs@apple.com>

Reviewed by Darin Adler.

REGRESSION (r57292): 1.5% page load speed regression from visited link information leak fix
​https://bugs.webkit.org/show_bug.cgi?id=38131

I did a number of separate optimizations which speed up style
resolution enough to more than make up for the regression. This
measures as a total PLT speedup of somewhere between 1.5% and
3.7%.


Optimizations done:

  • Cache determineLinkState results, to avoid the need to repeatedly compute the visited link hash for the same element. This directly addresses much of the slowdown, since all elements get their style computed twice now.
  • Added a fast way to get the length of a CSSMutableStyleDeclaration, and use in CSSStyleSelector::matchRulesForList, since it was hot there.
  • Hoist some loop invariant code that's not detected by the compiler out of the main loop in matchRulesForList
  • inline CSSStyleSelector::initElement and locateSharedStyle, since there is only one call site in each case
  • Inline the common non-line fast case of determineLinkState, and split the rest into out-of-line determineLinkStateSlowCase.
  • Added inline versions of the functions called by visitedLinkHash (the version called by determineLinkState).
  • css/CSSMutableStyleDeclaration.cpp: (WebCore::CSSMutableStyleDeclaration::length): Implemented in terms of new inline nonvirtual mutableLength().
  • css/CSSMutableStyleDeclaration.h: (WebCore::CSSMutableStyleDeclaration::mutableLength): Added new nonvirtual inline way to get the length if you know you have a mutable style decl.
  • css/CSSStyleSelector.cpp: (WebCore::CSSStyleSelector::init): Clear cached link state. (WebCore::CSSStyleSelector::matchRulesForList): hoist some code out of the main loop and get style decl length more efficiently. (WebCore::CSSStyleSelector::initElement): inline (only one call site) (WebCore::CSSStyleSelector::SelectorChecker::determineLinkState): Inline fast case, call slow case. (WebCore::CSSStyleSelector::SelectorChecker::determineLinkStateSlowCase): Split most of the above function into this slow case helper. (WebCore::CSSStyleSelector::canShareStyleWithElement): Use the cache-enabled way to get the current link state. (WebCore::CSSStyleSelector::locateSharedStyle): inline (WebCore::CSSStyleSelector::styleForElement): Use the cache-enabled way to get the current link state.
  • css/CSSStyleSelector.h: (WebCore::CSSStyleSelector::currentElementLinkState): inline way to get link state for the current element; manages the cache
  • platform/LinkHash.cpp: (WebCore::visitedLinkHashInline): inline version of below function (WebCore::visitedLinkHash): call the inline version (WebCore::visitedURLInline): inline version of below function (WebCore::visitedURL): call the inline version (WebCore::visitedURL): call inline versions of above two functions
Location:
trunk/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r58272 r58273  
     12010-04-26  Maciej Stachowiak  <mjs@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        REGRESSION (r57292): 1.5% page load speed regression from visited link information leak fix
     6        https://bugs.webkit.org/show_bug.cgi?id=38131
     7
     8        I did a number of separate optimizations which speed up style
     9        resolution enough to more than make up for the regression. This
     10        measures as a total PLT speedup of somewhere between 1.5% and
     11        3.7%.
     12       
     13        Optimizations done:
     14        - Cache determineLinkState results, to avoid the need to repeatedly compute
     15        the visited link hash for the same element. This directly addresses much
     16        of the slowdown, since all elements get their style computed twice now.
     17        - Added a fast way to get the length of a CSSMutableStyleDeclaration, and use
     18        in CSSStyleSelector::matchRulesForList, since it was hot there.
     19        - Hoist some loop invariant code that's not detected by the compiler out of the
     20        main loop in matchRulesForList
     21        - inline CSSStyleSelector::initElement and locateSharedStyle,
     22        since there is only one call site in each case
     23        - Inline the common non-line fast case of determineLinkState, and split the rest into
     24        out-of-line determineLinkStateSlowCase.
     25        - Added inline versions of the functions called by
     26        visitedLinkHash (the version called by determineLinkState).
     27
     28        * css/CSSMutableStyleDeclaration.cpp:
     29        (WebCore::CSSMutableStyleDeclaration::length): Implemented in terms of new
     30        inline nonvirtual mutableLength().
     31        * css/CSSMutableStyleDeclaration.h:
     32        (WebCore::CSSMutableStyleDeclaration::mutableLength): Added new nonvirtual
     33        inline way to get the length if you know you have a mutable style decl.
     34        * css/CSSStyleSelector.cpp:
     35        (WebCore::CSSStyleSelector::init): Clear cached link state.
     36        (WebCore::CSSStyleSelector::matchRulesForList): hoist some code out of the main
     37        loop and get style decl length more efficiently.
     38        (WebCore::CSSStyleSelector::initElement): inline (only one call site)
     39        (WebCore::CSSStyleSelector::SelectorChecker::determineLinkState): Inline fast
     40        case, call slow case.
     41        (WebCore::CSSStyleSelector::SelectorChecker::determineLinkStateSlowCase): Split
     42        most of the above function into this slow case helper.
     43        (WebCore::CSSStyleSelector::canShareStyleWithElement): Use the cache-enabled
     44        way to get the current link state.
     45        (WebCore::CSSStyleSelector::locateSharedStyle): inline
     46        (WebCore::CSSStyleSelector::styleForElement): Use the cache-enabled way
     47        to get the current link state.
     48        * css/CSSStyleSelector.h:
     49        (WebCore::CSSStyleSelector::currentElementLinkState): inline way to
     50        get link state for the current element; manages the cache
     51        * platform/LinkHash.cpp:
     52        (WebCore::visitedLinkHashInline): inline version of below function
     53        (WebCore::visitedLinkHash): call the inline version
     54        (WebCore::visitedURLInline): inline version of below function
     55        (WebCore::visitedURL): call the inline version
     56        (WebCore::visitedURL): call inline versions of above two functions
     57
    1582010-04-26  Sam Weinig  <sam@webkit.org>
    259

  • trunk/WebCore/css/CSSComputedStyleDeclaration.cpp

    r57809 r58273  
    15001500}
    15011501
    1502 unsigned CSSComputedStyleDeclaration::length() const
     1502unsigned CSSComputedStyleDeclaration::virtualLength() const
    15031503{
    15041504    Node* node = m_node.get();

  • trunk/WebCore/css/CSSComputedStyleDeclaration.h

    r57809 r58273  
    4040    virtual String cssText() const;
    4141
    42     virtual unsigned length() const;
     42    virtual unsigned virtualLength() const;
    4343    virtual String item(unsigned index) const;
    4444

  • trunk/WebCore/css/CSSMutableStyleDeclaration.cpp

    r52354 r58273  
    628628}
    629629
    630 unsigned CSSMutableStyleDeclaration::length() const
    631 {
    632     return m_properties.size();
     630unsigned CSSMutableStyleDeclaration::virtualLength() const
     631{
     632    return length();
    633633}
    634634

  • trunk/WebCore/css/CSSMutableStyleDeclaration.h

    r42377 r58273  
    8989    virtual void setCssText(const String&, ExceptionCode&);
    9090
    91     virtual unsigned length() const;
     91    virtual unsigned virtualLength() const;
     92    unsigned length() const { return m_properties.size(); }
     93
    9294    virtual String item(unsigned index) const;
    9395

  • trunk/WebCore/css/CSSStyleDeclaration.h

    r45014 r58273  
    4343    virtual void setCssText(const String&, ExceptionCode&) = 0;
    4444
    45     virtual unsigned length() const = 0;
     45    unsigned length() const { return virtualLength(); }
     46    virtual unsigned virtualLength() const = 0;
    4647    bool isEmpty() const { return !length(); }
    4748    virtual String item(unsigned index) const = 0;

  • trunk/WebCore/css/CSSStyleSelector.cpp

    r57809 r58273  
    487487{
    488488    m_element = 0;
     489    m_haveCachedLinkState = false;
    489490    m_matchedDecls.clear();
    490491    m_ruleList = 0;
    … …  
    695696        return;
    696697
     698    const AtomicString& localName = m_element->localName();
    697699    for (CSSRuleData* d = rules->first(); d; d = d->next()) {
    698700        CSSStyleRule* rule = d->rule();
    699         const AtomicString& localName = m_element->localName();
    700701        const AtomicString& selectorLocalName = d->selector()->m_tag.localName();
    701702        if ((localName == selectorLocalName || selectorLocalName == starAtom) && checkSelector(d->selector())) {
    … …  
    801802}
    802803
    803 void CSSStyleSelector::initElement(Element* e)
    804 {
     804inline void CSSStyleSelector::initElement(Element* e)
     805{
     806    if (m_element != e)
     807        m_haveCachedLinkState = false;
     808
    805809    m_element = e;
    806810    m_styledElement = m_element && m_element->isStyledElement() ? static_cast<StyledElement*>(m_element) : 0;
    … …  
    875879}
    876880
    877 EInsideLink CSSStyleSelector::SelectorChecker::determineLinkState(Element* element) const
     881inline EInsideLink CSSStyleSelector::SelectorChecker::determineLinkState(Element* element) const
    878882{
    879883    if (!element->isLink())
    880884        return NotInsideLink;
     885    return determineLinkStateSlowCase(element);
     886}
     887   
     888
     889EInsideLink CSSStyleSelector::SelectorChecker::determineLinkStateSlowCase(Element* element) const
     890{
     891    ASSERT(element->isLink());
    881892   
    882893    const AtomicString* attr = linkAttribute(element);
    … …  
    10261037                if (mappedAttrsMatch) {
    10271038                    if (s->isLink()) {
    1028                         if (m_checker.determineLinkState(m_element) != style->insideLink())
     1039                        if (currentElementLinkState() != style->insideLink())
    10291040                            return false;
    10301041                    }
    … …  
    10371048}
    10381049
    1039 RenderStyle* CSSStyleSelector::locateSharedStyle()
     1050ALWAYS_INLINE RenderStyle* CSSStyleSelector::locateSharedStyle()
    10401051{
    10411052    if (m_styledElement && !m_styledElement->inlineStyleDecl() && !m_styledElement->hasID() && !m_styledElement->document()->usesSiblingRules()) {
    … …  
    11701181    if (e->isLink()) {
    11711182        m_style->setIsLink(true);
    1172         m_style->setInsideLink(m_checker.determineLinkState(e));
     1183        m_style->setInsideLink(currentElementLinkState());
    11731184    }
    11741185   

  • trunk/WebCore/css/CSSStyleSelector.h

    r57292 r58273  
    8686        ~CSSStyleSelector();
    8787
    88         void initElement(Element*);
    8988        void initForStyleResolve(Element*, RenderStyle* parentStyle = 0, PseudoId = NOPSEUDO);
    9089        PassRefPtr<RenderStyle> styleForElement(Element*, RenderStyle* parentStyle = 0, bool allowSharing = true, bool resolveForRootDefault = false, bool matchVisitedLinks = false);
    … …  
    102101
    103102    private:
     103        void initElement(Element*);
    104104        RenderStyle* locateSharedStyle();
    105105        Node* locateCousinList(Element* parent, unsigned depth = 1);
    … …  
    202202
    203203            EInsideLink determineLinkState(Element* element) const;
     204            EInsideLink determineLinkStateSlowCase(Element* element) const;
    204205            void allVisitedStateChanged();
    205206            void visitedStateChanged(LinkHash visitedHash);
    … …  
    252253
    253254        StyleImage* styleImage(CSSValue* value);
     255
     256
     257        EInsideLink currentElementLinkState() const
     258        {
     259            if (!m_haveCachedLinkState) {
     260                m_cachedLinkState = m_checker.determineLinkState(m_element);
     261                m_haveCachedLinkState = true;
     262            }
     263            return m_cachedLinkState;
     264        }
     265
     266        mutable EInsideLink m_cachedLinkState;
     267        mutable bool m_haveCachedLinkState;
    254268
    255269        // We collect the set of decls that match in |m_matchedDecls|.  We then walk the
    … …  
    348362        CSSRuleData* m_last;
    349363    };
    350    
     364
    351365} // namespace WebCore
    352366

  • trunk/WebCore/platform/LinkHash.cpp

    r57292 r58273  
    148148}
    149149
     150static ALWAYS_INLINE LinkHash visitedLinkHashInline(const UChar* url, unsigned length)
     151{
     152    return AlreadyHashed::avoidDeletedValue(StringImpl::computeHash(url, length));
     153}
     154
    150155LinkHash visitedLinkHash(const UChar* url, unsigned length)
    151156{
    152   return AlreadyHashed::avoidDeletedValue(StringImpl::computeHash(url, length));
    153 }
    154 
    155 void visitedURL(const KURL& base, const AtomicString& attributeURL, Vector<UChar, 512>& buffer)
     157    return visitedLinkHashInline(url, length);
     158}
     159
     160static ALWAYS_INLINE void visitedURLInline(const KURL& base, const AtomicString& attributeURL, Vector<UChar, 512>& buffer)
    156161{
    157162    if (attributeURL.isNull())
    … …  
    214219}
    215220
     221void visitedURL(const KURL& base, const AtomicString& attributeURL, Vector<UChar, 512>& buffer)
     222{
     223    return visitedURLInline(base, attributeURL, buffer);
     224}
     225
    216226LinkHash visitedLinkHash(const KURL& base, const AtomicString& attributeURL)
    217227{
    218228    Vector<UChar, 512> url;
    219     visitedURL(base, attributeURL, url);
     229    visitedURLInline(base, attributeURL, url);
    220230    if (url.isEmpty())
    221231        return 0;
    222232
    223     return visitedLinkHash(url.data(), url.size());
     233    return visitedLinkHashInline(url.data(), url.size());
    224234}
    225235
Note: See TracChangeset for help on using the changeset viewer.