Remove caching of mapped attribute count on NamedNodeMap.
authorkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Jan 2012 20:28:30 +0000 (20:28 +0000)
committerkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Jan 2012 20:28:30 +0000 (20:28 +0000)
<http://webkit.org/b/76393>

Reviewed by Antti Koivisto.

Stop caching the mapped attribute count on Element's attribute map, effectively
shrinking NamedNodeMap by one CPU word. The price we pay is always walking over
the map in matchAllRules(), even if it has no mapped attribute styles.

This reduces memory consumption by 605 kB (on 64-bit) when viewing the full
HTML5 spec at <http://whatwg.org/c>

* css/CSSStyleSelector.cpp:
(WebCore::mappedAttributesEquivalent):

    Moved here from NamedNodeMap::mappedMapsEquivalent() to accomodate the only
    user under the added assumption that the two attribute maps have the same
    number of mapped attributes.

(WebCore::CSSStyleSelector::matchAllRules):

    We don't have NamedNodeMap::hasMappedAttributes() at our convenience any
    more so walk the attribute map (if there is one) looking for styles to add.

(WebCore::CSSStyleSelector::canShareStyleWithElement):

    Compare the elements' mapped attribute counts at an earlier time. This is
    slightly more expensive but we used to do it near the end anyway and this
    ends up rejecting elements that can't share style before doing a lot of
    semi-expensive checks.

* dom/StyledElement.h:
(WebCore::StyledElement::mappedAttributeCount):
* dom/NamedNodeMap.h:
(WebCore::NamedNodeMap::mappedAttributeCount):
* dom/NamedNodeMap.cpp:
(WebCore::NamedNodeMap::clearAttributes):

    Remove NamedNodeMap::m_mappedAttributeCount and add a function that walks
    the attribute map counting the attributes that have a decl().

* dom/NamedNodeMap.h:
* dom/StyledElement.cpp:
(WebCore::StyledElement::attributeChanged):

    Remove declAdded()/declRemoved() callbacks.

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSStyleSelector.cpp
Source/WebCore/dom/NamedNodeMap.cpp
Source/WebCore/dom/NamedNodeMap.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/dom/StyledElement.h

index 93be6d7..48dcc4c 100644 (file)
@@ -1,3 +1,52 @@
+2012-01-16  Andreas Kling  <awesomekling@apple.com>
+
+        Remove caching of mapped attribute count on NamedNodeMap.
+        <http://webkit.org/b/76393>
+
+        Reviewed by Antti Koivisto.
+
+        Stop caching the mapped attribute count on Element's attribute map, effectively
+        shrinking NamedNodeMap by one CPU word. The price we pay is always walking over
+        the map in matchAllRules(), even if it has no mapped attribute styles.
+
+        This reduces memory consumption by 605 kB (on 64-bit) when viewing the full
+        HTML5 spec at <http://whatwg.org/c>
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::mappedAttributesEquivalent):
+
+            Moved here from NamedNodeMap::mappedMapsEquivalent() to accomodate the only
+            user under the added assumption that the two attribute maps have the same
+            number of mapped attributes.
+
+        (WebCore::CSSStyleSelector::matchAllRules):
+
+            We don't have NamedNodeMap::hasMappedAttributes() at our convenience any
+            more so walk the attribute map (if there is one) looking for styles to add.
+
+        (WebCore::CSSStyleSelector::canShareStyleWithElement):
+
+            Compare the elements' mapped attribute counts at an earlier time. This is
+            slightly more expensive but we used to do it near the end anyway and this
+            ends up rejecting elements that can't share style before doing a lot of
+            semi-expensive checks.
+
+        * dom/StyledElement.h:
+        (WebCore::StyledElement::mappedAttributeCount):
+        * dom/NamedNodeMap.h:
+        (WebCore::NamedNodeMap::mappedAttributeCount):
+        * dom/NamedNodeMap.cpp:
+        (WebCore::NamedNodeMap::clearAttributes):
+
+            Remove NamedNodeMap::m_mappedAttributeCount and add a function that walks
+            the attribute map counting the attributes that have a decl().
+
+        * dom/NamedNodeMap.h:
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::attributeChanged):
+
+            Remove declAdded()/declRemoved() callbacks.
+
 2012-01-16  Jer Noble  <jer.noble@apple.com>
 
         Unreviewed build fix.
index c612533..c11e778 100644 (file)
@@ -817,22 +817,20 @@ void CSSStyleSelector::matchAllRules(MatchResult& result)
         
     // Now check author rules, beginning first with presentational attributes mapped from HTML.
     if (m_styledElement) {
-        // Ask if the HTML element has mapped attributes.
-        if (m_styledElement->hasMappedAttributes()) {
-            // Walk our attribute list and add in each decl.
-            const NamedNodeMap* map = m_styledElement->attributeMap();
+        if (const NamedNodeMap* map = m_styledElement->attributeMap()) {
+            // Walk the element's attribute map and add all mapped attribute declarations.
             for (unsigned i = 0; i < map->length(); ++i) {
-                Attribute* attr = map->attributeItem(i);
-                if (attr->decl()) {
-                    ASSERT(attr->isMappedAttribute());
-                    result.lastAuthorRule = m_matchedDecls.size();
-                    if (result.firstAuthorRule == -1)
-                        result.firstAuthorRule = result.lastAuthorRule;
-                    addMatchedDeclaration(attr->decl());
-                }
+                Attribute* attribute = map->attributeItem(i);
+                if (!attribute->decl())
+                    continue;
+                ASSERT(attribute->isMappedAttribute());
+                result.lastAuthorRule = m_matchedDecls.size();
+                if (result.firstAuthorRule == -1)
+                    result.firstAuthorRule = result.lastAuthorRule;
+                addMatchedDeclaration(attribute->decl());
             }
         }
-        
+
         // Now we check additional mapped declarations.
         // Tables and table cells share an additional mapped rule that must be applied
         // after all attributes, since their mapped style depends on the values of multiple attributes.
@@ -1056,6 +1054,23 @@ bool CSSStyleSelector::canShareStyleWithControl(StyledElement* element) const
     return true;
 }
 
+static inline bool mappedAttributesEquivalent(NamedNodeMap* a, NamedNodeMap* b)
+{
+    ASSERT(a->mappedAttributeCount() == b->mappedAttributeCount());
+
+    for (size_t i = 0; i < a->length(); ++i) {
+        Attribute* attribute = a->attributeItem(i);
+        if (!attribute->decl())
+            continue;
+        ASSERT(attribute->isMappedAttribute());
+        Attribute* otherAttribute = b->getAttributeItem(attribute->name());
+        if (!otherAttribute || attribute->value() != otherAttribute->value())
+            return false;
+        ASSERT(attribute->decl() == otherAttribute->decl());
+    }
+    return true;
+}
+
 bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
 {
     RenderStyle* style = element->renderStyle();
@@ -1070,7 +1085,8 @@ bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
         return false;
     if (element->inlineStyleDecl())
         return false;
-    if (element->hasMappedAttributes() != m_styledElement->hasMappedAttributes())
+    size_t mappedAttributeCount = element->mappedAttributeCount();
+    if (mappedAttributeCount != m_styledElement->mappedAttributeCount())
         return false;
     if (element->isLink() != m_element->isLink())
         return false;
@@ -1131,7 +1147,7 @@ bool CSSStyleSelector::canShareStyleWithElement(StyledElement* element) const
     if (element->hasClass() && m_element->getAttribute(classAttr) != element->getAttribute(classAttr))
         return false;
 
-    if (element->hasMappedAttributes() && !element->attributeMap()->mappedMapsEquivalent(m_styledElement->attributeMap()))
+    if (mappedAttributeCount && !mappedAttributesEquivalent(element->attributeMap(), m_styledElement->attributeMap()))
         return false;
 
     if (element->isLink() && m_elementLinkState != style->insideLink())
index c4a87ef..1b6aca0 100644 (file)
@@ -201,8 +201,6 @@ size_t NamedNodeMap::getAttributeItemIndexSlowCase(const String& name, bool shou
 void NamedNodeMap::clearAttributes()
 {
     m_classNames.clear();
-    m_mappedAttributeCount = 0;
-
     detachAttributesFromElement();
     m_attributes.clear();
 }
@@ -332,23 +330,4 @@ bool NamedNodeMap::mapsEquivalent(const NamedNodeMap* otherMap) const
     return true;
 }
 
-bool NamedNodeMap::mappedMapsEquivalent(const NamedNodeMap* otherMap) const
-{
-    if (m_mappedAttributeCount != otherMap->m_mappedAttributeCount)
-        return false;
-
-    // The values for each decl must match.
-    for (unsigned i = 0; i < length(); i++) {
-        Attribute* attr = attributeItem(i);
-        if (attr->decl()) {
-            ASSERT(attr->isMappedAttribute());
-
-            Attribute* otherAttr = otherMap->getAttributeItem(attr->name());
-            if (!otherAttr || attr->decl() != otherAttr->decl() || attr->value() != otherAttr->value())
-                return false;
-        }
-    }
-    return true;
-}
-
 } // namespace WebCore
index 40e9a59..0905d64 100644 (file)
@@ -87,9 +87,7 @@ public:
     const AtomicString& idForStyleResolution() const { return m_idForStyleResolution; }
     void setIdForStyleResolution(const AtomicString& newId) { m_idForStyleResolution = newId; }
 
-    // FIXME: These two functions should be merged if possible.
     bool mapsEquivalent(const NamedNodeMap* otherMap) const;
-    bool mappedMapsEquivalent(const NamedNodeMap* otherMap) const;
 
     // These functions do no error checking.
     void addAttribute(PassRefPtr<Attribute>);
@@ -102,14 +100,11 @@ public:
     void setClass(const String&);
     const SpaceSplitString& classNames() const { return m_classNames; }
 
-    bool hasMappedAttributes() const { return m_mappedAttributeCount > 0; }
-    void declRemoved() { m_mappedAttributeCount--; }
-    void declAdded() { m_mappedAttributeCount++; }
+    size_t mappedAttributeCount() const;
 
 private:
-    NamedNodeMap(Element* element) 
-        : m_mappedAttributeCount(0)
-        , m_element(element)
+    NamedNodeMap(Element* element)
+        : m_element(element)
     {
     }
 
@@ -122,7 +117,6 @@ private:
     void clearAttributes();
     void replaceAttribute(size_t index, PassRefPtr<Attribute>);
 
-    int m_mappedAttributeCount;
     SpaceSplitString m_classNames;
     Element* m_element;
     Vector<RefPtr<Attribute>, 4> m_attributes;
@@ -186,6 +180,16 @@ inline void NamedNodeMap::removeAttribute(const QualifiedName& name)
     removeAttribute(index);
 }
 
+inline size_t NamedNodeMap::mappedAttributeCount() const
+{
+    size_t count = 0;
+    for (size_t i = 0; i < m_attributes.size(); ++i) {
+        if (m_attributes[i]->decl())
+            ++count;
+    }
+    return count;
+}
+
 } // namespace WebCore
 
 #endif // NamedNodeMap_h
index 2bc86ae..1e51313 100644 (file)
@@ -155,8 +155,6 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
     if (attr->decl() && !preserveDecls) {
         attr->setDecl(0);
         setNeedsStyleRecalc();
-        if (attributeMap())
-            attributeMap()->declRemoved();
     }
 
     bool checkDecl = true;
@@ -165,8 +163,6 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
     if (preserveDecls) {
         if (attr->decl()) {
             setNeedsStyleRecalc();
-            if (attributeMap())
-                attributeMap()->declAdded();
             checkDecl = false;
         }
     } else if (!attr->isNull() && entry != eNone) {
@@ -174,8 +170,6 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
         if (decl) {
             attr->setDecl(decl);
             setNeedsStyleRecalc();
-            if (attributeMap())
-                attributeMap()->declAdded();
             checkDecl = false;
         } else
             needToParse = true;
@@ -196,8 +190,6 @@ void StyledElement::attributeChanged(Attribute* attr, bool preserveDecls)
         // Add the decl to the table in the appropriate spot.
         setMappedAttributeDecl(entry, attr, attr->decl());
         attr->decl()->setMappedState(entry, attr->name(), attr->value());
-        if (attributeMap())
-            attributeMap()->declAdded();
     }
 
     updateAfterAttributeChanged(attr);
index 3e62926..c7625eb 100644 (file)
@@ -38,7 +38,7 @@ class StyledElement : public Element {
 public:
     virtual ~StyledElement();
 
-    bool hasMappedAttributes() const { return attributeMap() && attributeMap()->hasMappedAttributes(); }
+    size_t mappedAttributeCount() const { return attributeMap() && attributeMap()->mappedAttributeCount(); }
     bool isMappedAttribute(const QualifiedName& name) const { MappedAttributeEntry res = eNone; mapToEntry(name, res); return res != eNone; }
 
     void addCSSLength(Attribute*, int id, const String& value);