Title: [109969] trunk/Source/WebCore
Revision
109969
Author
[email protected]
Date
2012-03-06 15:54:46 -0800 (Tue, 06 Mar 2012)

Log Message

[V8][Performance] Optimize V8 bindings for HTMLElement.classList,
Element.dataset and Node.attributes
https://bugs.webkit.org/show_bug.cgi?id=80376

Reviewed by Adam Barth.

This patch improves the performance of HTMLElement.classList, Element.dataset
and Node.attributes by 6.4 times, 7.1 times and 10.9 times, respectively.

Previously, a 'hiddenReferenceName' string was allocated on v8::Handle and
created every time the DOM attribute is accessed, in spite of the fact that
the 'hiddenReferenceName' string is static.

This patch moves the 'hiddenReferenceName' string to v8::Persistent and makes it static.
Also, this patch removes 'if (!elementValue.IsEmpty() && elementValue->IsObject())',
since if 'element' exists, it is guaranteed that 'elementValue' is not empty
and is an Object.

Performance tests: https://bugs.webkit.org/attachment.cgi?id=130283

AppleWebKit/_javascript_Core:
div.classList : 382ms
div.classList.foo = 123 : 335ms
div.dataset : 403ms
div.dataset.foo = 123 : 5250ms
div.attributes : 183ms

Chromium/V8 (without this patch):
div.classList : 9140ms
div.classList.foo = 123 : 9086ms
div.dataset : 9930ms
div.dataset.foo = 123 : 49698ms
div.attributes : 13489ms

Chromium/V8 (with this patch):
div.classList : 1435ms
div.classList.foo = 123 : 1470ms
div.dataset : 1400ms
div.dataset.foo = 123 : 30396ms
div.attributes : 1242ms

No tests. No change in behavior.

* bindings/v8/custom/V8DOMStringMapCustom.cpp: Modified as described above.
(WebCore::toV8):
* bindings/v8/custom/V8DOMTokenListCustom.cpp: Ditto.
(WebCore::toV8):
* bindings/v8/custom/V8NamedNodeMapCustom.cpp: Ditto.
(WebCore::toV8):

* bindings/v8/V8HiddenPropertyName.cpp: Defined a hidden property name string statically
to optimize the macro.
(WebCore):
(WebCore::V8HiddenPropertyName::hiddenReferenceName):
* bindings/v8/V8HiddenPropertyName.h: Modified to switch two prefixes "WebCore::HiddenProperty::"
and "WebCore::HiddenReference::", depending on whether a given name represents a hidden property
or a hidden reference.
(WebCore):
(V8HiddenPropertyName):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (109968 => 109969)


--- trunk/Source/WebCore/ChangeLog	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/ChangeLog	2012-03-06 23:54:46 UTC (rev 109969)
@@ -1,3 +1,65 @@
+2012-03-06  Kentaro Hara  <[email protected]>
+
+        [V8][Performance] Optimize V8 bindings for HTMLElement.classList,
+        Element.dataset and Node.attributes
+        https://bugs.webkit.org/show_bug.cgi?id=80376
+
+        Reviewed by Adam Barth.
+
+        This patch improves the performance of HTMLElement.classList, Element.dataset
+        and Node.attributes by 6.4 times, 7.1 times and 10.9 times, respectively.
+
+        Previously, a 'hiddenReferenceName' string was allocated on v8::Handle and
+        created every time the DOM attribute is accessed, in spite of the fact that
+        the 'hiddenReferenceName' string is static.
+
+        This patch moves the 'hiddenReferenceName' string to v8::Persistent and makes it static.
+        Also, this patch removes 'if (!elementValue.IsEmpty() && elementValue->IsObject())',
+        since if 'element' exists, it is guaranteed that 'elementValue' is not empty
+        and is an Object.
+
+        Performance tests: https://bugs.webkit.org/attachment.cgi?id=130283
+
+        AppleWebKit/_javascript_Core:
+        div.classList : 382ms
+        div.classList.foo = 123 : 335ms
+        div.dataset : 403ms
+        div.dataset.foo = 123 : 5250ms
+        div.attributes : 183ms
+
+        Chromium/V8 (without this patch):
+        div.classList : 9140ms
+        div.classList.foo = 123 : 9086ms
+        div.dataset : 9930ms
+        div.dataset.foo = 123 : 49698ms
+        div.attributes : 13489ms
+
+        Chromium/V8 (with this patch):
+        div.classList : 1435ms
+        div.classList.foo = 123 : 1470ms
+        div.dataset : 1400ms
+        div.dataset.foo = 123 : 30396ms
+        div.attributes : 1242ms
+
+        No tests. No change in behavior.
+
+        * bindings/v8/custom/V8DOMStringMapCustom.cpp: Modified as described above.
+        (WebCore::toV8):
+        * bindings/v8/custom/V8DOMTokenListCustom.cpp: Ditto.
+        (WebCore::toV8):
+        * bindings/v8/custom/V8NamedNodeMapCustom.cpp: Ditto.
+        (WebCore::toV8):
+
+        * bindings/v8/V8HiddenPropertyName.cpp: Defined a hidden property name string statically
+        to optimize the macro.
+        (WebCore):
+        (WebCore::V8HiddenPropertyName::hiddenReferenceName):
+        * bindings/v8/V8HiddenPropertyName.h: Modified to switch two prefixes "WebCore::HiddenProperty::"
+        and "WebCore::HiddenReference::", depending on whether a given name represents a hidden property
+        or a hidden reference.
+        (WebCore):
+        (V8HiddenPropertyName):
+
 2012-03-06  Alexis Menard  <[email protected]>
 
         getComputedStyle returns incorrect values for the width and height of pseudo-elements

Modified: trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp (109968 => 109969)


--- trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp	2012-03-06 23:54:46 UTC (rev 109969)
@@ -33,6 +33,7 @@
 
 #include "V8Binding.h"
 #include <string.h>
+#include <wtf/StdLibExtras.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -40,24 +41,19 @@
 #define V8_AS_STRING(x) V8_AS_STRING_IMPL(x)
 #define V8_AS_STRING_IMPL(x) #x
 
-#define V8_DEFINE_PROPERTY(name) \
+#define V8_DEFINE_HIDDEN_PROPERTY(name, prefix) \
 v8::Handle<v8::String> V8HiddenPropertyName::name() \
 { \
-    V8HiddenPropertyName* hiddenPropertyName = V8BindingPerIsolateData::current()->hiddenPropertyName(); \
-    if (hiddenPropertyName->m_##name.IsEmpty()) { \
-        hiddenPropertyName->m_##name = createString("WebCore::HiddenProperty::" V8_AS_STRING(name)); \
-    } \
-    return hiddenPropertyName->m_##name; \
+    DEFINE_STATIC_LOCAL(v8::Persistent<v8::String>, hiddenPropertyName, (createString(prefix V8_AS_STRING(name)))); \
+    return hiddenPropertyName; \
 }
 
-V8_HIDDEN_PROPERTIES(V8_DEFINE_PROPERTY);
+V8_HIDDEN_PROPERTIES(V8_DEFINE_HIDDEN_PROPERTY);
 
-static const char hiddenReferenceNamePrefix[] = "WebCore::HiddenReference::";
-
 v8::Handle<v8::String> V8HiddenPropertyName::hiddenReferenceName(const char* name)
 {
     Vector<char, 64> prefixedName;
-    prefixedName.append(hiddenReferenceNamePrefix, sizeof(hiddenReferenceNamePrefix) - 1);
+    prefixedName.append(V8_HIDDEN_REFERENCE_PREFIX, sizeof(V8_HIDDEN_REFERENCE_PREFIX) - 1);
     ASSERT(name && strlen(name));
     prefixedName.append(name, strlen(name));
     return v8::String::NewSymbol(prefixedName.data(), static_cast<int>(prefixedName.size()));

Modified: trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h (109968 => 109969)


--- trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h	2012-03-06 23:54:46 UTC (rev 109969)
@@ -35,22 +35,27 @@
 
 namespace WebCore {
 
+#define V8_HIDDEN_PROPERTY_PREFIX "WebCore::HiddenProperty::"
+#define V8_HIDDEN_REFERENCE_PREFIX "WebCore::HiddenReference::"
+
 #define V8_HIDDEN_PROPERTIES(V) \
-    V(objectPrototype) \
-    V(listener) \
-    V(attributeListener) \
-    V(scriptState) \
-    V(devtoolsInjectedScript) \
-    V(sleepFunction) \
-    V(toStringString) \
-    V(event) \
-    V(state)
+    V(objectPrototype, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(listener, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(attributeListener, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(scriptState, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(devtoolsInjectedScript, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(sleepFunction, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(toStringString, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(event, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(state, V8_HIDDEN_PROPERTY_PREFIX) \
+    V(domStringMap, V8_HIDDEN_REFERENCE_PREFIX) \
+    V(domTokenList, V8_HIDDEN_REFERENCE_PREFIX) \
+    V(ownerNode, V8_HIDDEN_REFERENCE_PREFIX)
 
-
     class V8HiddenPropertyName {
     public:
         V8HiddenPropertyName() { }
-#define V8_DECLARE_PROPERTY(name) static v8::Handle<v8::String> name();
+#define V8_DECLARE_PROPERTY(name, prefix) static v8::Handle<v8::String> name();
         V8_HIDDEN_PROPERTIES(V8_DECLARE_PROPERTY);
 #undef V8_DECLARE_PROPERTY
 
@@ -58,9 +63,6 @@
 
     private:
         static v8::Persistent<v8::String> createString(const char* key);
-#define V8_DECLARE_FIELD(name) v8::Persistent<v8::String> m_##name;
-        V8_HIDDEN_PROPERTIES(V8_DECLARE_FIELD);
-#undef V8_DECLARE_FIELD
     };
 
 }

Modified: trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp (109968 => 109969)


--- trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp	2012-03-06 23:54:46 UTC (rev 109969)
@@ -91,11 +91,8 @@
     v8::Handle<v8::Object> wrapper = V8DOMStringMap::wrap(impl);
     // Add a hidden reference from the element to the DOMStringMap.
     Element* element = impl->element();
-    if (!wrapper.IsEmpty() && element) {
-        v8::Handle<v8::Value> elementValue = toV8(element);
-        if (!elementValue.IsEmpty() && elementValue->IsObject())
-            V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domStringMap", wrapper);
-    }
+    if (!wrapper.IsEmpty() && element)
+        toV8(element).As<v8::Object>()->SetHiddenValue(V8HiddenPropertyName::domStringMap(), wrapper);
     return wrapper;
 }
 

Modified: trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp (109968 => 109969)


--- trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp	2012-03-06 23:54:46 UTC (rev 109969)
@@ -45,11 +45,8 @@
     v8::Handle<v8::Object> wrapper = V8DOMTokenList::wrap(impl);
     // Add a hidden reference from the element to the DOMTokenList.
     Element* element = impl->element();
-    if (!wrapper.IsEmpty() && element) {
-        v8::Handle<v8::Value> elementValue = toV8(element);
-        if (!elementValue.IsEmpty() && elementValue->IsObject())
-            V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domTokenList", wrapper);
-    }
+    if (!wrapper.IsEmpty() && element)
+        toV8(element).As<v8::Object>()->SetHiddenValue(V8HiddenPropertyName::domTokenList(), wrapper);
     return wrapper;
 }
 

Modified: trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp (109968 => 109969)


--- trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp	2012-03-06 23:52:29 UTC (rev 109968)
+++ trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp	2012-03-06 23:54:46 UTC (rev 109969)
@@ -79,7 +79,7 @@
     // Add a hidden reference from named node map to its owner node.
     Element* element = impl->element();
     if (!wrapper.IsEmpty() && element)
-        V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(element));
+        toV8(element).As<v8::Object>()->SetHiddenValue(V8HiddenPropertyName::ownerNode(), wrapper);
     return wrapper;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to