Changeset 116419 in webkit

Timestamp:

May 8, 2012 7:09:37 AM (9 years ago)

Author:

kling@webkit.org

Message:

Shrink ElementAttributeData by factoring out Attr object count.
<​http://webkit.org/b/85825>

Reviewed by Antti Koivisto.

Stop tracking the number of Attr objects that point to a given Element on the
Element itself and manage this by having a global hashmap of Element => AttrList,
where AttrList is a vector of (pointers to) the associated Attr objects.

This shrinks ElementAttributeData by one integer, effectively reducing memory
consumption by ~530kB when viewing the full HTML5 spec at <​http://whatwg.org/c>.

• dom/ElementAttributeData.h:

(ElementAttributeData):

Remove m_attrCount...

• dom/Node.h:

(WebCore::Node::hasAttrList):
(WebCore::Node::setHasAttrList):
(WebCore::Node::clearHasAttrList):

...replacing it with a Node flag that tells us whether there's an Attr
object map for this Node (only applies to Elements.)

• dom/ElementAttributeData.cpp:

(WebCore::attrListMap):
(WebCore::attrListForElement):
(WebCore::ensureAttrListForElement):
(WebCore::removeAttrListForElement):
(WebCore::ElementAttributeData::attrIfExists):
(WebCore::ElementAttributeData::ensureAttr):
(WebCore::ElementAttributeData::setAttr):
(WebCore::ElementAttributeData::removeAttr):
(WebCore::ElementAttributeData::detachAttributesFromElement):

Map Element => per-Element AttrList in a global hash.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ElementAttributeData.cpp (modified) (3 diffs)
• dom/ElementAttributeData.h (modified) (2 diffs)
• dom/Node.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-08  Andreas Kling  <kling@webkit.org>
+
+        Shrink ElementAttributeData by factoring out Attr object count.
+        <http://webkit.org/b/85825>
+
+        Reviewed by Antti Koivisto.
+
+        Stop tracking the number of Attr objects that point to a given Element on the
+        Element itself and manage this by having a global hashmap of Element => AttrList,
+        where AttrList is a vector of (pointers to) the associated Attr objects.
+
+        This shrinks ElementAttributeData by one integer, effectively reducing memory
+        consumption by ~530kB when viewing the full HTML5 spec at <http://whatwg.org/c>.
+
+        * dom/ElementAttributeData.h:
+        (ElementAttributeData):
+
+            Remove m_attrCount...
+
+        * dom/Node.h:
+        (WebCore::Node::hasAttrList):
+        (WebCore::Node::setHasAttrList):
+        (WebCore::Node::clearHasAttrList):
+
+            ...replacing it with a Node flag that tells us whether there's an Attr
+            object map for this Node (only applies to Elements.)
+
+        * dom/ElementAttributeData.cpp:
+        (WebCore::attrListMap):
+        (WebCore::attrListForElement):
+        (WebCore::ensureAttrListForElement):
+        (WebCore::removeAttrListForElement):
+        (WebCore::ElementAttributeData::attrIfExists):
+        (WebCore::ElementAttributeData::ensureAttr):
+        (WebCore::ElementAttributeData::setAttr):
+        (WebCore::ElementAttributeData::removeAttr):
+        (WebCore::ElementAttributeData::detachAttributesFromElement):
+
+            Map Element => per-Element AttrList in a global hash.
+
 2012-05-08  Chris Guan  <chris.guan@torchmobile.com.cn>
 

trunk/Source/WebCore/dom/ElementAttributeData.cpp

@@ -33 +33 @@
 namespace WebCore {
 
-typedef HashMap<pair<Element*, QualifiedName>, RefPtr<Attr> > AttrMap;
-static AttrMap& attrMap()
-{
-    DEFINE_STATIC_LOCAL(AttrMap, map, ());
+typedef Vector<RefPtr<Attr> > AttrList;
+typedef HashMap<Element*, OwnPtr<AttrList> > AttrListMap;
+
+static AttrListMap& attrListMap()
+{
+    DEFINE_STATIC_LOCAL(AttrListMap, map, ());
     return map;
 }
 
+static AttrList* attrListForElement(Element* element)
+{
+    ASSERT(element);
+    if (!element->hasAttrList())
+        return false;
+    ASSERT(attrListMap().contains(element));
+    return attrListMap().get(element);
+}
+
+static AttrList* ensureAttrListForElement(Element* element)
+{
+    ASSERT(element);
+    if (element->hasAttrList()) {
+        ASSERT(attrListMap().contains(element));
+        return attrListMap().get(element);
+    }
+    ASSERT(!attrListMap().contains(element));
+    element->setHasAttrList();
+    AttrListMap::AddResult result = attrListMap().add(element, adoptPtr(new AttrList));
+    return result.iterator->second.get();
+}
+
+static void removeAttrListForElement(Element* element)
+{
+    ASSERT(element);
+    ASSERT(element->hasAttrList());
+    ASSERT(attrListMap().contains(element));
+    attrListMap().remove(element);
+    element->clearHasAttrList();
+}
+
+static Attr* findAttrInList(AttrList* attrList, const QualifiedName& name)
+{
+    for (unsigned i = 0; i < attrList->size(); ++i) {
+        if (attrList->at(i)->qualifiedName() == name)
+            return attrList->at(i).get();
+    }
+    return 0;
+}
+
 PassRefPtr<Attr> ElementAttributeData::attrIfExists(Element* element, const QualifiedName& name)
 {
-    if (!m_attrCount)
-        return 0;
-    return attrMap().get(std::make_pair(element, name)).get();
+    if (AttrList* attrList = attrListForElement(element))
+        return findAttrInList(attrList, name);
+    return 0;
 }
 
 PassRefPtr<Attr> ElementAttributeData::ensureAttr(Element* element, const QualifiedName& name)
 {
-    AttrMap::AddResult result = attrMap().add(std::make_pair(element, name), 0);
-    if (result.isNewEntry) {
-        result.iterator->second = Attr::create(element, name);
-        ++m_attrCount;
-    }
-    return result.iterator->second.get();
+    AttrList* attrList = ensureAttrListForElement(element);
+    RefPtr<Attr> attr = findAttrInList(attrList, name);
+    if (!attr) {
+        attr = Attr::create(element, name);
+        attrList->append(attr);
+    }
+    return attr.release();
 }
 
 void ElementAttributeData::setAttr(Element* element, const QualifiedName& name, Attr* attr)
 {
-    ASSERT(!attrMap().contains(std::make_pair(element, name)));
-    attrMap().add(std::make_pair(element, name), attr);
+    AttrList* attrList = ensureAttrListForElement(element);
+
+    if (findAttrInList(attrList, name))
+        return;
+
+    attrList->append(attr);
     attr->attachToElement(element);
-    ++m_attrCount;
 }
 
 void ElementAttributeData::removeAttr(Element* element, const QualifiedName& name)
 {
-    ASSERT(attrMap().contains(std::make_pair(element, name)));
-    attrMap().remove(std::make_pair(element, name));
-    --m_attrCount;
+    AttrList* attrList = attrListForElement(element);
+    ASSERT(attrList);
+
+    for (unsigned i = 0; i < attrList->size(); ++i) {
+        if (attrList->at(i)->qualifiedName() == name) {
+            attrList->remove(i);
+            if (attrList->isEmpty())
+                removeAttrListForElement(element);
+            return;
+        }
+    }
+
+    ASSERT_NOT_REACHED();
 }
 
@@ -171 +227 @@
 void ElementAttributeData::detachAttributesFromElement(Element* element)
 {
-    if (!m_attrCount)
+    if (!element->hasAttrList())
         return;
 
@@ -178 +234 @@
             attr->detachFromElementWithValue(m_attributes[i].value());
     }
+
+    // The loop above should have cleaned out this element's Attr map.
+    ASSERT(!element->hasAttrList());
 }
 

trunk/Source/WebCore/dom/ElementAttributeData.h

@@ -107 +107 @@
 
     ElementAttributeData()
-        : m_attrCount(0)
     {
     }
@@ -126 +125 @@
     AtomicString m_idForStyleResolution;
     Vector<Attribute> m_attributes;
-
-    unsigned m_attrCount;
 };
 

trunk/Source/WebCore/dom/Node.h

@@ -217 +217 @@
     bool isShadowRoot() const { return getFlag(IsShadowRootFlag); }
     bool inNamedFlow() const { return getFlag(InNamedFlowFlag); }
+    bool hasAttrList() const { return getFlag(HasAttrListFlag); }
 
     Node* shadowAncestorNode() const;
@@ -330 +331 @@
     void setInNamedFlow() { setFlag(InNamedFlowFlag); }
     void clearInNamedFlow() { clearFlag(InNamedFlowFlag); }
+
+    void setHasAttrList() { setFlag(HasAttrListFlag); }
+    void clearHasAttrList() { clearFlag(HasAttrListFlag); }
 
     enum ShouldSetAttached {
@@ -690 +694 @@
         DefaultNodeFlags = IsParsingChildrenFinishedFlag | IsStyleAttributeValidFlag,
 #endif
-        InNamedFlowFlag = 1 << 29
+        InNamedFlowFlag = 1 << 29,
+        HasAttrListFlag = 1 << 30
     };
 
-    // 3 bits remaining
+    // 2 bits remaining
 
     bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }