https://bugs.webkit.org/show_bug.cgi?id=69106
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Sep 2011 07:07:17 +0000 (07:07 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Sep 2011 07:07:17 +0000 (07:07 +0000)
Universal attribute selectors disable style sharing

Reviewed by Dave Hyatt.

Selectors of type [foo="bar"] ended up marking every element style with the affectedByAttributeSelectors bit
rendering style sharing inoperative. This happens on http://www.whatwg.org/specs/web-apps/current-work/ for example.

Instead we now mark style with affectedByUncommonAttributeSelectors bit only if an attribute selector actually
matches the element. Before sharing, we also check the current element against collected attribute rules.
We can share the style if neither element was affected.

This speeds up style matching and applying ~15% on full HTML5 spec (=~0.7s). Sharing percentage goes from 0% to ~30%.
Increased sharing should also save a substantial amount of memory.

* css/CSSSelector.h:
(WebCore::CSSSelector::isAttributeSelector):
* css/CSSStyleSelector.cpp:
(WebCore::RuleData::containsUncommonAttributeSelector):
(WebCore::collectSpecialRulesInDefaultStyle):
(WebCore::assertNoSiblingRulesInDefaultStyle):
(WebCore::CSSStyleSelector::CSSStyleSelector):
(WebCore::CSSStyleSelector::matchRules):
(WebCore::CSSStyleSelector::matchesRuleSet):
(WebCore::CSSStyleSelector::canShareStyleWithElement):
(WebCore::CSSStyleSelector::locateSharedStyle):
(WebCore::CSSStyleSelector::styleForElement):
(WebCore::selectorListContainsUncommonAttributeSelector):
(WebCore::isCommonAttributeSelectorAttribute):
(WebCore::containsUncommonAttributeSelector):
(WebCore::RuleData::RuleData):
(WebCore::collectFeaturesFromSelector):
(WebCore::collectFeaturesFromList):
* css/CSSStyleSelector.h:
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOneSelector):
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::RenderStyle):
* rendering/style/RenderStyle.h:
(WebCore::InheritedFlags::affectedByUncommonAttributeSelectors):
(WebCore::InheritedFlags::setAffectedByUncommonAttributeSelectors):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@96393 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/CSSSelector.h
Source/WebCore/css/CSSStyleSelector.cpp
Source/WebCore/css/CSSStyleSelector.h
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/RenderStyle.h

index f97db1a..fd7cde9 100644 (file)
@@ -1,3 +1,47 @@
+2011-09-29  Antti Koivisto  <antti@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=69106
+        Universal attribute selectors disable style sharing
+
+        Reviewed by Dave Hyatt.
+
+        Selectors of type [foo="bar"] ended up marking every element style with the affectedByAttributeSelectors bit
+        rendering style sharing inoperative. This happens on http://www.whatwg.org/specs/web-apps/current-work/ for example.
+
+        Instead we now mark style with affectedByUncommonAttributeSelectors bit only if an attribute selector actually 
+        matches the element. Before sharing, we also check the current element against collected attribute rules.
+        We can share the style if neither element was affected.
+        
+        This speeds up style matching and applying ~15% on full HTML5 spec (=~0.7s). Sharing percentage goes from 0% to ~30%.
+        Increased sharing should also save a substantial amount of memory.
+
+        * css/CSSSelector.h:
+        (WebCore::CSSSelector::isAttributeSelector):
+        * css/CSSStyleSelector.cpp:
+        (WebCore::RuleData::containsUncommonAttributeSelector):
+        (WebCore::collectSpecialRulesInDefaultStyle):
+        (WebCore::assertNoSiblingRulesInDefaultStyle):
+        (WebCore::CSSStyleSelector::CSSStyleSelector):
+        (WebCore::CSSStyleSelector::matchRules):
+        (WebCore::CSSStyleSelector::matchesRuleSet):
+        (WebCore::CSSStyleSelector::canShareStyleWithElement):
+        (WebCore::CSSStyleSelector::locateSharedStyle):
+        (WebCore::CSSStyleSelector::styleForElement):
+        (WebCore::selectorListContainsUncommonAttributeSelector):
+        (WebCore::isCommonAttributeSelectorAttribute):
+        (WebCore::containsUncommonAttributeSelector):
+        (WebCore::RuleData::RuleData):
+        (WebCore::collectFeaturesFromSelector):
+        (WebCore::collectFeaturesFromList):
+        * css/CSSStyleSelector.h:
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkOneSelector):
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::RenderStyle):
+        * rendering/style/RenderStyle.h:
+        (WebCore::InheritedFlags::affectedByUncommonAttributeSelectors):
+        (WebCore::InheritedFlags::setAffectedByUncommonAttributeSelectors):
+
 2011-09-30  James Robinson  <jamesr@chromium.org>
 
         [chromium] Add WebKit API for sending input events to the compositor thread
index fb07fb5..157207f 100644 (file)
@@ -242,6 +242,7 @@ namespace WebCore {
         bool matchesPseudoElement() const;
         bool isUnknownPseudoElement() const;
         bool isSiblingSelector() const;
+        bool isAttributeSelector() const;
 
         Relation relation() const { return static_cast<Relation>(m_relation); }
 
@@ -326,7 +327,20 @@ inline bool CSSSelector::isSiblingSelector() const
         || type == PseudoNthLastChild
         || type == PseudoNthLastOfType;
 }
-    
+
+inline bool CSSSelector::isAttributeSelector() const
+{
+    if (!hasAttribute())
+        return false;
+    return m_match == CSSSelector::Exact
+        || m_match ==  CSSSelector::Set
+        || m_match == CSSSelector::List
+        || m_match == CSSSelector::Hyphen
+        || m_match == CSSSelector::Contain
+        || m_match == CSSSelector::Begin
+        || m_match == CSSSelector::End;
+}
+
 inline void CSSSelector::setValue(const AtomicString& value)
 {
     // Need to do ref counting manually for the union.
index f2071d9..01ab3d0 100644 (file)
@@ -189,6 +189,7 @@ public:
     bool hasFastCheckableSelector() const { return m_hasFastCheckableSelector; }
     bool hasMultipartSelector() const { return m_hasMultipartSelector; }
     bool hasRightmostSelectorMatchingHTMLBasedOnRuleHash() const { return m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash; }
+    bool containsUncommonAttributeSelector() const { return m_containsUncommonAttributeSelector; }
     unsigned specificity() const { return m_specificity; }
     
     // Try to balance between memory usage (there can be lots of RuleData objects) and good filtering performance.
@@ -199,10 +200,11 @@ private:
     CSSStyleRule* m_rule;
     CSSSelector* m_selector;
     unsigned m_specificity;
-    unsigned m_position : 29;
+    unsigned m_position : 28;
     bool m_hasFastCheckableSelector : 1;
     bool m_hasMultipartSelector : 1;
     bool m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash : 1;
+    bool m_containsUncommonAttributeSelector : 1;
     // Use plain array instead of a Vector to minimize memory overhead.
     unsigned m_descendantSelectorIdentifierHashes[maximumIdentifierCount];
 };
@@ -258,6 +260,7 @@ static RuleSet* defaultViewSourceStyle;
 static CSSStyleSheet* simpleDefaultStyleSheet;
     
 static RuleSet* siblingRulesInDefaultStyle;
+static RuleSet* uncommonAttributeRulesInDefaultStyle;
 
 RenderStyle* CSSStyleSelector::s_styleNotYetAvailable;
 
@@ -271,13 +274,15 @@ static inline bool elementCanUseSimpleDefaultStyle(Element* e)
     return e->hasTagName(htmlTag) || e->hasTagName(headTag) || e->hasTagName(bodyTag) || e->hasTagName(divTag) || e->hasTagName(spanTag) || e->hasTagName(brTag) || e->hasTagName(aTag);
 }
 
-static inline void collectSiblingRulesInDefaultStyle()
+static inline void collectSpecialRulesInDefaultStyle()
 {
     CSSStyleSelector::Features features;
     defaultStyle->collectFeatures(features);
     ASSERT(features.idsInRules.isEmpty());
     delete siblingRulesInDefaultStyle;
+    delete uncommonAttributeRulesInDefaultStyle;
     siblingRulesInDefaultStyle = features.siblingRules.leakPtr();
+    uncommonAttributeRulesInDefaultStyle = features.uncommonAttributeRules.leakPtr();
 }
 
 static inline void assertNoSiblingRulesInDefaultStyle()
@@ -285,7 +290,7 @@ static inline void assertNoSiblingRulesInDefaultStyle()
 #ifndef NDEBUG
     if (siblingRulesInDefaultStyle)
         return;
-    collectSiblingRulesInDefaultStyle();
+    collectSpecialRulesInDefaultStyle();
     ASSERT(!siblingRulesInDefaultStyle);
 #endif
 }
@@ -411,6 +416,8 @@ CSSStyleSelector::CSSStyleSelector(Document* document, StyleSheetList* styleShee
     // Usually there are no sibling rules in the default style but the MathML sheet has some.
     if (siblingRulesInDefaultStyle)
         siblingRulesInDefaultStyle->collectFeatures(m_features);
+    if (uncommonAttributeRulesInDefaultStyle)
+        uncommonAttributeRulesInDefaultStyle->collectFeatures(m_features);
     m_authorStyle->collectFeatures(m_features);
     if (m_userStyle)
         m_userStyle->collectFeatures(m_features);
@@ -418,6 +425,8 @@ CSSStyleSelector::CSSStyleSelector(Document* document, StyleSheetList* styleShee
     m_authorStyle->shrinkToFit();
     if (m_features.siblingRules)
         m_features.siblingRules->shrinkToFit();
+    if (m_features.uncommonAttributeRules)
+        m_features.uncommonAttributeRules->shrinkToFit();
 
     if (document->renderer() && document->renderer()->style())
         document->renderer()->style()->font().update(fontSelector());
@@ -559,8 +568,11 @@ void CSSStyleSelector::matchRules(RuleSet* rules, int& firstRuleIndex, int& last
     
     // Now transfer the set of matched rules over to our list of decls.
     if (!m_checker.isCollectingRulesOnly()) {
-        for (unsigned i = 0; i < m_matchedRules.size(); i++)
+        for (unsigned i = 0; i < m_matchedRules.size(); i++) {
+            if (m_style && m_matchedRules[i]->containsUncommonAttributeSelector())
+                m_style->setAffectedByUncommonAttributeSelectors();
             addMatchedDeclaration(m_matchedRules[i]->rule()->declaration());
+        }
     } else {
         for (unsigned i = 0; i < m_matchedRules.size(); i++) {
             if (!m_ruleList)
@@ -740,10 +752,10 @@ Node* CSSStyleSelector::locateCousinList(Element* parent, unsigned& visitedNodeC
     return 0;
 }
 
-bool CSSStyleSelector::matchesSiblingRules()
+bool CSSStyleSelector::matchesRuleSet(RuleSet* ruleSet)
 {
     int firstSiblingRule = -1, lastSiblingRule = -1;
-    matchRules(m_features.siblingRules.get(), firstSiblingRule, lastSiblingRule, false);
+    matchRules(ruleSet, firstSiblingRule, lastSiblingRule, false);
     if (m_matchedDecls.isEmpty())
         return false;
     m_matchedDecls.clear();
@@ -827,7 +839,7 @@ bool CSSStyleSelector::canShareStyleWithElement(Node* node) const
         return false;
     if (element->isLink() != m_element->isLink())
         return false;
-    if (style->affectedByAttributeSelectors())
+    if (style->affectedByUncommonAttributeSelectors())
         return false;
     if (element->hovered() != m_element->hovered())
         return false;
@@ -911,7 +923,7 @@ static inline bool parentStylePreventsSharing(const RenderStyle* parentStyle)
         || parentStyle->childrenAffectedByLastChildRules();
 }
 
-ALWAYS_INLINE RenderStyle* CSSStyleSelector::locateSharedStyle()
+RenderStyle* CSSStyleSelector::locateSharedStyle()
 {
     if (!m_styledElement || !m_parentStyle)
         return 0;
@@ -941,7 +953,10 @@ ALWAYS_INLINE RenderStyle* CSSStyleSelector::locateSharedStyle()
         return 0;
 
     // Can't share if sibling rules apply. This is checked at the end as it should rarely fail.
-    if (matchesSiblingRules())
+    if (matchesRuleSet(m_features.siblingRules.get()))
+        return 0;
+    // Can't share if attribute rules apply.
+    if (matchesRuleSet(m_features.uncommonAttributeRules.get()))
         return 0;
     // Tracking child index requires unique style for each node. This may get set by the sibling rule match above.
     if (parentStylePreventsSharing(m_parentStyle))
@@ -1112,6 +1127,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
     if (simpleDefaultStyleSheet && !elementCanUseSimpleDefaultStyle(e)) {
         loadFullDefaultStyle();
         assertNoSiblingRulesInDefaultStyle();
+        collectSpecialRulesInDefaultStyle();
     }
 
 #if ENABLE(SVG)
@@ -1123,6 +1139,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         defaultStyle->addRulesFromSheet(svgSheet, screenEval());
         defaultPrintStyle->addRulesFromSheet(svgSheet, printEval());
         assertNoSiblingRulesInDefaultStyle();
+        collectSpecialRulesInDefaultStyle();
     }
 #endif
 
@@ -1134,8 +1151,8 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         CSSStyleSheet* mathMLSheet = parseUASheet(mathmlUserAgentStyleSheet, sizeof(mathmlUserAgentStyleSheet));
         defaultStyle->addRulesFromSheet(mathMLSheet, screenEval());
         defaultPrintStyle->addRulesFromSheet(mathMLSheet, printEval());
-        // There are some sibling rules here.
-        collectSiblingRulesInDefaultStyle();
+        // There are some sibling and uncommon attribute rules here.
+        collectSpecialRulesInDefaultStyle();
     }
 #endif
 
@@ -1147,7 +1164,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         CSSStyleSheet* mediaControlsSheet = parseUASheet(mediaRules);
         defaultStyle->addRulesFromSheet(mediaControlsSheet, screenEval());
         defaultPrintStyle->addRulesFromSheet(mediaControlsSheet, printEval());
-        assertNoSiblingRulesInDefaultStyle();
+        collectSpecialRulesInDefaultStyle();
     }
 #endif
 
@@ -1159,6 +1176,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         CSSStyleSheet* fullscreenSheet = parseUASheet(fullscreenRules);
         defaultStyle->addRulesFromSheet(fullscreenSheet, screenEval());
         defaultQuirksStyle->addRulesFromSheet(fullscreenSheet, screenEval());
+        collectSpecialRulesInDefaultStyle();
     }
 #endif
 
@@ -1838,6 +1856,46 @@ static inline bool isSelectorMatchingHTMLBasedOnRuleHash(const CSSSelector* sele
     return selector->m_match == CSSSelector::Id || selector->m_match == CSSSelector::Class;
 }
 
+static inline bool selectorListContainsUncommonAttributeSelector(const CSSSelector* selector)
+{
+    CSSSelectorList* selectorList = selector->selectorList();
+    if (!selectorList)
+        return false;
+    for (CSSSelector* subSelector = selectorList->first(); subSelector; subSelector = CSSSelectorList::next(subSelector)) {
+        if (subSelector->isAttributeSelector())
+            return true;
+    }
+    return false;
+}
+
+static inline bool isCommonAttributeSelectorAttribute(const QualifiedName& attribute)
+{
+    // These are explicitly tested for equality in canShareStyleWithElement.
+    return attribute == typeAttr || attribute == readonlyAttr;
+}
+
+static inline bool containsUncommonAttributeSelector(const CSSSelector* selector)
+{
+    while (selector) {
+        // Allow certain common attributes (used in the default style) in the selectors that match the current element.
+        if (selector->isAttributeSelector() && !isCommonAttributeSelectorAttribute(selector->attribute()))
+            return true;
+        if (selectorListContainsUncommonAttributeSelector(selector))
+            return true;
+        if (selector->relation() != CSSSelector::SubSelector)
+            break;
+        selector = selector->tagHistory();
+    };
+
+    for (selector = selector->tagHistory(); selector; selector = selector->tagHistory()) {            
+        if (selector->isAttributeSelector())
+            return true;
+        if (selectorListContainsUncommonAttributeSelector(selector))
+            return true;
+    }
+    return false;
+}
+
 RuleData::RuleData(CSSStyleRule* rule, CSSSelector* selector, unsigned position)
     : m_rule(rule)
     , m_selector(selector)
@@ -1846,6 +1904,7 @@ RuleData::RuleData(CSSStyleRule* rule, CSSSelector* selector, unsigned position)
     , m_hasFastCheckableSelector(SelectorChecker::isFastCheckableSelector(selector))
     , m_hasMultipartSelector(selector->tagHistory())
     , m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash(isSelectorMatchingHTMLBasedOnRuleHash(selector))
+    , m_containsUncommonAttributeSelector(WebCore::containsUncommonAttributeSelector(selector))
 {
     SelectorChecker::collectIdentifierHashes(m_selector, m_descendantSelectorIdentifierHashes, maximumIdentifierCount);
 }
@@ -1995,21 +2054,8 @@ static inline void collectFeaturesFromSelector(CSSStyleSelector::Features& featu
 {
     if (selector->m_match == CSSSelector::Id && !selector->value().isEmpty())
         features.idsInRules.add(selector->value().impl());
-    if (selector->hasAttribute()) {
-        switch (selector->m_match) {
-        case CSSSelector::Exact:
-        case CSSSelector::Set:
-        case CSSSelector::List:
-        case CSSSelector::Hyphen:
-        case CSSSelector::Contain:
-        case CSSSelector::Begin:
-        case CSSSelector::End:
-            features.attrsInRules.add(selector->attribute().localName().impl());
-            break;
-        default:
-            break;
-        }
-    }
+    if (selector->isAttributeSelector())
+        features.attrsInRules.add(selector->attribute().localName().impl());
     switch (selector->pseudoType()) {
     case CSSSelector::PseudoFirstLine:
         features.usesFirstLineRules = true;
@@ -2050,6 +2096,11 @@ static void collectFeaturesFromList(CSSStyleSelector::Features& features, const
                 features.siblingRules = adoptPtr(new RuleSet);
             features.siblingRules->addRule(ruleData.rule(), ruleData.selector());   
         }
+        if (ruleData.containsUncommonAttributeSelector()) {
+            if (!features.uncommonAttributeRules)
+                features.uncommonAttributeRules = adoptPtr(new RuleSet);
+            features.uncommonAttributeRules->addRule(ruleData.rule(), ruleData.selector()); 
+        }
     }
 }
 
index 4a44aa1..a758a27 100644 (file)
@@ -122,7 +122,7 @@ private:
     void initForStyleResolve(Element*, RenderStyle* parentStyle = 0, PseudoId = NOPSEUDO);
     void initElement(Element*);
     RenderStyle* locateSharedStyle();
-    bool matchesSiblingRules();
+    bool matchesRuleSet(RuleSet*);
     Node* locateCousinList(Element* parent, unsigned& visitedNodeCount) const;
     Node* findSiblingForStyleSharing(Node*, unsigned& count) const;
     bool canShareStyleWithElement(Node*) const;
@@ -202,6 +202,7 @@ public:
         HashSet<AtomicStringImpl*> idsInRules;
         HashSet<AtomicStringImpl*> attrsInRules;
         OwnPtr<RuleSet> siblingRules;
+        OwnPtr<RuleSet> uncommonAttributeRules;
         bool usesFirstLineRules;
         bool usesBeforeAfterRules;
         bool usesLinkRules;
index 733ae11..6307f0e 100644 (file)
@@ -686,10 +686,6 @@ bool SelectorChecker::checkOneSelector(CSSSelector* sel, Element* e, PseudoId& d
         
         const QualifiedName& attr = sel->attribute();
         
-        // FIXME: Handle the case were elementStyle is 0.
-        if (elementStyle && (!e->isStyledElement() || (!static_cast<StyledElement*>(e)->isMappedAttribute(attr) && attr != typeAttr && attr != readonlyAttr)))
-            elementStyle->setAffectedByAttributeSelectors(); // Special-case the "type" and "readonly" attributes so input form controls can share style.
-        
         NamedNodeMap* attributes = e->attributes(true);
         if (!attributes)
             return false;
index 6c0d385..5c74b72 100644 (file)
@@ -71,7 +71,7 @@ PassRefPtr<RenderStyle> RenderStyle::clone(const RenderStyle* other)
 }
 
 ALWAYS_INLINE RenderStyle::RenderStyle()
-    : m_affectedByAttributeSelectors(false)
+    : m_affectedByUncommonAttributeSelectors(false)
     , m_unique(false)
     , m_affectedByEmpty(false)
     , m_emptyState(false)
@@ -98,7 +98,7 @@ ALWAYS_INLINE RenderStyle::RenderStyle()
 }
 
 ALWAYS_INLINE RenderStyle::RenderStyle(bool)
-    : m_affectedByAttributeSelectors(false)
+    : m_affectedByUncommonAttributeSelectors(false)
     , m_unique(false)
     , m_affectedByEmpty(false)
     , m_emptyState(false)
@@ -138,7 +138,7 @@ ALWAYS_INLINE RenderStyle::RenderStyle(bool)
 
 ALWAYS_INLINE RenderStyle::RenderStyle(const RenderStyle& o)
     : RefCounted<RenderStyle>()
-    , m_affectedByAttributeSelectors(false)
+    , m_affectedByUncommonAttributeSelectors(false)
     , m_unique(false)
     , m_affectedByEmpty(false)
     , m_emptyState(false)
index d15a3e2..4f62722 100644 (file)
@@ -127,7 +127,7 @@ protected:
 
     // The following bitfield is 32-bits long, which optimizes padding with the
     // int refCount in the base class. Beware when adding more bits.
-    bool m_affectedByAttributeSelectors : 1;
+    bool m_affectedByUncommonAttributeSelectors : 1;
     bool m_unique : 1;
 
     // Bits for dynamic child matching.
@@ -1306,8 +1306,8 @@ public:
     void setWritingMode(WritingMode v) { inherited_flags.m_writingMode = v; }
 
     // To tell if this style matched attribute selectors. This makes it impossible to share.
-    bool affectedByAttributeSelectors() const { return m_affectedByAttributeSelectors; }
-    void setAffectedByAttributeSelectors() { m_affectedByAttributeSelectors = true; }
+    bool affectedByUncommonAttributeSelectors() const { return m_affectedByUncommonAttributeSelectors; }
+    void setAffectedByUncommonAttributeSelectors() { m_affectedByUncommonAttributeSelectors = true; }
 
     bool affectedByDirectAdjacentRules() const { return m_affectedByDirectAdjacentRules; }
     void setAffectedByDirectAdjacentRules() { m_affectedByDirectAdjacentRules = true; }