Title: [197751] releases/WebKitGTK/webkit-2.12/Source/_javascript_Core
Revision
197751
Author
[email protected]
Date
2016-03-08 02:11:41 -0800 (Tue, 08 Mar 2016)

Log Message

Merge r197485 - RegExpPrototype should check for exceptions after calling toString and doing so should not be expensive
https://bugs.webkit.org/show_bug.cgi?id=154927

Reviewed by Saam Barati.

While working on regexp optimizations, I found that RegExpPrototype calls toString(), an
effectful operation that could do anything, without then checking for hadException().

So I added a call to hadException().

But that regressed Octane/regexp by 5%!  That's a lot!  It turns out that
exec->hadException() is soooper slow. So, I made it cheaper to check for exceptions from
toString(): there is now a variant called toStringFast() that returns null iff it throws an
exception.

This allowed me to add the exception check without regressing perf.

Note that toString() must retain its old behavior of returning an empty string on exception.
There is just too much code that relies on that behavior.

* runtime/JSCJSValue.cpp:
(JSC::JSValue::isValidCallee):
(JSC::JSValue::toStringSlowCase):
(JSC::JSValue::toWTFStringSlowCase):
* runtime/JSCJSValue.h:
(JSC::JSValue::asValue):
* runtime/JSString.h:
(JSC::JSValue::toString):
(JSC::JSValue::toStringFast):
(JSC::JSValue::toWTFString):
* runtime/RegExpPrototype.cpp:
(JSC::regExpProtoFuncTest):
(JSC::regExpProtoFuncExec):
(JSC::regExpProtoFuncCompile):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog (197750 => 197751)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-03-08 09:48:32 UTC (rev 197750)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-03-08 10:11:41 UTC (rev 197751)
@@ -1,3 +1,40 @@
+2016-03-02  Filip Pizlo  <[email protected]>
+
+        RegExpPrototype should check for exceptions after calling toString and doing so should not be expensive
+        https://bugs.webkit.org/show_bug.cgi?id=154927
+
+        Reviewed by Saam Barati.
+
+        While working on regexp optimizations, I found that RegExpPrototype calls toString(), an
+        effectful operation that could do anything, without then checking for hadException().
+
+        So I added a call to hadException().
+
+        But that regressed Octane/regexp by 5%!  That's a lot!  It turns out that
+        exec->hadException() is soooper slow. So, I made it cheaper to check for exceptions from
+        toString(): there is now a variant called toStringFast() that returns null iff it throws an
+        exception.
+
+        This allowed me to add the exception check without regressing perf.
+
+        Note that toString() must retain its old behavior of returning an empty string on exception.
+        There is just too much code that relies on that behavior.
+
+        * runtime/JSCJSValue.cpp:
+        (JSC::JSValue::isValidCallee):
+        (JSC::JSValue::toStringSlowCase):
+        (JSC::JSValue::toWTFStringSlowCase):
+        * runtime/JSCJSValue.h:
+        (JSC::JSValue::asValue):
+        * runtime/JSString.h:
+        (JSC::JSValue::toString):
+        (JSC::JSValue::toStringFast):
+        (JSC::JSValue::toWTFString):
+        * runtime/RegExpPrototype.cpp:
+        (JSC::regExpProtoFuncTest):
+        (JSC::regExpProtoFuncExec):
+        (JSC::regExpProtoFuncCompile):
+
 2016-03-02  Benjamin Poulain  <[email protected]>
 
         [JSC] Use a Move without REX byte when possible

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.cpp (197750 => 197751)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.cpp	2016-03-08 09:48:32 UTC (rev 197750)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.cpp	2016-03-08 10:11:41 UTC (rev 197751)
@@ -361,8 +361,14 @@
     return asObject(asCell())->globalObject();
 }
 
-JSString* JSValue::toStringSlowCase(ExecState* exec) const
+JSString* JSValue::toStringSlowCase(ExecState* exec, bool returnEmptyStringOnError) const
 {
+    auto errorValue = [&] () -> JSString* {
+        if (returnEmptyStringOnError)
+            return jsEmptyString(exec);
+        return nullptr;
+    };
+    
     VM& vm = exec->vm();
     ASSERT(!isString());
     if (isInt32()) {
@@ -383,15 +389,18 @@
         return vm.smallStrings.undefinedString();
     if (isSymbol()) {
         throwTypeError(exec);
-        return jsEmptyString(exec);
+        return errorValue();
     }
 
     ASSERT(isCell());
     JSValue value = asCell()->toPrimitive(exec, PreferString);
-    if (exec->hadException())
-        return jsEmptyString(exec);
+    if (vm.exception())
+        return errorValue();
     ASSERT(!value.isObject());
-    return value.toString(exec);
+    JSString* result = value.toString(exec);
+    if (vm.exception())
+        return errorValue();
+    return result;
 }
 
 String JSValue::toWTFStringSlowCase(ExecState* exec) const

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.h (197750 => 197751)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.h	2016-03-08 09:48:32 UTC (rev 197750)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSCJSValue.h	2016-03-08 10:11:41 UTC (rev 197751)
@@ -252,7 +252,8 @@
     // toNumber conversion is expected to be side effect free if an exception has
     // been set in the ExecState already.
     double toNumber(ExecState*) const;
-    JSString* toString(ExecState*) const;
+    JSString* toString(ExecState*) const; // On exception, this returns the empty string.
+    JSString* toStringOrNull(ExecState*) const; // On exception, this returns null, to make exception checks faster.
     Identifier toPropertyKey(ExecState*) const;
     WTF::String toWTFString(ExecState*) const;
     JSObject* toObject(ExecState*) const;
@@ -436,7 +437,7 @@
 
     inline const JSValue asValue() const { return *this; }
     JS_EXPORT_PRIVATE double toNumberSlowCase(ExecState*) const;
-    JS_EXPORT_PRIVATE JSString* toStringSlowCase(ExecState*) const;
+    JS_EXPORT_PRIVATE JSString* toStringSlowCase(ExecState*, bool returnEmptyStringOnError) const;
     JS_EXPORT_PRIVATE WTF::String toWTFStringSlowCase(ExecState*) const;
     JS_EXPORT_PRIVATE JSObject* toObjectSlowCase(ExecState*, JSGlobalObject*) const;
     JS_EXPORT_PRIVATE JSValue toThisSlowCase(ExecState*, ECMAMode) const;

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSString.h (197750 => 197751)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSString.h	2016-03-08 09:48:32 UTC (rev 197750)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/JSString.h	2016-03-08 10:11:41 UTC (rev 197751)
@@ -733,9 +733,18 @@
 {
     if (isString())
         return jsCast<JSString*>(asCell());
-    return toStringSlowCase(exec);
+    bool returnEmptyStringOnError = true;
+    return toStringSlowCase(exec, returnEmptyStringOnError);
 }
 
+inline JSString* JSValue::toStringOrNull(ExecState* exec) const
+{
+    if (isString())
+        return jsCast<JSString*>(asCell());
+    bool returnEmptyStringOnError = false;
+    return toStringSlowCase(exec, returnEmptyStringOnError);
+}
+
 inline String JSValue::toWTFString(ExecState* exec) const
 {
     if (isString())

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/RegExpPrototype.cpp (197750 => 197751)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/RegExpPrototype.cpp	2016-03-08 09:48:32 UTC (rev 197750)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/runtime/RegExpPrototype.cpp	2016-03-08 10:11:41 UTC (rev 197751)
@@ -97,7 +97,10 @@
     JSValue thisValue = exec->thisValue();
     if (!thisValue.inherits(RegExpObject::info()))
         return throwVMTypeError(exec);
-    return JSValue::encode(jsBoolean(asRegExpObject(thisValue)->test(exec, exec->argument(0).toString(exec))));
+    JSString* string = exec->argument(0).toStringOrNull(exec);
+    if (!string)
+        return JSValue::encode(jsUndefined());
+    return JSValue::encode(jsBoolean(asRegExpObject(thisValue)->test(exec, string)));
 }
 
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncExec(ExecState* exec)
@@ -105,7 +108,10 @@
     JSValue thisValue = exec->thisValue();
     if (!thisValue.inherits(RegExpObject::info()))
         return throwVMTypeError(exec);
-    return JSValue::encode(asRegExpObject(thisValue)->exec(exec, exec->argument(0).toString(exec)));
+    JSString* string = exec->argument(0).toStringOrNull(exec);
+    if (!string)
+        return JSValue::encode(jsUndefined());
+    return JSValue::encode(asRegExpObject(thisValue)->exec(exec, string));
 }
 
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState* exec)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to