[V8][Performance] Optimize Element.firstElementChild, Element.lastElementChild,
authorharaken@chromium.org <haraken@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2012 22:36:54 +0000 (22:36 +0000)
committerharaken@chromium.org <haraken@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Mar 2012 22:36:54 +0000 (22:36 +0000)
Element.previousElementSibling, Element.nextElementSibling, Node.parentElement
https://bugs.webkit.org/show_bug.cgi?id=80506

Reviewed by Adam Barth.

This patch improves the performance of Element.firstElementChild by 5.8 times,
Element.lastElementChild by 6.2 times, Element.previousElementSibling by 7.1 times,
Element.nextElementSibling by 7.1 times, and Node.parentElement by 6.7 times.

Previously, while toV8(Node*) caches a wrapper object on a node object
(i.e. node->wrapper(), node->setWrapper()), toV8(Element*) does not
cache a wrapper object.

This patch removes toV8(Element*), so that DOM attribute getters that return
Element* use toV8(Node*). This change makes these DOM attribute getters
cache the wrapper object on a node object. This optimization is already
implemented in JavaScriptCore.

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

The test results in my local Mac environment are as follows:

AppleWebKit/JavaScriptCore:
div.firstElementChild : 1162ms
div.lastElementChild : 1016ms
div.previousElementSibling : 918ms
div.nextElementSibling : 900ms
div.parentElement : 901ms

Chromium/V8 (without this patch):
div.firstElementChild : 9515ms
div.lastElementChild : 9449ms
div.previousElementSibling : 9254ms
div.nextElementSibling : 9315ms
div.parentElement : 9380ms

Chromium/V8 (with this patch):
div.firstElementChild : 1628ms
div.lastElementChild : 1527ms
div.previousElementSibling : 1310ms
div.nextElementSibling : 1310ms
div.parentElement : 1410ms

No tests. No change in behavior.

* dom/Element.idl: Removed toV8(Element*)
* bindings/v8/custom/V8NodeCustom.cpp: Ditto.
(WebCore::toV8Slow):
* bindings/scripts/CodeGeneratorV8.pm: Ditto.
(GenerateHeader):

* bindings/v8/custom/V8ElementCustom.cpp: Removed.
* Target.pri: Removed V8ElementCustom.cpp.
* UseV8.cmake: Ditto.
* WebCore.gypi: Ditto.

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

Source/WebCore/ChangeLog
Source/WebCore/Target.pri
Source/WebCore/UseV8.cmake
Source/WebCore/WebCore.gypi
Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp [deleted file]
Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp
Source/WebCore/dom/Element.idl

index 1bc18d9..03b75c6 100644 (file)
@@ -1,3 +1,62 @@
+2012-03-07  Kentaro Hara  <haraken@chromium.org>
+
+        [V8][Performance] Optimize Element.firstElementChild, Element.lastElementChild,
+        Element.previousElementSibling, Element.nextElementSibling, Node.parentElement
+        https://bugs.webkit.org/show_bug.cgi?id=80506
+
+        Reviewed by Adam Barth.
+
+        This patch improves the performance of Element.firstElementChild by 5.8 times,
+        Element.lastElementChild by 6.2 times, Element.previousElementSibling by 7.1 times,
+        Element.nextElementSibling by 7.1 times, and Node.parentElement by 6.7 times.
+
+        Previously, while toV8(Node*) caches a wrapper object on a node object
+        (i.e. node->wrapper(), node->setWrapper()), toV8(Element*) does not
+        cache a wrapper object.
+
+        This patch removes toV8(Element*), so that DOM attribute getters that return
+        Element* use toV8(Node*). This change makes these DOM attribute getters
+        cache the wrapper object on a node object. This optimization is already
+        implemented in JavaScriptCore.
+
+        Performance tests: https://bugs.webkit.org/attachment.cgi?id=130594
+
+        The test results in my local Mac environment are as follows:
+
+        AppleWebKit/JavaScriptCore:
+        div.firstElementChild : 1162ms
+        div.lastElementChild : 1016ms
+        div.previousElementSibling : 918ms
+        div.nextElementSibling : 900ms
+        div.parentElement : 901ms
+
+        Chromium/V8 (without this patch):
+        div.firstElementChild : 9515ms
+        div.lastElementChild : 9449ms
+        div.previousElementSibling : 9254ms
+        div.nextElementSibling : 9315ms
+        div.parentElement : 9380ms
+
+        Chromium/V8 (with this patch):
+        div.firstElementChild : 1628ms
+        div.lastElementChild : 1527ms
+        div.previousElementSibling : 1310ms
+        div.nextElementSibling : 1310ms
+        div.parentElement : 1410ms
+
+        No tests. No change in behavior.
+
+        * dom/Element.idl: Removed toV8(Element*)
+        * bindings/v8/custom/V8NodeCustom.cpp: Ditto.
+        (WebCore::toV8Slow):
+        * bindings/scripts/CodeGeneratorV8.pm: Ditto.
+        (GenerateHeader):
+
+        * bindings/v8/custom/V8ElementCustom.cpp: Removed.
+        * Target.pri: Removed V8ElementCustom.cpp.
+        * UseV8.cmake: Ditto.
+        * WebCore.gypi: Ditto.
+
 2012-03-07  Joshua Bell  <jsbell@chromium.org>
 
         [Chromium] IndexedDB: V8LocalContext creation in IDBKey extraction/injection is slow
index 06e30d2..7fa434b 100644 (file)
@@ -181,7 +181,6 @@ v8 {
         bindings/v8/custom/V8DedicatedWorkerContextCustom.cpp \
         bindings/v8/custom/V8DocumentCustom.cpp \
         bindings/v8/custom/V8DocumentLocationCustom.cpp \
-        bindings/v8/custom/V8ElementCustom.cpp \
         bindings/v8/custom/V8EventCustom.cpp \
         bindings/v8/custom/V8FileReaderCustom.cpp \
         bindings/v8/custom/V8HTMLAllCollectionCustom.cpp
index 0c79b8a..b8f5fd2 100755 (executable)
@@ -95,7 +95,6 @@ LIST(APPEND WebCore_SOURCES
     bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp
     bindings/v8/custom/V8DocumentCustom.cpp
     bindings/v8/custom/V8DocumentLocationCustom.cpp
-    bindings/v8/custom/V8ElementCustom.cpp
     bindings/v8/custom/V8EntrySyncCustom.cpp
     bindings/v8/custom/V8EventConstructors.cpp
     bindings/v8/custom/V8EventCustom.cpp
index 6cc8923..e9fce06 100644 (file)
             'bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp',
             'bindings/v8/custom/V8DocumentCustom.cpp',
             'bindings/v8/custom/V8DocumentLocationCustom.cpp',
-            'bindings/v8/custom/V8ElementCustom.cpp',
             'bindings/v8/custom/V8EntryCustom.cpp',
             'bindings/v8/custom/V8EntrySyncCustom.cpp',
             'bindings/v8/custom/V8EventCustom.cpp',
index feb9b29..b1de4bd 100644 (file)
@@ -455,7 +455,9 @@ END
 }
 END
 
-    if (!($dataNode->extendedAttributes->{"CustomToJSObject"} or $dataNode->extendedAttributes->{"V8CustomToJSObject"})) {
+    if ($interfaceName eq 'Element') {
+        # Do not generate toV8() for performance optimization.
+    } elsif (!($dataNode->extendedAttributes->{"CustomToJSObject"} or $dataNode->extendedAttributes->{"V8CustomToJSObject"})) {
         push(@headerContent, <<END);
 
 inline v8::Handle<v8::Value> toV8(${nativeType}* impl${forceNewObjectParameter})
diff --git a/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp b/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp
deleted file mode 100644 (file)
index e9b9336..0000000
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (C) 2007-2009 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *     * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "config.h"
-#include "V8Element.h"
-
-#include "Attr.h"
-#include "Document.h"
-#include "Element.h"
-#include "ExceptionCode.h"
-#include "HTMLFrameElementBase.h"
-#include "HTMLNames.h"
-#include "Node.h"
-
-#include "V8Attr.h"
-#include "V8Binding.h"
-#include "V8BindingState.h"
-#include "V8HTMLElement.h"
-#include "V8Proxy.h"
-
-#if ENABLE(SVG)
-#include "V8SVGElement.h"
-#endif
-
-#include <wtf/RefPtr.h>
-
-namespace WebCore {
-
-v8::Handle<v8::Value> toV8(Element* impl, bool forceNewObject)
-{
-    if (!impl)
-        return v8::Null();
-    if (impl->isHTMLElement())
-        return toV8(toHTMLElement(impl), forceNewObject);
-#if ENABLE(SVG)
-    if (impl->isSVGElement())
-        return toV8(static_cast<SVGElement*>(impl), forceNewObject);
-#endif
-    return V8Element::wrap(impl, forceNewObject);
-}
-} // namespace WebCore
index 4490315..2427113 100644 (file)
 #include "V8Entity.h"
 #include "V8EntityReference.h"
 #include "V8EventListener.h"
+#include "V8HTMLElement.h"
 #include "V8Node.h"
 #include "V8Notation.h"
 #include "V8ProcessingInstruction.h"
 #include "V8Proxy.h"
 #include "V8Text.h"
 
+#if ENABLE(SVG)
+#include "V8SVGElement.h"
+#endif
+
 #include <wtf/RefPtr.h>
 
 namespace WebCore {
@@ -143,7 +148,13 @@ v8::Handle<v8::Value> toV8Slow(Node* impl, bool forceNewObject)
     }
     switch (impl->nodeType()) {
     case Node::ELEMENT_NODE:
-        return toV8(static_cast<Element*>(impl), forceNewObject);
+        if (impl->isHTMLElement())
+            return toV8(toHTMLElement(impl), forceNewObject);
+#if ENABLE(SVG)
+        if (impl->isSVGElement())
+            return toV8(static_cast<SVGElement*>(impl), forceNewObject);
+#endif
+        return V8Element::wrap(static_cast<Element*>(impl), forceNewObject);
     case Node::ATTRIBUTE_NODE:
         return toV8(static_cast<Attr*>(impl), forceNewObject);
     case Node::TEXT_NODE:
index c66b145..8725bcb 100644 (file)
@@ -22,8 +22,7 @@ module core {
 
     interface [
         JSGenerateToNativeObject,
-        JSInlineGetOwnPropertySlot,
-        V8CustomToJSObject
+        JSInlineGetOwnPropertySlot
     ] Element : Node {
 
         // DOM Level 1 Core