- Revision
- 121439
- Author
- [email protected]
- Date
- 2012-06-28 10:07:01 -0700 (Thu, 28 Jun 2012)
Log Message
Optimize Dromaeo/dom-attr.html by speeding up Element::getAttribute()
https://bugs.webkit.org/show_bug.cgi?id=90174
Reviewed by Adam Barth.
This patch improves performance of Dromaeo/dom-attr.html by 4.0%.
The patch improves performance of getAttribute() and _javascript_
property setter for DOM objects (e.g. 'div.foo = 123').
The performance improvement becomes larger, as the number of
attributes defined on the DOM object increases.
Without the patch in Chromium/Linux (runs/s)
7679.4, 7739.7, 7634.0, 7726.4, 7663.9
With the patch in Chromium/Linux (runs/s)
7977.7, 8032.2, 8112.8, 7948.1, 7924.5
This patch just changes a type of 'name' of Element::getAttribute(String& name)
from String& to AtomicString&.
The key observation is that AtomicString(String& x) is faster than
operator==(String& x, AtomicString& y). AtomicString(String& x) calculates
a hash of a given String and adds it to a hash table. The calculation
complexity is O(the length of x). On the other hand,
operator==(String& x, AtomicString& y) compares a String and an AtomicString by
StringImpl::equal(StringImpl*, StringImpl*), the calculation complexity of
which is O(2 * min(the length of x, the length of y)).
In addition, the comparison logic is more complicated than the logic
of calculating the hash. Consequently, AtomicString(String& x) is
faster than operator==(String& x, AtomicString& y).
Keeping that in mind, let's estimate the performance of
Element::getAttribute("class") for <div id="A" lang="B" title="C" class="D" dir="E">.
Here "id", "lang", "title", "class" and "dir" are stored as AtomicStrings
in QualifiedName::localName(). Initially, "class" in Element::getAttribute("class")
is a String.
If we use Element::getAttribute(String& name) (i.e. without the patch),
ElementAttributeData::getAttributeItemIndex() executes four
operator==(String&, AtomicString&) by the time it finds the "class" attribute:
(1) if ("class" == "id") // operator==(String&, AtomicString&)
(2) if ("class" == "lang") // operator==(String&, AtomicString&)
(3) if ("class" == "title") // operator==(String&, AtomicString&)
(4) if ("class" == "class") // operator==(String&, AtomicString&)
On the other hand, if we use Element::getAttribute(AtomicString& name)
(i.e. with the patch), ElementAttributeData::getAttributeItemIndex()
executes one AtomicString(String&) and four operator==(AtomicString&, AtomicString&)
by the time it finds the "class" attribute:
(1) AtomicString("class") // AtomicString(String&)
(2) if ("class" == "id") // operator==(AtomicString&, AtomicString&)
(3) if ("class" == "lang") // operator==(AtomicString&, AtomicString&)
(4) if ("class" == "title") // operator==(AtomicString&, AtomicString&)
(5) if ("class" == "class") // operator==(AtomicString&, AtomicString&)
Considering that the overhead of operator==(AtomicString&, AtomicString&) is close
to 0 since it is just a pointer comparison, the latter approach is faster than
the former approach.
Performance improvement will be large for elements that have multiple attributes,
but it is faster even for elements that have only one attribute.
For exmaple, Dromaeo/dom-attr.html tests getAttribute() for an element that has
only one attribute, the result shows 4.0% improvement.
Another example optimized by this patch is 'div.foo = 123', where foo is not
an attribute of div. In this case, before 123 is set, _javascript_ calls back
Element::getAttribute() to check whether 'foo' is defined on div by
scanning all the attributes of div.
No tests. No change in behavior.
* dom/Element.cpp:
(WebCore::Element::getAttribute):
* dom/Element.h:
(Element):
(WebCore::Element::getAttributeItemIndex):
* dom/ElementAttributeData.cpp:
(WebCore::ElementAttributeData::getAttributeItemIndexSlowCase):
* dom/ElementAttributeData.h:
(ElementAttributeData):
(WebCore::ElementAttributeData::getAttributeItem):
(WebCore::ElementAttributeData::getAttributeItemIndex):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (121438 => 121439)
--- trunk/Source/WebCore/ChangeLog 2012-06-28 16:52:40 UTC (rev 121438)
+++ trunk/Source/WebCore/ChangeLog 2012-06-28 17:07:01 UTC (rev 121439)
@@ -1,3 +1,90 @@
+2012-06-28 Kentaro Hara <[email protected]>
+
+ Optimize Dromaeo/dom-attr.html by speeding up Element::getAttribute()
+ https://bugs.webkit.org/show_bug.cgi?id=90174
+
+ Reviewed by Adam Barth.
+
+ This patch improves performance of Dromaeo/dom-attr.html by 4.0%.
+ The patch improves performance of getAttribute() and _javascript_
+ property setter for DOM objects (e.g. 'div.foo = 123').
+ The performance improvement becomes larger, as the number of
+ attributes defined on the DOM object increases.
+
+ Without the patch in Chromium/Linux (runs/s)
+ 7679.4, 7739.7, 7634.0, 7726.4, 7663.9
+
+ With the patch in Chromium/Linux (runs/s)
+ 7977.7, 8032.2, 8112.8, 7948.1, 7924.5
+
+ This patch just changes a type of 'name' of Element::getAttribute(String& name)
+ from String& to AtomicString&.
+
+ The key observation is that AtomicString(String& x) is faster than
+ operator==(String& x, AtomicString& y). AtomicString(String& x) calculates
+ a hash of a given String and adds it to a hash table. The calculation
+ complexity is O(the length of x). On the other hand,
+ operator==(String& x, AtomicString& y) compares a String and an AtomicString by
+ StringImpl::equal(StringImpl*, StringImpl*), the calculation complexity of
+ which is O(2 * min(the length of x, the length of y)).
+ In addition, the comparison logic is more complicated than the logic
+ of calculating the hash. Consequently, AtomicString(String& x) is
+ faster than operator==(String& x, AtomicString& y).
+
+ Keeping that in mind, let's estimate the performance of
+ Element::getAttribute("class") for <div id="A" lang="B" title="C" class="D" dir="E">.
+ Here "id", "lang", "title", "class" and "dir" are stored as AtomicStrings
+ in QualifiedName::localName(). Initially, "class" in Element::getAttribute("class")
+ is a String.
+
+ If we use Element::getAttribute(String& name) (i.e. without the patch),
+ ElementAttributeData::getAttributeItemIndex() executes four
+ operator==(String&, AtomicString&) by the time it finds the "class" attribute:
+
+ (1) if ("class" == "id") // operator==(String&, AtomicString&)
+ (2) if ("class" == "lang") // operator==(String&, AtomicString&)
+ (3) if ("class" == "title") // operator==(String&, AtomicString&)
+ (4) if ("class" == "class") // operator==(String&, AtomicString&)
+
+ On the other hand, if we use Element::getAttribute(AtomicString& name)
+ (i.e. with the patch), ElementAttributeData::getAttributeItemIndex()
+ executes one AtomicString(String&) and four operator==(AtomicString&, AtomicString&)
+ by the time it finds the "class" attribute:
+
+ (1) AtomicString("class") // AtomicString(String&)
+ (2) if ("class" == "id") // operator==(AtomicString&, AtomicString&)
+ (3) if ("class" == "lang") // operator==(AtomicString&, AtomicString&)
+ (4) if ("class" == "title") // operator==(AtomicString&, AtomicString&)
+ (5) if ("class" == "class") // operator==(AtomicString&, AtomicString&)
+
+ Considering that the overhead of operator==(AtomicString&, AtomicString&) is close
+ to 0 since it is just a pointer comparison, the latter approach is faster than
+ the former approach.
+
+ Performance improvement will be large for elements that have multiple attributes,
+ but it is faster even for elements that have only one attribute.
+ For exmaple, Dromaeo/dom-attr.html tests getAttribute() for an element that has
+ only one attribute, the result shows 4.0% improvement.
+
+ Another example optimized by this patch is 'div.foo = 123', where foo is not
+ an attribute of div. In this case, before 123 is set, _javascript_ calls back
+ Element::getAttribute() to check whether 'foo' is defined on div by
+ scanning all the attributes of div.
+
+ No tests. No change in behavior.
+
+ * dom/Element.cpp:
+ (WebCore::Element::getAttribute):
+ * dom/Element.h:
+ (Element):
+ (WebCore::Element::getAttributeItemIndex):
+ * dom/ElementAttributeData.cpp:
+ (WebCore::ElementAttributeData::getAttributeItemIndexSlowCase):
+ * dom/ElementAttributeData.h:
+ (ElementAttributeData):
+ (WebCore::ElementAttributeData::getAttributeItem):
+ (WebCore::ElementAttributeData::getAttributeItemIndex):
+
2012-06-28 Hans Wennborg <[email protected]>
Speech _javascript_ API: Don't dispatch end event after ActiveDOMObject::stop()
Modified: trunk/Source/WebCore/dom/Element.cpp (121438 => 121439)
--- trunk/Source/WebCore/dom/Element.cpp 2012-06-28 16:52:40 UTC (rev 121438)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-06-28 17:07:01 UTC (rev 121439)
@@ -607,10 +607,10 @@
return e && e->document()->isHTMLDocument() && e->isHTMLElement();
}
-const AtomicString& Element::getAttribute(const String& name) const
+const AtomicString& Element::getAttribute(const AtomicString& name) const
{
bool ignoreCase = shouldIgnoreAttributeCase(this);
-
+
// Update the 'style' attribute if it's invalid and being requested:
if (!isStyleAttributeValid() && equalPossiblyIgnoringCase(name, styleAttr.localName(), ignoreCase))
updateStyleAttribute();
Modified: trunk/Source/WebCore/dom/Element.h (121438 => 121439)
--- trunk/Source/WebCore/dom/Element.h 2012-06-28 16:52:40 UTC (rev 121438)
+++ trunk/Source/WebCore/dom/Element.h 2012-06-28 17:07:01 UTC (rev 121439)
@@ -142,7 +142,7 @@
bool hasAttribute(const String& name) const;
bool hasAttributeNS(const String& namespaceURI, const String& localName) const;
- const AtomicString& getAttribute(const String& name) const;
+ const AtomicString& getAttribute(const AtomicString& name) const;
const AtomicString& getAttributeNS(const String& namespaceURI, const String& localName) const;
void setAttribute(const AtomicString& name, const AtomicString& value, ExceptionCode&);
@@ -165,7 +165,7 @@
Attribute* attributeItem(unsigned index) const;
Attribute* getAttributeItem(const QualifiedName&) const;
size_t getAttributeItemIndex(const QualifiedName& name) const { return attributeData()->getAttributeItemIndex(name); }
- size_t getAttributeItemIndex(const String& name, bool shouldIgnoreAttributeCase) const { return attributeData()->getAttributeItemIndex(name, shouldIgnoreAttributeCase); }
+ size_t getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const { return attributeData()->getAttributeItemIndex(name, shouldIgnoreAttributeCase); }
void scrollIntoView(bool alignToTop = true);
void scrollIntoViewIfNeeded(bool centerIfNeeded = true);
Modified: trunk/Source/WebCore/dom/ElementAttributeData.cpp (121438 => 121439)
--- trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-06-28 16:52:40 UTC (rev 121438)
+++ trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-06-28 17:07:01 UTC (rev 121439)
@@ -230,7 +230,7 @@
ASSERT(!element->hasAttrList());
}
-size_t ElementAttributeData::getAttributeItemIndexSlowCase(const String& name, bool shouldIgnoreAttributeCase) const
+size_t ElementAttributeData::getAttributeItemIndexSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const
{
// Continue to checking case-insensitively and/or full namespaced names if necessary:
for (unsigned i = 0; i < m_attributes.size(); ++i) {
Modified: trunk/Source/WebCore/dom/ElementAttributeData.h (121438 => 121439)
--- trunk/Source/WebCore/dom/ElementAttributeData.h 2012-06-28 16:52:40 UTC (rev 121438)
+++ trunk/Source/WebCore/dom/ElementAttributeData.h 2012-06-28 17:07:01 UTC (rev 121439)
@@ -82,7 +82,7 @@
Attribute* attributeItem(unsigned index) const { return &const_cast<ElementAttributeData*>(this)->m_attributes[index]; }
Attribute* getAttributeItem(const QualifiedName& name) const { return findAttributeInVector(m_attributes, name); }
size_t getAttributeItemIndex(const QualifiedName&) const;
- size_t getAttributeItemIndex(const String& name, bool shouldIgnoreAttributeCase) const;
+ size_t getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
// These functions do no error checking.
void addAttribute(const Attribute&, Element*, EInUpdateStyleAttribute = NotInUpdateStyleAttribute);
@@ -122,8 +122,8 @@
Vector<Attribute> clonedAttributeVector() const { return m_attributes; }
void detachAttrObjectsFromElement(Element*);
- Attribute* getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const;
- size_t getAttributeItemIndexSlowCase(const String& name, bool shouldIgnoreAttributeCase) const;
+ Attribute* getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
+ size_t getAttributeItemIndexSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
void cloneDataFrom(const ElementAttributeData& sourceData, const Element& sourceElement, Element& targetElement);
void clearAttributes(Element*);
void replaceAttribute(size_t index, const Attribute&, Element*);
@@ -145,7 +145,7 @@
return;
}
-inline Attribute* ElementAttributeData::getAttributeItem(const String& name, bool shouldIgnoreAttributeCase) const
+inline Attribute* ElementAttributeData::getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const
{
size_t index = getAttributeItemIndex(name, shouldIgnoreAttributeCase);
if (index != notFound)
@@ -164,7 +164,7 @@
// We use a boolean parameter instead of calling shouldIgnoreAttributeCase so that the caller
// can tune the behavior (hasAttribute is case sensitive whereas getAttribute is not).
-inline size_t ElementAttributeData::getAttributeItemIndex(const String& name, bool shouldIgnoreAttributeCase) const
+inline size_t ElementAttributeData::getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const
{
unsigned len = length();
bool doSlowCheck = shouldIgnoreAttributeCase;
