Optimizations to Element::getAttribute
authorsnej@chromium.org <snej@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Nov 2009 22:03:20 +0000 (22:03 +0000)
committersnej@chromium.org <snej@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Nov 2009 22:03:20 +0000 (22:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=30926

Reviewed by Darin Adler.

* dom/Element.cpp:
(WebCore::Element::getAttribute):  User case-insensitive compare instead of lowercasing the name.
* dom/NamedAttrMap.cpp:
(WebCore::NamedNodeMap::getAttributeItem):  Avoid redundant compares, and do fast/likely compares first.
* platform/text/PlatformString.h:
(WebCore::equalPossiblyIgnoringCase):  New inline method, used by both of the above.

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

WebCore/ChangeLog
WebCore/dom/Element.cpp
WebCore/dom/NamedAttrMap.cpp
WebCore/platform/text/PlatformString.h

index a3305f02a37e32f7426ec36db4714726501f31ff..ce6482423701c2cbe729b1499fc7fe8ff2c10410 100644 (file)
@@ -1,3 +1,17 @@
+2009-11-10  Jens Alfke  <snej@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Optimizations to Element::getAttribute
+        https://bugs.webkit.org/show_bug.cgi?id=30926
+
+        * dom/Element.cpp:
+        (WebCore::Element::getAttribute):  User case-insensitive compare instead of lowercasing the name.
+        * dom/NamedAttrMap.cpp:
+        (WebCore::NamedNodeMap::getAttributeItem):  Avoid redundant compares, and do fast/likely compares first.
+        * platform/text/PlatformString.h:
+        (WebCore::equalPossiblyIgnoringCase):  New inline method, used by both of the above.
+
 2009-11-10  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Darin Adler.
index 8513b264e679f47d46414e7d90dc72c218c9e1d0..4ade4393cfb3e02b9079e76bc894215e7a09918e 100644 (file)
@@ -489,8 +489,10 @@ static inline bool shouldIgnoreAttributeCase(const Element* e)
 
 const AtomicString& Element::getAttribute(const String& name) const
 {
-    String localName = shouldIgnoreAttributeCase(this) ? name.lower() : name;
-    if (localName == styleAttr.localName() && !m_isStyleAttributeValid)
+    bool ignoreCase = shouldIgnoreAttributeCase(this);
+    
+    // Update the 'style' attribute if it's invalid and being requested:
+    if (!m_isStyleAttributeValid && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
         updateStyleAttribute();
 
 #if ENABLE(SVG)
@@ -499,8 +501,8 @@ const AtomicString& Element::getAttribute(const String& name) const
 #endif
 
     if (namedAttrMap)
-        if (Attribute* a = namedAttrMap->getAttributeItem(name, shouldIgnoreAttributeCase(this)))
-            return a->value();
+        if (Attribute* attribute = namedAttrMap->getAttributeItem(name, ignoreCase))
+            return attribute->value();
     
     return nullAtom;
 }
index d4ec598d88565a723b9ce2225ace81f7bbf8f19b..56b40b97a5287a5f205f62ad31b105b7723865da 100644 (file)
@@ -177,11 +177,33 @@ PassRefPtr<Node> NamedNodeMap::item(unsigned index) const
 Attribute* NamedNodeMap::getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const
 {
     unsigned len = length();
+    bool doSlowCheck = shouldIgnoreAttributeCase;
+    
+    // Optimize for the case where the attribute exists and its name exactly matches.
     for (unsigned i = 0; i < len; ++i) {
-        if (!m_attributes[i]->name().hasPrefix() && m_attributes[i]->name().localName() == name)
-            return m_attributes[i].get();
-        if (shouldIgnoreAttributeCase ? equalIgnoringCase(m_attributes[i]->name().toString(), name) : name == m_attributes[i]->name().toString())
-            return m_attributes[i].get();
+        const QualifiedName& attrName = m_attributes[i]->name();
+        if (!attrName.hasPrefix()) {
+            if (name == attrName.localName())
+                return m_attributes[i].get();
+        } else
+            doSlowCheck = true;
+    }
+    
+    // Continue to checking case-insensitively and/or full namespaced names if necessary:
+    if (doSlowCheck) {
+        for (unsigned i = 0; i < len; ++i) {
+            const QualifiedName& attrName = m_attributes[i]->name();
+            if (!attrName.hasPrefix()) {
+                if (shouldIgnoreAttributeCase && equalIgnoringCase(name, attrName.localName()))
+                    return m_attributes[i].get();
+            } else {
+                // FIXME: Would be faster to do this comparison without calling toString, which
+                // generates a temporary string by concatenation. But this branch is only reached
+                // if the attribute name has a prefix, which is rare in HTML.
+                if (equalPossiblyIgnoringCase(name, attrName.toString(), shouldIgnoreAttributeCase))
+                    return m_attributes[i].get();
+            }
+        }
     }
     return 0;
 }
index ea47d8ee3a26bf2a86bb9d7f5a754d6d4210ed92..247536aa4bcfc7b808be9adc9a5741120d996bf4 100644 (file)
@@ -286,6 +286,11 @@ inline bool equalIgnoringCase(const String& a, const String& b) { return equalIg
 inline bool equalIgnoringCase(const String& a, const char* b) { return equalIgnoringCase(a.impl(), b); }
 inline bool equalIgnoringCase(const char* a, const String& b) { return equalIgnoringCase(a, b.impl()); }
 
+inline bool equalPossiblyIgnoringCase(const String& a, const String& b, bool ignoreCase) 
+{
+    return ignoreCase ? equalIgnoringCase(a, b) : (a == b);
+}
+
 inline bool equalIgnoringNullity(const String& a, const String& b) { return equalIgnoringNullity(a.impl(), b.impl()); }
 
 inline bool operator!(const String& str) { return str.isNull(); }