Changeset 148961 in webkit

Timestamp:

Apr 23, 2013 7:21:05 AM (11 years ago)

Author:

akling@apple.com

Message:

Speed up ElementData::getAttributeItem(), which is hot.
<​http://webkit.org/b/115031>

Reviewed by Antti Koivisto.

From Blink r148622 by <​cevans@chromium.org>.

The previous version had a couple of issues:

• Incurred new WTF::Vector::operator[] checks.
• Had a branch in _every_ loop iteration that checked whether the storage was Vector or array[] backed.

Both these issues are fixed, and CloneNodes.html seems reliably at +2%.
I also believe GetElement.html is maybe +1%.

For WebKit, also applied the same optimization to getAttributeItemIndex().

• dom/Element.h:

(WebCore::ElementData::attributeBase):
(WebCore::ElementData::getAttributeItemIndex):
(WebCore::ElementData::getAttributeItem):
(WebCore::ElementData::attributeItem):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Element.h (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-04-23  Andreas Kling  <akling@apple.com>
+
+        Speed up ElementData::getAttributeItem(), which is hot.
+        <http://webkit.org/b/115031>
+
+        Reviewed by Antti Koivisto.
+
+        From Blink r148622 by <cevans@chromium.org>.
+
+        The previous version had a couple of issues:
+        - Incurred new WTF::Vector::operator[] checks.
+        - Had a branch in _every_ loop iteration that checked whether the storage was Vector or array[] backed.
+
+        Both these issues are fixed, and CloneNodes.html seems reliably at +2%.
+        I also believe GetElement.html is maybe +1%.
+
+        For WebKit, also applied the same optimization to getAttributeItemIndex().
+
+        * dom/Element.h:
+        (WebCore::ElementData::attributeBase):
+        (WebCore::ElementData::getAttributeItemIndex):
+        (WebCore::ElementData::getAttributeItem):
+        (WebCore::ElementData::attributeItem):
+
 2013-04-23  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/dom/Element.h

@@ -111 +111 @@
 #endif
 
+    const Attribute* attributeBase() const;
     const Attribute* getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
     size_t getAttributeItemIndexSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
@@ -941 +942 @@
 }
 
+inline const Attribute* ElementData::attributeBase() const
+{
+    if (m_isUnique)
+        return static_cast<const UniqueElementData*>(this)->m_attributeVector.data();
+    return static_cast<const ShareableElementData*>(this)->m_attributeArray;
+}
+
 inline const StylePropertySet* ElementData::presentationAttributeStyle() const
 {
@@ -958 +966 @@
 inline size_t ElementData::getAttributeItemIndex(const QualifiedName& name) const
 {
-    for (unsigned i = 0; i < length(); ++i) {
-        if (attributeItem(i)->name().matches(name))
+    const Attribute* attributes = attributeBase();
+    for (unsigned i = 0, count = length(); i < count; ++i) {
+        if (attributes[i].name().matches(name))
             return i;
     }
@@ -969 +978 @@
 inline size_t ElementData::getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const
 {
-    unsigned len = length();
+    const Attribute* attributes = attributeBase();
     bool doSlowCheck = shouldIgnoreAttributeCase;
-
-    const AtomicString caseAdjustedName = shouldIgnoreAttributeCase ? name.lower() : name;
+    const AtomicString& caseAdjustedName = shouldIgnoreAttributeCase ? name.lower() : name;
+
     // Optimize for the case where the attribute exists and its name exactly matches.
-    for (unsigned i = 0; i < len; ++i) {
-        const Attribute* attribute = attributeItem(i);
-        if (!attribute->name().hasPrefix()) {
-            if (caseAdjustedName == attribute->localName())
+    for (unsigned i = 0, count = length(); i < count; ++i) {
+        if (!attributes[i].name().hasPrefix()) {
+            if (caseAdjustedName == attributes[i].localName())
                 return i;
         } else
@@ -990 +998 @@
 inline const Attribute* ElementData::getAttributeItem(const QualifiedName& name) const
 {
-    for (unsigned i = 0; i < length(); ++i) {
-        if (attributeItem(i)->name().matches(name))
-            return attributeItem(i);
+    const Attribute* attributes = attributeBase();
+    for (unsigned i = 0, count = length(); i < count; ++i) {
+        if (attributes[i].name().matches(name))
+            return &attributes[i];
     }
     return 0;
@@ -1000 +1009 @@
 {
     ASSERT_WITH_SECURITY_IMPLICATION(index < length());
-    if (m_isUnique)
-        return &static_cast<const UniqueElementData*>(this)->m_attributeVector.at(index);
-    return &static_cast<const ShareableElementData*>(this)->m_attributeArray[index];
+    return attributeBase() + index;
 }