Title: [110106] trunk/Source/WebCore
Revision
110106
Author
hara...@chromium.org
Date
2012-03-07 14:36:54 -0800 (Wed, 07 Mar 2012)

Log Message

[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 _javascript_Core.

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

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

AppleWebKit/_javascript_Core:
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.

Modified Paths

Removed Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (110105 => 110106)


--- trunk/Source/WebCore/ChangeLog	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/ChangeLog	2012-03-07 22:36:54 UTC (rev 110106)
@@ -1,3 +1,62 @@
+2012-03-07  Kentaro Hara  <hara...@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 _javascript_Core.
+
+        Performance tests: https://bugs.webkit.org/attachment.cgi?id=130594
+
+        The test results in my local Mac environment are as follows:
+
+        AppleWebKit/_javascript_Core:
+        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  <jsb...@chromium.org>
 
         [Chromium] IndexedDB: V8LocalContext creation in IDBKey extraction/injection is slow

Modified: trunk/Source/WebCore/Target.pri (110105 => 110106)


--- trunk/Source/WebCore/Target.pri	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/Target.pri	2012-03-07 22:36:54 UTC (rev 110106)
@@ -181,7 +181,6 @@
         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

Modified: trunk/Source/WebCore/UseV8.cmake (110105 => 110106)


--- trunk/Source/WebCore/UseV8.cmake	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/UseV8.cmake	2012-03-07 22:36:54 UTC (rev 110106)
@@ -95,7 +95,6 @@
     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

Modified: trunk/Source/WebCore/WebCore.gypi (110105 => 110106)


--- trunk/Source/WebCore/WebCore.gypi	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/WebCore.gypi	2012-03-07 22:36:54 UTC (rev 110106)
@@ -2014,7 +2014,6 @@
             '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',

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (110105 => 110106)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm	2012-03-07 22:36:54 UTC (rev 110106)
@@ -455,7 +455,9 @@
 }
 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})

Deleted: trunk/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp (110105 => 110106)


--- trunk/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp	2012-03-07 22:36:54 UTC (rev 110106)
@@ -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

Modified: trunk/Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp (110105 => 110106)


--- trunk/Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp	2012-03-07 22:36:54 UTC (rev 110106)
@@ -48,12 +48,17 @@
 #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 @@
     }
     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:

Modified: trunk/Source/WebCore/dom/Element.idl (110105 => 110106)


--- trunk/Source/WebCore/dom/Element.idl	2012-03-07 22:36:43 UTC (rev 110105)
+++ trunk/Source/WebCore/dom/Element.idl	2012-03-07 22:36:54 UTC (rev 110106)
@@ -22,8 +22,7 @@
 
     interface [
         JSGenerateToNativeObject,
-        JSInlineGetOwnPropertySlot,
-        V8CustomToJSObject
+        JSInlineGetOwnPropertySlot
     ] Element : Node {
 
         // DOM Level 1 Core
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to