Diff
Modified: trunk/Source/WebCore/ChangeLog (139468 => 139469)
--- trunk/Source/WebCore/ChangeLog 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/ChangeLog 2013-01-11 19:27:39 UTC (rev 139469)
@@ -1,3 +1,68 @@
+2013-01-11 Kentaro Hara <[email protected]>
+
+ [V8] Do not create a local handle for a cached v8 string that is returned to V8 immediately
+ https://bugs.webkit.org/show_bug.cgi?id=106557
+
+ Reviewed by Adam Barth.
+
+ Currently we are always creating a local handle for a cached
+ V8 string returned to V8:
+
+ Handle<Value> v8String(StringImpl* impl, Isolate* isolate) {
+ ...;
+ return Local<String>::New(isolate, m_cachedString);
+ }
+
+ However, we don't need to create a local handle in a case
+ where it is guaranteed that no V8 object allocation is conducted
+ before a control flow returns back to V8. In particular, in a case
+ where a cached V8 string is immediately returned to V8, we don't
+ need to create a local handle:
+
+ Handle<Value> xxxxAttrGetter() {
+ ...;
+ return v8String(imp->xxxx(), isolate); // This can return a persistent handle safely.
+ }
+
+ This patch improves performance of div.id by 9.2%.
+
+ No tests. No change in behavior.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateNormalAttrGetter):
+ (GenerateCallbackImplementation):
+ (GenerateFunctionCallString):
+ (NativeToJSValue):
+ * bindings/scripts/test/V8/V8TestEventConstructor.cpp:
+ (WebCore::TestEventConstructorV8Internal::attr1AttrGetter):
+ (WebCore::TestEventConstructorV8Internal::attr2AttrGetter):
+ * bindings/scripts/test/V8/V8TestException.cpp:
+ (WebCore::TestExceptionV8Internal::nameAttrGetter):
+ * bindings/scripts/test/V8/V8TestInterface.cpp:
+ (WebCore::TestInterfaceV8Internal::supplementalStaticAttrAttrGetter):
+ (WebCore::TestInterfaceV8Internal::supplementalStr1AttrGetter):
+ (WebCore::TestInterfaceV8Internal::supplementalStr2AttrGetter):
+ * bindings/scripts/test/V8/V8TestObj.cpp:
+ (WebCore::TestObjV8Internal::readOnlyStringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::staticStringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedStringAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedURLAttrAttrGetter):
+ (WebCore::TestObjV8Internal::reflectedCustomURLAttrAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrWithGetterExceptionAttrGetter):
+ (WebCore::TestObjV8Internal::stringAttrWithSetterExceptionAttrGetter):
+ (WebCore::TestObjV8Internal::hashAttrGetter):
+ (WebCore::TestObjV8Internal::conditionalMethod1Callback):
+ * bindings/v8/V8Binding.h:
+ (WebCore::v8String):
+ (WebCore::v8StringOrNull):
+ (WebCore::v8StringOrUndefined):
+ * bindings/v8/V8ValueCache.cpp:
+ (WebCore::StringCache::v8ExternalStringSlow):
+ * bindings/v8/V8ValueCache.h:
+ (WebCore::StringCache::v8ExternalString):
+ (StringCache):
+
2013-01-11 Carlos Garcia Campos <[email protected]>
Unreviewed. Fix make distcheck.
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (139468 => 139469)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2013-01-11 19:27:39 UTC (rev 139469)
@@ -929,7 +929,7 @@
# Generate super-compact call for regular attribute getter:
my ($functionName, @arguments) = $codeGenerator->GetterExpression(\%implIncludes, $interfaceName, $attribute);
push(@implContentDecls, " Element* imp = V8Element::toNative(info.Holder());\n");
- push(@implContentDecls, " return v8String(imp->${functionName}(" . join(", ", @arguments) . "), info.GetIsolate());\n");
+ push(@implContentDecls, " return v8String(imp->${functionName}(" . join(", ", @arguments) . "), info.GetIsolate(), ReturnUnsafeHandle);\n");
push(@implContentDecls, "}\n\n");
push(@implContentDecls, "#endif // ${conditionalString}\n\n") if $conditionalString;
return;
@@ -1114,7 +1114,7 @@
return value;
END
} else {
- push(@implContentDecls, " return " . NativeToJSValue($attribute->signature, $result, "info.Holder()", "info.GetIsolate()", "info", "imp").";\n");
+ push(@implContentDecls, " return " . NativeToJSValue($attribute->signature, $result, "info.Holder()", "info.GetIsolate()", "info", "imp", "ReturnUnsafeHandle").";\n");
}
push(@implContentDecls, "}\n\n"); # end of getter
@@ -3618,9 +3618,9 @@
my $nativeValue;
# FIXME: Update for all ScriptWrappables.
if (IsDOMNodeType($interfaceName)) {
- $nativeValue = NativeToJSValue($function->signature, $return, "args.Holder()", "args.GetIsolate()", "args", "imp");
+ $nativeValue = NativeToJSValue($function->signature, $return, "args.Holder()", "args.GetIsolate()", "args", "imp", "ReturnUnsafeHandle");
} else {
- $nativeValue = NativeToJSValue($function->signature, $return, "args.Holder()", "args.GetIsolate()");
+ $nativeValue = NativeToJSValue($function->signature, $return, "args.Holder()", "args.GetIsolate()", 0, 0, "ReturnUnsafeHandle");
}
$result .= $indent . "return " . $nativeValue . ";\n";
@@ -4030,6 +4030,8 @@
my $getHolderContainerArg = $getHolderContainer ? ", $getHolderContainer" : "";
my $getScriptWrappable = shift;
my $getScriptWrappableArg = $getScriptWrappable ? ", $getScriptWrappable" : "";
+ my $returnHandleType = shift;
+
my $type = $signature->type;
return ($getIsolate ? "v8Boolean($value, $getIsolate)" : "v8Boolean($value)") if $type eq "boolean";
@@ -4058,12 +4060,12 @@
if ($codeGenerator->IsStringType($type)) {
my $conv = $signature->extendedAttributes->{"TreatReturnedNullStringAs"};
if (defined $conv) {
- return "v8StringOrNull($value$getIsolateArg)" if $conv eq "Null";
- return "v8StringOrUndefined($value$getIsolateArg)" if $conv eq "Undefined";
+ return "v8StringOrNull($value$getIsolateArg, $returnHandleType)" if $conv eq "Null";
+ return "v8StringOrUndefined($value$getIsolateArg, $returnHandleType)" if $conv eq "Undefined";
die "Unknown value for TreatReturnedNullStringAs extended attribute";
}
- return $getIsolate ? "v8String($value, $getIsolate)" : "deprecatedV8String($value)";
+ return $getIsolate ? "v8String($value$getIsolateArg, $returnHandleType)" : "deprecatedV8String($value)";
}
my $arrayType = $codeGenerator->GetArrayType($type);
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp (139468 => 139469)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp 2013-01-11 19:27:39 UTC (rev 139469)
@@ -41,13 +41,13 @@
static v8::Handle<v8::Value> attr1AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestEventConstructor* imp = V8TestEventConstructor::toNative(info.Holder());
- return v8String(imp->attr1(), info.GetIsolate());
+ return v8String(imp->attr1(), info.GetIsolate(), ReturnPersistentHandle);
}
static v8::Handle<v8::Value> attr2AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestEventConstructor* imp = V8TestEventConstructor::toNative(info.Holder());
- return v8String(imp->attr2(), info.GetIsolate());
+ return v8String(imp->attr2(), info.GetIsolate(), ReturnPersistentHandle);
}
} // namespace TestEventConstructorV8Internal
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestException.cpp (139468 => 139469)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestException.cpp 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestException.cpp 2013-01-11 19:27:39 UTC (rev 139469)
@@ -40,7 +40,7 @@
static v8::Handle<v8::Value> nameAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestException* imp = V8TestException::toNative(info.Holder());
- return v8String(imp->name(), info.GetIsolate());
+ return v8String(imp->name(), info.GetIsolate(), ReturnPersistentHandle);
}
} // namespace TestExceptionV8Internal
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp (139468 => 139469)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp 2013-01-11 19:27:39 UTC (rev 139469)
@@ -59,7 +59,7 @@
static v8::Handle<v8::Value> supplementalStaticAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
- return v8String(TestSupplemental::supplementalStaticAttr(), info.GetIsolate());
+ return v8String(TestSupplemental::supplementalStaticAttr(), info.GetIsolate(), ReturnPersistentHandle);
}
#endif // ENABLE(Condition11) || ENABLE(Condition12)
@@ -80,7 +80,7 @@
static v8::Handle<v8::Value> supplementalStr1AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestInterface* imp = V8TestInterface::toNative(info.Holder());
- return v8String(TestSupplemental::supplementalStr1(imp), info.GetIsolate());
+ return v8String(TestSupplemental::supplementalStr1(imp), info.GetIsolate(), ReturnPersistentHandle);
}
#endif // ENABLE(Condition11) || ENABLE(Condition12)
@@ -90,7 +90,7 @@
static v8::Handle<v8::Value> supplementalStr2AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestInterface* imp = V8TestInterface::toNative(info.Holder());
- return v8String(TestSupplemental::supplementalStr2(imp), info.GetIsolate());
+ return v8String(TestSupplemental::supplementalStr2(imp), info.GetIsolate(), ReturnPersistentHandle);
}
#endif // ENABLE(Condition11) || ENABLE(Condition12)
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp (139468 => 139469)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2013-01-11 19:27:39 UTC (rev 139469)
@@ -90,7 +90,7 @@
static v8::Handle<v8::Value> readOnlyStringAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->readOnlyStringAttr(), info.GetIsolate());
+ return v8String(imp->readOnlyStringAttr(), info.GetIsolate(), ReturnPersistentHandle);
}
static v8::Handle<v8::Value> readOnlyTestObjAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
@@ -113,7 +113,7 @@
static v8::Handle<v8::Value> staticStringAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
- return v8String(TestObj::staticStringAttr(), info.GetIsolate());
+ return v8String(TestObj::staticStringAttr(), info.GetIsolate(), ReturnPersistentHandle);
}
static void staticStringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -196,7 +196,7 @@
static v8::Handle<v8::Value> stringAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->stringAttr(), info.GetIsolate());
+ return v8String(imp->stringAttr(), info.GetIsolate(), ReturnPersistentHandle);
}
static void stringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -254,7 +254,7 @@
static v8::Handle<v8::Value> reflectedStringAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->fastGetAttribute(WebCore::HTMLNames::reflectedstringattrAttr), info.GetIsolate());
+ return v8String(imp->fastGetAttribute(WebCore::HTMLNames::reflectedstringattrAttr), info.GetIsolate(), ReturnPersistentHandle);
}
static void reflectedStringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -310,7 +310,7 @@
static v8::Handle<v8::Value> reflectedURLAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getURLAttribute(WebCore::HTMLNames::reflectedurlattrAttr), info.GetIsolate());
+ return v8String(imp->getURLAttribute(WebCore::HTMLNames::reflectedurlattrAttr), info.GetIsolate(), ReturnPersistentHandle);
}
static void reflectedURLAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -324,7 +324,7 @@
static v8::Handle<v8::Value> reflectedStringAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->fastGetAttribute(WebCore::HTMLNames::customContentStringAttrAttr), info.GetIsolate());
+ return v8String(imp->fastGetAttribute(WebCore::HTMLNames::customContentStringAttrAttr), info.GetIsolate(), ReturnPersistentHandle);
}
static void reflectedStringAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -366,7 +366,7 @@
static v8::Handle<v8::Value> reflectedCustomURLAttrAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->getURLAttribute(WebCore::HTMLNames::customContentURLAttrAttr), info.GetIsolate());
+ return v8String(imp->getURLAttribute(WebCore::HTMLNames::customContentURLAttrAttr), info.GetIsolate(), ReturnPersistentHandle);
}
static void reflectedCustomURLAttrAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -433,7 +433,7 @@
String v = imp->stringAttrWithGetterException(ec);
if (UNLIKELY(ec))
return setDOMException(ec, info.GetIsolate());
- return v8String(v, info.GetIsolate());
+ return v8String(v, info.GetIsolate(), ReturnPersistentHandle);
}
static void stringAttrWithGetterExceptionAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -447,7 +447,7 @@
static v8::Handle<v8::Value> stringAttrWithSetterExceptionAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->stringAttrWithSetterException(), info.GetIsolate());
+ return v8String(imp->stringAttrWithSetterException(), info.GetIsolate(), ReturnPersistentHandle);
}
static void stringAttrWithSetterExceptionAttrSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
@@ -913,7 +913,7 @@
static v8::Handle<v8::Value> hashAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
TestObj* imp = V8TestObj::toNative(info.Holder());
- return v8String(imp->hash(), info.GetIsolate());
+ return v8String(imp->hash(), info.GetIsolate(), ReturnPersistentHandle);
}
static v8::Handle<v8::Value> replaceableAttributeAttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
@@ -1371,7 +1371,7 @@
static v8::Handle<v8::Value> conditionalMethod1Callback(const v8::Arguments& args)
{
TestObj* imp = V8TestObj::toNative(args.Holder());
- return v8String(imp->conditionalMethod1(), args.GetIsolate());
+ return v8String(imp->conditionalMethod1(), args.GetIsolate(), ReturnPersistentHandle);
}
#endif // ENABLE(Condition1)
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (139468 => 139469)
--- trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-01-11 19:27:39 UTC (rev 139469)
@@ -145,13 +145,13 @@
// 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::Handle<v8::String> v8String(const String& string, v8::Isolate* isolate = 0)
+ inline v8::Handle<v8::String> v8String(const String& string, v8::Isolate* isolate = 0, ReturnHandleType handleType = ReturnLocalHandle)
{
if (UNLIKELY(!isolate))
isolate = v8::Isolate::GetCurrent();
if (string.isNull())
return v8::String::Empty(isolate);
- return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), isolate);
+ return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), handleType, isolate);
}
// FIXME: All call sites of this method should use v8String().
@@ -160,20 +160,20 @@
return v8String(string, v8::Isolate::GetCurrent());
}
- inline v8::Handle<v8::Value> v8StringOrNull(const String& string, v8::Isolate* isolate)
+ inline v8::Handle<v8::Value> v8StringOrNull(const String& string, v8::Isolate* isolate, ReturnHandleType handleType = ReturnLocalHandle)
{
ASSERT(isolate);
if (string.isNull())
return v8Null(isolate);
- return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), isolate);
+ return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), handleType, isolate);
}
- inline v8::Handle<v8::Value> v8StringOrUndefined(const String& string, v8::Isolate* isolate)
+ inline v8::Handle<v8::Value> v8StringOrUndefined(const String& string, v8::Isolate* isolate, ReturnHandleType handleType = ReturnLocalHandle)
{
ASSERT(isolate);
if (string.isNull())
return v8::Undefined(isolate);
- return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), isolate);
+ return V8PerIsolateData::from(isolate)->stringCache()->v8ExternalString(string.impl(), handleType, isolate);
}
inline v8::Handle<v8::Integer> v8Integer(int value, v8::Isolate* isolate = 0)
Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp (139468 => 139469)
--- trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp 2013-01-11 19:27:39 UTC (rev 139469)
@@ -76,7 +76,7 @@
clearOnGC();
}
-v8::Local<v8::String> StringCache::v8ExternalStringSlow(StringImpl* stringImpl, v8::Isolate* isolate)
+v8::Handle<v8::String> StringCache::v8ExternalStringSlow(StringImpl* stringImpl, ReturnHandleType handleType, v8::Isolate* isolate)
{
if (!stringImpl->length())
return v8::String::Empty(isolate);
@@ -87,6 +87,8 @@
if (handle.IsWeak()) {
m_lastStringImpl = stringImpl;
m_lastV8String = handle;
+ if (handleType == ReturnUnsafeHandle)
+ return handle;
return v8::Local<v8::String>::New(handle);
}
}
Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.h (139468 => 139469)
--- trunk/Source/WebCore/bindings/v8/V8ValueCache.h 2013-01-11 19:21:55 UTC (rev 139468)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.h 2013-01-11 19:27:39 UTC (rev 139469)
@@ -35,16 +35,28 @@
namespace WebCore {
+// ReturnUnsafeHandle can be used only when it is guaranteed
+// that no V8 object allocation is conducted before the handle
+// is returned to V8. For safety, ReturnUnsafeHandle should be
+// used by auto-generated code.
+enum ReturnHandleType {
+ ReturnLocalHandle,
+ ReturnUnsafeHandle
+};
+
class StringCache {
public:
StringCache() { }
- v8::Local<v8::String> v8ExternalString(StringImpl* stringImpl, v8::Isolate* isolate)
+ v8::Handle<v8::String> v8ExternalString(StringImpl* stringImpl, ReturnHandleType handleType, v8::Isolate* isolate)
{
- if (m_lastStringImpl.get() == stringImpl && m_lastV8String.IsWeak(isolate))
+ if (m_lastStringImpl.get() == stringImpl && m_lastV8String.IsWeak(isolate)) {
+ if (handleType == ReturnUnsafeHandle)
+ return m_lastV8String;
return v8::Local<v8::String>::New(isolate, m_lastV8String);
+ }
- return v8ExternalStringSlow(stringImpl, isolate);
+ return v8ExternalStringSlow(stringImpl, handleType, isolate);
}
void clearOnGC()
@@ -57,7 +69,7 @@
void reportMemoryUsage(MemoryObjectInfo*) const;
private:
- v8::Local<v8::String> v8ExternalStringSlow(StringImpl*, v8::Isolate*);
+ v8::Handle<v8::String> v8ExternalStringSlow(StringImpl*, ReturnHandleType, v8::Isolate*);
HashMap<StringImpl*, v8::String*> m_stringCache;
v8::Persistent<v8::String> m_lastV8String;
