Reviewed by Geoff.

        Fix for <rdar://problem/5728171> Potential PLT speedup: don't
        realloc every time inside NamedAttrMap::addAttribute

        The speed-up for this turned out to be so small that it is mostly
        imperceptible. It is likely that it is a tiny boost, though, and
        the new code is much cleaner.

        * dom/Element.cpp:
        (WebCore::Element::setAttributeMap): attrs is now called
        m_attributes
        * dom/NamedAttrMap.cpp: The array attrs is now the Vector of
        RefPtrs called m_attributes, and there is no longer any need for
        the len member variable.
        (WebCore::NamedAttrMap::NamedAttrMap):
        (WebCore::NamedAttrMap::item):
        (WebCore::NamedAttrMap::getAttributeItem):
        (WebCore::NamedAttrMap::clearAttributes):
        (WebCore::NamedAttrMap::operator=):
        (WebCore::NamedAttrMap::addAttribute):
        (WebCore::NamedAttrMap::removeAttribute):
        (WebCore::NamedAttrMap::mapsEquivalent):
        * dom/NamedAttrMap.h: Same.
        (WebCore::NamedAttrMap::length):
        (WebCore::NamedAttrMap::attributeItem):
        (WebCore::NamedAttrMap::shrinkToLength):
        (WebCore::NamedAttrMap::reserveCapacity):
        * html/HTMLTokenizer.cpp: One of the benefits of the old array was
        that it never took up more memory than it needed to. So the
        tokenizer utilizes new member functions on NamedAttrMap
        (shrinkToLength and reserveCapacity) to try to keep memory usage at
        a minimum.
        (WebCore::Token::addAttribute):
        (WebCore::HTMLTokenizer::processToken):



Canonical link: https://commits.webkit.org/24743@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31060 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog

@@ -1,3 +1,41 @@
+2008-03-14  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Geoff.
+
+        Fix for <rdar://problem/5728171> Potential PLT speedup: don't 
+        realloc every time inside NamedAttrMap::addAttribute
+
+        The speed-up for this turned out to be so small that it is mostly 
+        imperceptible. It is likely that it is a tiny boost, though, and 
+        the new code is much cleaner.
+
+        * dom/Element.cpp:
+        (WebCore::Element::setAttributeMap): attrs is now called 
+        m_attributes
+        * dom/NamedAttrMap.cpp: The array attrs is now the Vector of 
+        RefPtrs called m_attributes, and there is no longer any need for 
+        the len member variable. 
+        (WebCore::NamedAttrMap::NamedAttrMap): 
+        (WebCore::NamedAttrMap::item):
+        (WebCore::NamedAttrMap::getAttributeItem):
+        (WebCore::NamedAttrMap::clearAttributes):
+        (WebCore::NamedAttrMap::operator=):
+        (WebCore::NamedAttrMap::addAttribute):
+        (WebCore::NamedAttrMap::removeAttribute):
+        (WebCore::NamedAttrMap::mapsEquivalent):
+        * dom/NamedAttrMap.h: Same.
+        (WebCore::NamedAttrMap::length):
+        (WebCore::NamedAttrMap::attributeItem):
+        (WebCore::NamedAttrMap::shrinkToLength):
+        (WebCore::NamedAttrMap::reserveCapacity):
+        * html/HTMLTokenizer.cpp: One of the benefits of the old array was 
+        that it never took up more memory than it needed to. So the 
+        tokenizer utilizes new member functions on NamedAttrMap 
+        (shrinkToLength and reserveCapacity) to try to keep memory usage at 
+        a minimum. 
+        (WebCore::Token::addAttribute):
+        (WebCore::HTMLTokenizer::processToken):
+
 2008-03-14  David D. Kilzer  <ddkilzer@apple.com>
 
         BUILD FIX when ENABLE(MAC_JAVA_BRIDGE) set to 0.

WebCore/dom/Element.cpp

@@ -531,7 +531,7 @@ void Element::setAttributeMap(NamedAttrMap* list)
         namedAttrMap->element = this;
         unsigned len = namedAttrMap->length();
         for (unsigned i = 0; i < len; i++)
-            attributeChanged(namedAttrMap->attrs[i]);
+            attributeChanged(namedAttrMap->m_attributes[i].get());
         // FIXME: What about attributes that were in the old map that are not in the new map?
     }
 }

WebCore/dom/NamedAttrMap.cpp

@@ -41,8 +41,6 @@ static inline bool shouldIgnoreAttributeCase(const Element* e)
 
 NamedAttrMap::NamedAttrMap(Element *e)
     : element(e)
-    , attrs(0)
-    , len(0)
 {
 }
 
@@ -176,48 +174,46 @@ PassRefPtr<Node> NamedAttrMap::removeNamedItem(const QualifiedName& name, Except
     return r.release();
 }
 
-PassRefPtr<Node> NamedAttrMap::item ( unsigned index ) const
+PassRefPtr<Node> NamedAttrMap::item (unsigned index) const
 {
-    if (index >= len)
+    if (index >= length())
         return 0;
 
-    return attrs[index]->createAttrIfNeeded(element);
+    return m_attributes[index]->createAttrIfNeeded(element);
 }
 
 Attribute* NamedAttrMap::getAttributeItem(const String& name) const
 {
+    unsigned len = length();
     for (unsigned i = 0; i < len; ++i) {
-        if (!attrs[i]->name().hasPrefix() && 
-            attrs[i]->name().localName() == name)
-                return attrs[i];
+        if (!m_attributes[i]->name().hasPrefix() && 
+            m_attributes[i]->name().localName() == name)
+                return m_attributes[i].get();
 
-        if (attrs[i]->name().toString() == name)
-            return attrs[i];
+        if (m_attributes[i]->name().toString() == name)
+            return m_attributes[i].get();
     }
     return 0;
 }
 
 Attribute* NamedAttrMap::getAttributeItem(const QualifiedName& name) const
 {
+    unsigned len = length();
     for (unsigned i = 0; i < len; ++i) {
-        if (attrs[i]->name().matches(name))
-            return attrs[i];
+        if (m_attributes[i]->name().matches(name))
+            return m_attributes[i].get();
     }
     return 0;
 }
 
 void NamedAttrMap::clearAttributes()
 {
-    if (attrs) {
-        for (unsigned i = 0; i < len; i++) {
-            if (attrs[i]->attr())
-                attrs[i]->attr()->m_element = 0;
-            attrs[i]->deref();
-        }
-        fastFree(attrs);
-        attrs = 0;
-    }
-    len = 0;
+    unsigned len = length();
+    for (unsigned i = 0; i < len; i++)
+        if (Attr* attr = m_attributes[i]->attr())
+            attr->m_element = 0;
+
+    m_attributes.clear();
 }
 
 void NamedAttrMap::detachFromElement()
@@ -236,87 +232,69 @@ NamedAttrMap& NamedAttrMap::operator=(const NamedAttrMap& other)
 
     // If assigning the map changes the id attribute, we need to call
     // updateId.
-
     Attribute *oldId = getAttributeItem(idAttr);
     Attribute *newId = other.getAttributeItem(idAttr);
 
     if (oldId || newId)
         element->updateId(oldId ? oldId->value() : nullAtom, newId ? newId->value() : nullAtom);
 
     clearAttributes();
-    len = other.len;
-    attrs = static_cast<Attribute **>(fastMalloc(len * sizeof(Attribute *)));
-
-    // first initialize attrs vector, then call attributeChanged on it
-    // this allows attributeChanged to use getAttribute
-    for (unsigned i = 0; i < len; i++) {
-        attrs[i] = other.attrs[i]->clone();
-        attrs[i]->ref();
-    }
+    unsigned newLength = other.length();
+    m_attributes.resize(newLength);
+    for (unsigned i = 0; i < newLength; i++)
+        m_attributes[i] = other.m_attributes[i]->clone();
 
     // FIXME: This is wasteful.  The class list could be preserved on a copy, and we
     // wouldn't have to waste time reparsing the attribute.
     // The derived class, HTMLNamedAttrMap, which manages a parsed class list for the CLASS attribute,
     // will update its member variable when parse attribute is called.
-    for(unsigned i = 0; i < len; i++)
-        element->attributeChanged(attrs[i], true);
+    for(unsigned i = 0; i < newLength; i++)
+        element->attributeChanged(m_attributes[i].get(), true);
 
     return *this;
 }
 
 void NamedAttrMap::addAttribute(PassRefPtr<Attribute> prpAttribute)
 {
-    Attribute* attribute = prpAttribute.releaseRef(); // The attrs array will own this pointer.
-
+    RefPtr<Attribute> attribute = prpAttribute;
+    
     // Add the attribute to the list
-    attrs = static_cast<Attribute**>(fastRealloc(attrs, (len + 1) * sizeof(Attribute*)));
-    attrs[len++] = attribute;
+    m_attributes.append(attribute);
 
     if (Attr* attr = attribute->attr())
         attr->m_element = element;
 
     // Notify the element that the attribute has been added, and dispatch appropriate mutation events
     // Note that element may be null here if we are called from insertAttr() during parsing
     if (element) {
-        element->attributeChanged(attribute);
+        element->attributeChanged(attribute.get());
         // Because of our updateStyleAttributeIfNeeded() style modification events are never sent at the right time, so don't bother sending them.
         if (attribute->name() != styleAttr) {
-            element->dispatchAttrAdditionEvent(attribute);
+            element->dispatchAttrAdditionEvent(attribute.get());
             element->dispatchSubtreeModifiedEvent();
         }
     }
 }
 
 void NamedAttrMap::removeAttribute(const QualifiedName& name)
 {
-    unsigned index = len+1;
+    unsigned len = length();
+    unsigned index = len + 1;
     for (unsigned i = 0; i < len; ++i)
-        if (attrs[i]->name().matches(name)) {
+        if (m_attributes[i]->name().matches(name)) {
             index = i;
             break;
         }
 
-    if (index >= len) return;
+    if (index >= len)
+        return;
 
     // Remove the attribute from the list
-    Attribute* attr = attrs[index];
-    if (attrs[index]->attr())
-        attrs[index]->attr()->m_element = 0;
-    if (len == 1) {
-        fastFree(attrs);
-        attrs = 0;
-        len = 0;
-    } else {
-        Attribute **newAttrs = static_cast<Attribute **>(fastMalloc((len - 1) * sizeof(Attribute *)));
-        unsigned i;
-        for (i = 0; i < unsigned(index); i++)
-            newAttrs[i] = attrs[i];
-        len--;
-        for (; i < len; i++)
-            newAttrs[i] = attrs[i+1];
-        fastFree(attrs);
-        attrs = newAttrs;
-    }
+    Attribute* attr = m_attributes[index].get();
+    if (Attr* a = m_attributes[index]->attr())
+        a->m_element = 0;
+
+    m_attributes.remove(index);
 
     // Notify the element that the attribute has been removed
     // dispatch appropriate mutation events
@@ -330,18 +308,18 @@ void NamedAttrMap::removeAttribute(const QualifiedName& name)
         element->dispatchAttrRemovalEvent(attr);
         element->dispatchSubtreeModifiedEvent();
     }
-    attr->deref();
 }
 
 bool NamedAttrMap::mapsEquivalent(const NamedAttrMap* otherMap) const
 {
     if (!otherMap)
         return false;
 
-    if (length() != otherMap->length())
+    unsigned len = length();
+    if (len != otherMap->length())
         return false;
 
-    for (unsigned i = 0; i < length(); i++) {
+    for (unsigned i = 0; i < len; i++) {
         Attribute *attr = attributeItem(i);
         Attribute *otherAttr = otherMap->getAttributeItem(attr->name());
 

WebCore/dom/NamedAttrMap.h

@@ -29,6 +29,8 @@
 
 #include "Attribute.h"
 #include "NamedNodeMap.h"
+#include <wtf/RefPtr.h>
+#include <wtf/Vector.h>
 
 #ifdef __OBJC__
 #define id id_AVOID_KEYWORD
@@ -58,13 +60,16 @@ class NamedAttrMap : public NamedNodeMap {
     virtual PassRefPtr<Node> setNamedItem(Node* arg, ExceptionCode&);
 
     virtual PassRefPtr<Node> item(unsigned index) const;
-    unsigned length() const { return len; }
+    unsigned length() const { return m_attributes.size(); }
 
     // Other methods (not part of DOM)
-    Attribute* attributeItem(unsigned index) const { return attrs[index]; }
+    Attribute* attributeItem(unsigned index) const { return m_attributes[index].get(); }
     Attribute* getAttributeItem(const QualifiedName& name) const;
     Attribute* getAttributeItem(const String& name) const;
     virtual bool isReadOnlyNode();
+
+    void shrinkToLength() { m_attributes.shrinkCapacity(length()); }
+    void reserveCapacity(unsigned capacity) { m_attributes.reserveCapacity(capacity); }
 
     // used during parsing: only inserts if not already there
     // no error checking!
@@ -91,8 +96,7 @@ class NamedAttrMap : public NamedNodeMap {
     void detachFromElement();
 
     Element *element;
-    Attribute **attrs;
-    unsigned len;
+    Vector<RefPtr<Attribute> > m_attributes;
     AtomicString m_id;
 };
 

WebCore/html/HTMLTokenizer.cpp

@@ -137,8 +137,10 @@ inline void Token::addAttribute(Document* doc, AtomicString& attrName, const Ato
     if (!attrName.isEmpty()) {
         ASSERT(!attrName.contains('/'));
         RefPtr<MappedAttribute> a = new MappedAttribute(attrName, v);
-        if (!attrs)
+        if (!attrs) {
             attrs = new NamedMappedAttrMap(0);
+            attrs->reserveCapacity(10);
+        }
         attrs->insertAttribute(a.release(), viewSourceMode);
     }
 
@@ -1885,6 +1887,8 @@ PassRefPtr<Node> HTMLTokenizer::processToken()
     RefPtr<Node> n;
 
     if (!m_parserStopped) {
+        if (NamedMappedAttrMap* map = currToken.attrs.get())
+            map->shrinkToLength();
         if (inViewSourceMode())
             static_cast<HTMLViewSourceDocument*>(m_doc)->addViewSourceToken(&currToken);
         else