[webkit-changes] [136526] trunk/Source/WebCore

Tue, 04 Dec 2012 10:26:44 -0800

Title: [136526] trunk/Source/WebCore
Revision
136526
Author
[email protected]
Date
2012-12-04 10:28:32 -0800 (Tue, 04 Dec 2012)

Log Message

[V8] Make v8::Null(isolate) faster by caching the value in V8 bindings
https://bugs.webkit.org/show_bug.cgi?id=103862

Reviewed by Adam Barth.

Performance comparison between V8 and JSC revealed that DOM attributes
that return v8::Null(isolate) are much slower than JSC.
(https://docs.google.com/a/chromium.org/spreadsheet/ccc?key=0AlobCOyvTnPKdDg5S0dMdGRGRTRSaW53V1ppVzh6eXc#gid=2)

We can optimize v8::Null(isolate) by caching the value to V8PerIsolateData.
(Although I don't like the optimization, it looks difficult to optimize
v8::Null(isolate) more.)

[Bindings/undefined-first-child.html]
Before: 969 runs/sec, 970 runs/sec, 962 runs/sec
After:  1086 runs/sec, 1085 runs/sec, 1086 runs/sec

No tests. No change in behavior.

* bindings/scripts/CodeGeneratorV8.pm:
(GenerateHeader):
(GenerateNormalAttrGetter):
(GenerateFunctionCallback):
(NativeToJSValue):
* bindings/scripts/test/V8/V8TestNode.h:
(WebCore::toV8Fast):
* bindings/scripts/test/V8/V8TestObj.cpp:
(WebCore::TestObjV8Internal::contentDocumentAttrGetter):
(WebCore::TestObjV8Internal::getSVGDocumentCallback):
* bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp:
(WebCore::TestSerializedScriptValueInterfaceV8Internal::valueAttrGetter):
(WebCore::TestSerializedScriptValueInterfaceV8Internal::readonlyValueAttrGetter):
(WebCore::TestSerializedScriptValueInterfaceV8Internal::cachedValueAttrGetter):
(WebCore::TestSerializedScriptValueInterfaceV8Internal::cachedReadonlyValueAttrGetter):
* bindings/v8/V8Binding.h:
(WebCore):
(WebCore::v8Null):
(WebCore::v8NullWithCheck):
* bindings/v8/V8PerIsolateData.cpp:
(WebCore::V8PerIsolateData::V8PerIsolateData):
* bindings/v8/V8PerIsolateData.h:
(WebCore::V8PerIsolateData::v8Null):
(V8PerIsolateData):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (136525 => 136526)


--- trunk/Source/WebCore/ChangeLog	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/ChangeLog	2012-12-04 18:28:32 UTC (rev 136526)
@@ -1,3 +1,49 @@
+2012-12-04  Kentaro Hara  <[email protected]>
+
+        [V8] Make v8::Null(isolate) faster by caching the value in V8 bindings
+        https://bugs.webkit.org/show_bug.cgi?id=103862
+
+        Reviewed by Adam Barth.
+
+        Performance comparison between V8 and JSC revealed that DOM attributes
+        that return v8::Null(isolate) are much slower than JSC.
+        (https://docs.google.com/a/chromium.org/spreadsheet/ccc?key=0AlobCOyvTnPKdDg5S0dMdGRGRTRSaW53V1ppVzh6eXc#gid=2)
+
+        We can optimize v8::Null(isolate) by caching the value to V8PerIsolateData.
+        (Although I don't like the optimization, it looks difficult to optimize
+        v8::Null(isolate) more.)
+
+        [Bindings/undefined-first-child.html]
+        Before: 969 runs/sec, 970 runs/sec, 962 runs/sec
+        After:  1086 runs/sec, 1085 runs/sec, 1086 runs/sec
+
+        No tests. No change in behavior.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateHeader):
+        (GenerateNormalAttrGetter):
+        (GenerateFunctionCallback):
+        (NativeToJSValue):
+        * bindings/scripts/test/V8/V8TestNode.h:
+        (WebCore::toV8Fast):
+        * bindings/scripts/test/V8/V8TestObj.cpp:
+        (WebCore::TestObjV8Internal::contentDocumentAttrGetter):
+        (WebCore::TestObjV8Internal::getSVGDocumentCallback):
+        * bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp:
+        (WebCore::TestSerializedScriptValueInterfaceV8Internal::valueAttrGetter):
+        (WebCore::TestSerializedScriptValueInterfaceV8Internal::readonlyValueAttrGetter):
+        (WebCore::TestSerializedScriptValueInterfaceV8Internal::cachedValueAttrGetter):
+        (WebCore::TestSerializedScriptValueInterfaceV8Internal::cachedReadonlyValueAttrGetter):
+        * bindings/v8/V8Binding.h:
+        (WebCore):
+        (WebCore::v8Null):
+        (WebCore::v8NullWithCheck):
+        * bindings/v8/V8PerIsolateData.cpp:
+        (WebCore::V8PerIsolateData::V8PerIsolateData):
+        * bindings/v8/V8PerIsolateData.h:
+        (WebCore::V8PerIsolateData::v8Null):
+        (V8PerIsolateData):
+
 2012-12-04  Alexis Menard  <[email protected]>
 
         Add an helper function in CSSParser to check for '/' character.

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (136525 => 136526)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm	2012-12-04 18:28:32 UTC (rev 136526)
@@ -589,7 +589,7 @@
 inline v8::Handle<v8::Value> toV8Fast(${nativeType}* impl, const v8::AccessorInfo& info, Node* holder)
 {
     if (UNLIKELY(!impl))
-        return v8::Null(info.GetIsolate());
+        return v8Null(info.GetIsolate());
     // What we'd really like to check here is whether we're in the main world or
     // in an isolated world. The fastest way we know how to do that is to check
     // whether the holder's inline wrapper is the same wrapper we see in the
@@ -971,7 +971,7 @@
 
     # Generate security checks if necessary
     if ($attribute->signature->extendedAttributes->{"CheckSecurityForNode"}) {
-        push(@implContentDecls, "    if (!BindingSecurity::shouldAllowAccessToNode(BindingState::instance(), imp->" . $attribute->signature->name . "()))\n        return v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));\n\n");
+        push(@implContentDecls, "    if (!BindingSecurity::shouldAllowAccessToNode(BindingState::instance(), imp->" . $attribute->signature->name . "()))\n        return v8::Handle<v8::Value>(v8Null(info.GetIsolate()));\n\n");
     }
 
     my $useExceptions = 1 if @{$attribute->getterExceptions};
@@ -1125,7 +1125,7 @@
         my $getterFunc = $codeGenerator->WK_lcfirst($attribute->signature->name);
         push(@implContentDecls, <<END);
     SerializedScriptValue* serialized = imp->${getterFunc}();
-    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
     info.Holder()->SetHiddenValue(propertyName, value);
     return value;
 END
@@ -1608,7 +1608,7 @@
 
     if ($function->signature->extendedAttributes->{"CheckSecurityForNode"}) {
         push(@implContentDecls, "    if (!BindingSecurity::shouldAllowAccessToNode(BindingState::instance(), imp->" . $function->signature->name . "(ec)))\n");
-        push(@implContentDecls, "        return v8::Handle<v8::Value>(v8::Null(args.GetIsolate()));\n");
+        push(@implContentDecls, "        return v8::Handle<v8::Value>(v8Null(args.GetIsolate()));\n");
 END
     }
 
@@ -4118,12 +4118,12 @@
 
     if ($type eq "EventListener") {
         AddToImplIncludes("V8AbstractEventListener.h");
-        return "${value} ? v8::Handle<v8::Value>(static_cast<V8AbstractEventListener*>(${value})->getListenerObject(imp->scriptExecutionContext())) : v8::Handle<v8::Value>(" . ($getIsolate ? "v8::Null($getIsolate)" : "v8::Null()") . ")";
+        return "${value} ? v8::Handle<v8::Value>(static_cast<V8AbstractEventListener*>(${value})->getListenerObject(imp->scriptExecutionContext())) : v8::Handle<v8::Value>(" . ($getIsolate ? "v8Null($getIsolate)" : "v8::Null()") . ")";
     }
 
     if ($type eq "SerializedScriptValue") {
         AddToImplIncludes("$type.h");
-        return "$value ? $value->deserialize() : v8::Handle<v8::Value>(" . ($getIsolate ? "v8::Null($getIsolate)" : "v8::Null()") . ")";
+        return "$value ? $value->deserialize() : v8::Handle<v8::Value>(" . ($getIsolate ? "v8Null($getIsolate)" : "v8::Null()") . ")";
     }
 
     AddToImplIncludes("wtf/RefCounted.h");

Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestNode.h (136525 => 136526)


--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestNode.h	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestNode.h	2012-12-04 18:28:32 UTC (rev 136526)
@@ -83,7 +83,7 @@
 inline v8::Handle<v8::Value> toV8Fast(TestNode* impl, const v8::AccessorInfo& info, Node* holder)
 {
     if (UNLIKELY(!impl))
-        return v8::Null(info.GetIsolate());
+        return v8Null(info.GetIsolate());
     // What we'd really like to check here is whether we're in the main world or
     // in an isolated world. The fastest way we know how to do that is to check
     // whether the holder's inline wrapper is the same wrapper we see in the

Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp (136525 => 136526)


--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp	2012-12-04 18:28:32 UTC (rev 136526)
@@ -920,7 +920,7 @@
     INC_STATS("DOM.TestObj.contentDocument._get");
     TestObj* imp = V8TestObj::toNative(info.Holder());
     if (!BindingSecurity::shouldAllowAccessToNode(BindingState::instance(), imp->contentDocument()))
-        return v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+        return v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
 
     return toV8(imp->contentDocument(), info.Holder(), info.GetIsolate());
 }
@@ -1847,7 +1847,7 @@
     ExceptionCode ec = 0;
     {
     if (!BindingSecurity::shouldAllowAccessToNode(BindingState::instance(), imp->getSVGDocument(ec)))
-        return v8::Handle<v8::Value>(v8::Null(args.GetIsolate()));
+        return v8::Handle<v8::Value>(v8Null(args.GetIsolate()));
     RefPtr<SVGDocument> result = imp->getSVGDocument(ec);
     if (UNLIKELY(ec))
         goto fail;

Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp (136525 => 136526)


--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp	2012-12-04 18:28:32 UTC (rev 136526)
@@ -48,7 +48,7 @@
 {
     INC_STATS("DOM.TestSerializedScriptValueInterface.value._get");
     TestSerializedScriptValueInterface* imp = V8TestSerializedScriptValueInterface::toNative(info.Holder());
-    return imp->value() ? imp->value()->deserialize() : v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+    return imp->value() ? imp->value()->deserialize() : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
 }
 
 static void valueAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -64,7 +64,7 @@
 {
     INC_STATS("DOM.TestSerializedScriptValueInterface.readonlyValue._get");
     TestSerializedScriptValueInterface* imp = V8TestSerializedScriptValueInterface::toNative(info.Holder());
-    return imp->readonlyValue() ? imp->readonlyValue()->deserialize() : v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+    return imp->readonlyValue() ? imp->readonlyValue()->deserialize() : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
 }
 
 static v8::Handle<v8::Value> cachedValueAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
@@ -76,7 +76,7 @@
         return value;
     TestSerializedScriptValueInterface* imp = V8TestSerializedScriptValueInterface::toNative(info.Holder());
     SerializedScriptValue* serialized = imp->cachedValue();
-    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
     info.Holder()->SetHiddenValue(propertyName, value);
     return value;
 }
@@ -114,7 +114,7 @@
         return value;
     TestSerializedScriptValueInterface* imp = V8TestSerializedScriptValueInterface::toNative(info.Holder());
     SerializedScriptValue* serialized = imp->cachedReadonlyValue();
-    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8::Null(info.GetIsolate()));
+    value = serialized ? serialized->deserialize() : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
     info.Holder()->SetHiddenValue(propertyName, value);
     return value;
 }

Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (136525 => 136526)


--- trunk/Source/WebCore/bindings/v8/V8Binding.h	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h	2012-12-04 18:28:32 UTC (rev 136526)
@@ -72,13 +72,21 @@
     // A helper for throwing _javascript_ TypeError for not enough arguments.
     v8::Handle<v8::Value> throwNotEnoughArgumentsError(v8::Isolate*);
 
+    // A fast accessor for v8::Null(isolate). isolate must not be 0.
+    // If isolate can be 0, use v8NullWithCheck().
+    inline v8::Handle<v8::Value> v8Null(v8::Isolate* isolate)
+    {
+        ASSERT(isolate);
+        return V8PerIsolateData::from(isolate)->v8Null();
+    }
+
     // Since v8::Null(isolate) crashes if we pass a null isolate,
     // we need to use v8NullWithCheck(isolate) if an isolate can be null.
     //
     // FIXME: Remove all null isolates from V8 bindings, and remove v8NullWithCheck(isolate).
     inline v8::Handle<v8::Value> v8NullWithCheck(v8::Isolate* isolate)
     {
-        return isolate ? v8::Null(isolate) : v8::Null();
+        return isolate ? v8Null(isolate) : v8::Handle<v8::Value>(v8::Null());
     }
 
     // Convert v8 types to a WTF::String. If the V8 string is not already

Modified: trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp (136525 => 136526)


--- trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp	2012-12-04 18:28:32 UTC (rev 136526)
@@ -57,6 +57,7 @@
     , m_gcEventData(adoptPtr(new GCEventData()))
     , m_shouldCollectGarbageSoon(false)
 {
+    m_v8Null.set(v8::Null(isolate));
 }
 
 V8PerIsolateData::~V8PerIsolateData()

Modified: trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h (136525 => 136526)


--- trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h	2012-12-04 18:24:45 UTC (rev 136525)
+++ trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h	2012-12-04 18:28:32 UTC (rev 136526)
@@ -78,6 +78,8 @@
     StringCache* stringCache() { return m_stringCache.get(); }
     IntegerCache* integerCache() { return m_integerCache.get(); }
 
+    v8::Handle<v8::Value> v8Null() { return m_v8Null.get(); }
+
     v8::Persistent<v8::Value> ensureLiveRoot();
 
 #if ENABLE(INSPECTOR)
@@ -138,6 +140,7 @@
     v8::Persistent<v8::FunctionTemplate> m_lazyEventListenerToStringTemplate;
     OwnPtr<StringCache> m_stringCache;
     OwnPtr<IntegerCache> m_integerCache;
+    ScopedPersistent<v8::Value> m_v8Null;
 
     Vector<DOMDataStore*> m_domDataList;
     DOMDataStore* m_domDataStore;

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to