- Revision
- 114912
- Author
- hara...@chromium.org
- Date
- 2012-04-23 09:55:13 -0700 (Mon, 23 Apr 2012)
Log Message
[Performance][V8] Skip Isolate look-up to find StringCache
https://bugs.webkit.org/show_bug.cgi?id=84103
Reviewed by Nate Chapin.
This patch improves the performance of a lot of DOM attribute
getters that return a string.
- Improves the performance of Dromaeo/dom-attr.html(element.property)
by 27.7%.
- Improves the performance of Dromaeo/dom-attr.html(getAttribute)
by 10.6%.
- Improves the performance of div.id, div.className,
div.nodeName, text.nodeValue, text.textContent by 12% -- 21%.
The followings are the test results in my Linux desktop.
Performance test: Dromaeo/dom-attr.html
Total: 674.64runs/s -> 707.03runs/s (+ 4.8%)
getAttribute: 1537.60runs/s -> 1700.20runs/s (+10.6%)
element.property: 1389.00runs/s -> 1774.20runs/s (+27.7%)
setAttribute: 538.88runs/s -> 548.87runs/s (+ 1.9%)
element.property = value: 644.07runs/s -> 656.67runs/s (+ 2.0%)
element.expando = value: 219.76runs/s -> 207.14runs/s (- 6.8%)
element.expando: 578.77runs/s -> 554.67runs/s (- 4.2%)
Performance test: https://bugs.webkit.org/attachment.cgi?id=137440
div.id: 30.70ns -> 26.70ns (+15%)
div.className: 31.10ns -> 26.40ns (+18%)
div.nodeName: 37.70ns -> 33.00ns (+14%)
text.nodeValue: 31.40ns -> 25.90ns (+21%)
text.textContent: 51.50ns -> 45.90ns (+12%)
Previously V8 bindings need to look up an Isolate to find
an Isolate-local StringCache. This patch skips the look-up
by getting the Isolate from AccessorInfo.GetIsolate()
or Arguments.GetIsolate().
No tests. No change in behavior.
* bindings/scripts/CodeGeneratorV8.pm:
(GenerateNormalAttrGetter):
(GenerateFunctionCallString):
(NativeToJSValue):
* bindings/v8/V8Binding.cpp:
(WebCore::getElementStringAttr):
* bindings/v8/V8Binding.h:
(WebCore::v8ExternalString): Make 'isolate' an optional argument.
Ideally we want to make 'isolate' a non-optional argument,
but it is difficult to rewrite all v8ExternalString() callers
at a breath. We can rewrite them incrementally.
(WebCore::v8String): Ditto.
(WebCore::v8StringOrNull): Ditto.
(WebCore::v8StringOrUndefined): Ditto.
(WebCore::v8StringOrFalse): Ditto.
* bindings/scripts/test/V8/V8TestEventConstructor.cpp: Updated run-bindings-tests results.
(WebCore::TestEventConstructorV8Internal::attr1AttrGetter):
(WebCore::TestEventConstructorV8Internal::attr2AttrGetter):
* bindings/scripts/test/V8/V8TestInterface.cpp:
(WebCore::TestInterfaceV8Internal::supplementalStr1AttrGetter):
(WebCore::TestInterfaceV8Internal::supplementalStr2AttrGetter):
* bindings/scripts/test/V8/V8TestObj.cpp:
(WebCore::TestObjV8Internal::readOnlyStringAttrAttrGetter):
(WebCore::TestObjV8Internal::stringAttrAttrGetter):
(WebCore::TestObjV8Internal::reflectedStringAttrAttrGetter):
(WebCore::TestObjV8Internal::reflectedURLAttrAttrGetter):
(WebCore::TestObjV8Internal::reflectedCustomURLAttrAttrGetter):
(WebCore::TestObjV8Internal::stringAttrWithGetterExceptionAttrGetter):
(WebCore::TestObjV8Internal::stringAttrWithSetterExceptionAttrGetter):
(WebCore::TestObjV8Internal::hashAttrGetter):
(WebCore::TestObjV8Internal::conditionalMethod1Callback):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (114911 => 114912)
--- trunk/Source/WebCore/ChangeLog 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/ChangeLog 2012-04-23 16:55:13 UTC (rev 114912)
@@ -1,5 +1,80 @@
2012-04-17 Kentaro Hara <hara...@chromium.org>
+ [Performance][V8] Skip Isolate look-up to find StringCache
+ https://bugs.webkit.org/show_bug.cgi?id=84103
+
+ Reviewed by Nate Chapin.
+
+ This patch improves the performance of a lot of DOM attribute
+ getters that return a string.
+
+ - Improves the performance of Dromaeo/dom-attr.html(element.property)
+ by 27.7%.
+ - Improves the performance of Dromaeo/dom-attr.html(getAttribute)
+ by 10.6%.
+ - Improves the performance of div.id, div.className,
+ div.nodeName, text.nodeValue, text.textContent by 12% -- 21%.
+
+ The followings are the test results in my Linux desktop.
+
+ Performance test: Dromaeo/dom-attr.html
+ Total: 674.64runs/s -> 707.03runs/s (+ 4.8%)
+ getAttribute: 1537.60runs/s -> 1700.20runs/s (+10.6%)
+ element.property: 1389.00runs/s -> 1774.20runs/s (+27.7%)
+ setAttribute: 538.88runs/s -> 548.87runs/s (+ 1.9%)
+ element.property = value: 644.07runs/s -> 656.67runs/s (+ 2.0%)
+ element.expando = value: 219.76runs/s -> 207.14runs/s (- 6.8%)
+ element.expando: 578.77runs/s -> 554.67runs/s (- 4.2%)
+
+ Performance test: https://bugs.webkit.org/attachment.cgi?id=137440
+ div.id: 30.70ns -> 26.70ns (+15%)
+ div.className: 31.10ns -> 26.40ns (+18%)
+ div.nodeName: 37.70ns -> 33.00ns (+14%)
+ text.nodeValue: 31.40ns -> 25.90ns (+21%)
+ text.textContent: 51.50ns -> 45.90ns (+12%)
+
+ Previously V8 bindings need to look up an Isolate to find
+ an Isolate-local StringCache. This patch skips the look-up
+ by getting the Isolate from AccessorInfo.GetIsolate()
+ or Arguments.GetIsolate().
+
+ No tests. No change in behavior.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateNormalAttrGetter):
+ (GenerateFunctionCallString):
+ (NativeToJSValue):
+ * bindings/v8/V8Binding.cpp:
+ (WebCore::getElementStringAttr):
+ * bindings/v8/V8Binding.h:
+ (WebCore::v8ExternalString): Make 'isolate' an optional argument.
+ Ideally we want to make 'isolate' a non-optional argument,
+ but it is difficult to rewrite all v8ExternalString() callers
+ at a breath. We can rewrite them incrementally.
+ (WebCore::v8String): Ditto.
+ (WebCore::v8StringOrNull): Ditto.
+ (WebCore::v8StringOrUndefined): Ditto.
+ (WebCore::v8StringOrFalse): Ditto.
+
+ * bindings/scripts/test/V8/V8TestEventConstructor.cpp: Updated run-bindings-tests results.
+ (WebCore::TestEventConstructorV8Internal::attr1AttrGetter):
+ (WebCore::TestEventConstructorV8Internal::attr2AttrGetter):
+ * bindings/scripts/test/V8/V8TestInterface.cpp:
+ (WebCore::TestInterfaceV8Internal::supplementalStr1AttrGetter):
+ (WebCore::TestInterfaceV8Internal::supplementalStr2AttrGetter):
+ * bindings/scripts/test/V8/V8TestObj.cpp:
+ (WebCore::TestObjV8Internal::readOnlyStringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedStringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedURLAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedCustomURLAttrAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrWithGetterExceptionAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrWithSetterExceptionAttrGetter):
+ (WebCore::TestObjV8Internal::hashAttrGetter):
+ (WebCore::TestObjV8Internal::conditionalMethod1Callback):
+
+2012-04-17 Kentaro Hara <hara...@chromium.org>
+
[V8] Add an optional Isolate argument to wrap()
https://bugs.webkit.org/show_bug.cgi?id=84202
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (114911 => 114912)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-04-23 16:55:13 UTC (rev 114912)
@@ -1010,7 +1010,7 @@
return value;
END
} else {
- push(@implContentDecls, " " . ReturnNativeToJSValue($attribute->signature, $result).";\n");
+ push(@implContentDecls, " " . ReturnNativeToJSValue($attribute->signature, $result, "info.GetIsolate()").";\n");
}
}
@@ -3343,7 +3343,7 @@
}
$return .= ".release()" if ($returnIsRef);
- $result .= $indent . ReturnNativeToJSValue($function->signature, $return) . ";\n";
+ $result .= $indent . ReturnNativeToJSValue($function->signature, $return, "args.GetIsolate()") . ";\n";
return $result;
}
@@ -3760,6 +3760,7 @@
{
my $signature = shift;
my $value = shift;
+ my $getIsolate = shift;
my $type = GetTypeFromSignature($signature);
return "v8Boolean($value)" if $type eq "boolean";
@@ -3790,14 +3791,15 @@
if ($codeGenerator->IsStringType($type)) {
my $conv = $signature->extendedAttributes->{"TreatReturnedNullStringAs"};
+ my $getIsolateArgument = $getIsolate ? ", $getIsolate" : "";
if (defined $conv) {
- return "v8StringOrNull($value)" if $conv eq "Null";
- return "v8StringOrUndefined($value)" if $conv eq "Undefined";
- return "v8StringOrFalse($value)" if $conv eq "False";
+ return "v8StringOrNull($value$getIsolateArgument)" if $conv eq "Null";
+ return "v8StringOrUndefined($value$getIsolateArgument)" if $conv eq "Undefined";
+ return "v8StringOrFalse($value$getIsolateArgument)" if $conv eq "False";
die "Unknown value for TreatReturnedNullStringAs extended attribute";
}
- return "v8String($value)";
+ return "v8String($value$getIsolateArgument)";
}
my $arrayType = $codeGenerator->GetArrayType($type);
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp (114911 => 114912)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp 2012-04-23 16:55:13 UTC (rev 114912)
@@ -43,14 +43,14 @@
{
INC_STATS("DOM.TestEventConstructor.attr1._get");
TestEventConstructor* imp = V8TestEventConstructor::toNative(info.Holder());
- return v8String(imp->attr1());
+ return v8String(imp->attr1(), info.GetIsolate());
}
static v8::Handle<v8::Value> attr2AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
INC_STATS("DOM.TestEventConstructor.attr2._get");
TestEventConstructor* imp = V8TestEventConstructor::toNative(info.Holder());
- return v8String(imp->attr2());
+ return v8String(imp->attr2(), info.GetIsolate());
}
} // namespace TestEventConstructorV8Internal
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp (114911 => 114912)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp 2012-04-23 16:55:13 UTC (rev 114912)
@@ -53,7 +53,7 @@
{
INC_STATS("DOM.TestInterface.supplementalStr1._get");
TestInterface* imp = V8TestInterface::toNative(info.Holder());
- return v8String(TestSupplemental::supplementalStr1(imp));
+ return v8String(TestSupplemental::supplementalStr1(imp), info.GetIsolate());
}
#endif // ENABLE(Condition11) || ENABLE(Condition12)
@@ -64,7 +64,7 @@
{
INC_STATS("DOM.TestInterface.supplementalStr2._get");
TestInterface* imp = V8TestInterface::toNative(info.Holder());
- return v8String(TestSupplemental::supplementalStr2(imp));
+ return v8String(TestSupplemental::supplementalStr2(imp), info.GetIsolate());
}
#endif // ENABLE(Condition11) || ENABLE(Condition12)
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp (114911 => 114912)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2012-04-23 16:55:13 UTC (rev 114912)
@@ -90,7 +90,7 @@
{
INC_STATS("DOM.TestObj.readOnlyStringAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->readOnlyStringAttr());
+ return v8String(imp->readOnlyStringAttr(), info.GetIsolate());
}
static v8::Handle<v8::Value> readOnlyTestObjAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
@@ -191,7 +191,7 @@
{
INC_STATS("DOM.TestObj.stringAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->stringAttr());
+ return v8String(imp->stringAttr(), info.GetIsolate());
}
static void stringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -479,7 +479,7 @@
{
INC_STATS("DOM.TestObj.reflectedStringAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getAttribute(WebCore::HTMLNames::reflectedstringattrAttr));
+ return v8String(imp->getAttribute(WebCore::HTMLNames::reflectedstringattrAttr), info.GetIsolate());
}
static void reflectedStringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -543,7 +543,7 @@
{
INC_STATS("DOM.TestObj.reflectedURLAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getURLAttribute(WebCore::HTMLNames::reflectedurlattrAttr));
+ return v8String(imp->getURLAttribute(WebCore::HTMLNames::reflectedurlattrAttr), info.GetIsolate());
}
static void reflectedURLAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -559,7 +559,7 @@
{
INC_STATS("DOM.TestObj.reflectedStringAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getAttribute(WebCore::HTMLNames::customContentStringAttrAttr));
+ return v8String(imp->getAttribute(WebCore::HTMLNames::customContentStringAttrAttr), info.GetIsolate());
}
static void reflectedStringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -607,7 +607,7 @@
{
INC_STATS("DOM.TestObj.reflectedCustomURLAttr._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getURLAttribute(WebCore::HTMLNames::customContentURLAttrAttr));
+ return v8String(imp->getURLAttribute(WebCore::HTMLNames::customContentURLAttrAttr), info.GetIsolate());
}
static void reflectedCustomURLAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -673,7 +673,7 @@
V8Proxy::setDOMException(ec);
return v8::Handle<v8::Value>();
}
- return v8String(v);
+ return v8String(v, info.GetIsolate());
}
static void stringAttrWithGetterExceptionAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -692,7 +692,7 @@
{
INC_STATS("DOM.TestObj.stringAttrWithSetterException._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->stringAttrWithSetterException());
+ return v8String(imp->stringAttrWithSetterException(), info.GetIsolate());
}
static void stringAttrWithSetterExceptionAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -1207,7 +1207,7 @@
{
INC_STATS("DOM.TestObj.hash._get");
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->hash());
+ return v8String(imp->hash(), info.GetIsolate());
}
static v8::Handle<v8::Value> TestObjConstructorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
@@ -1664,7 +1664,7 @@
{
INC_STATS("DOM.TestObj.conditionalMethod1");
TestObj* imp = V8TestObj::toNative(args.Holder());
- return v8String(imp->conditionalMethod1());
+ return v8String(imp->conditionalMethod1(), args.GetIsolate());
}
#endif // ENABLE(Condition1)
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.cpp (114911 => 114912)
--- trunk/Source/WebCore/bindings/v8/V8Binding.cpp 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.cpp 2012-04-23 16:55:13 UTC (rev 114912)
@@ -551,7 +551,7 @@
const QualifiedName& name)
{
Element* imp = V8Element::toNative(info.Holder());
- return v8ExternalString(imp->getAttribute(name));
+ return v8ExternalString(imp->getAttribute(name), info.GetIsolate());
}
void setElementStringAttr(const v8::AccessorInfo& info,
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (114911 => 114912)
--- trunk/Source/WebCore/bindings/v8/V8Binding.h 2012-04-23 16:50:53 UTC (rev 114911)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h 2012-04-23 16:55:13 UTC (rev 114912)
@@ -244,20 +244,20 @@
// Return a V8 external string that shares the underlying buffer with the given
// WebCore string. The reference counting mechanism is used to keep the
// underlying buffer alive while the string is still live in the V8 engine.
- inline v8::Local<v8::String> v8ExternalString(const String& string)
+ inline v8::Local<v8::String> v8ExternalString(const String& string, v8::Isolate* isolate = 0)
{
StringImpl* stringImpl = string.impl();
if (!stringImpl)
return v8::String::Empty();
- V8BindingPerIsolateData* data = ""
+ V8BindingPerIsolateData* data = "" ? static_cast<V8BindingPerIsolateData*>(isolate->GetData()) : V8BindingPerIsolateData::current();
return data->stringCache()->v8ExternalString(stringImpl);
}
// Convert a string to a V8 string.
- inline v8::Handle<v8::String> v8String(const String& string)
+ inline v8::Handle<v8::String> v8String(const String& string, v8::Isolate* isolate = 0)
{
- return v8ExternalString(string);
+ return v8ExternalString(string, isolate);
}
template<typename T>
@@ -396,19 +396,19 @@
return v8::String::NewUndetectable(fromWebCoreString(str), str.length());
}
- inline v8::Handle<v8::Value> v8StringOrNull(const String& str)
+ inline v8::Handle<v8::Value> v8StringOrNull(const String& str, v8::Isolate* isolate = 0)
{
- return str.isNull() ? v8::Handle<v8::Value>(v8::Null()) : v8::Handle<v8::Value>(v8String(str));
+ return str.isNull() ? v8::Handle<v8::Value>(v8::Null()) : v8::Handle<v8::Value>(v8String(str, isolate));
}
- inline v8::Handle<v8::Value> v8StringOrUndefined(const String& str)
+ inline v8::Handle<v8::Value> v8StringOrUndefined(const String& str, v8::Isolate* isolate = 0)
{
- return str.isNull() ? v8::Handle<v8::Value>(v8::Undefined()) : v8::Handle<v8::Value>(v8String(str));
+ return str.isNull() ? v8::Handle<v8::Value>(v8::Undefined()) : v8::Handle<v8::Value>(v8String(str, isolate));
}
- inline v8::Handle<v8::Value> v8StringOrFalse(const String& str)
+ inline v8::Handle<v8::Value> v8StringOrFalse(const String& str, v8::Isolate* isolate = 0)
{
- return str.isNull() ? v8::Handle<v8::Value>(v8::False()) : v8::Handle<v8::Value>(v8String(str));
+ return str.isNull() ? v8::Handle<v8::Value>(v8::False()) : v8::Handle<v8::Value>(v8String(str, isolate));
}
template <class T> v8::Handle<v8::Value> v8NumberArray(const Vector<T>& values)
