Title: [197485] trunk/Source/_javascript_Core
- Revision
- 197485
- Author
- fpi...@apple.com
- Date
- 2016-03-02 18:06:18 -0800 (Wed, 02 Mar 2016)
Log Message
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: trunk/Source/_javascript_Core/ChangeLog (197484 => 197485)
--- trunk/Source/_javascript_Core/ChangeLog 2016-03-03 02:04:49 UTC (rev 197484)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-03-03 02:06:18 UTC (rev 197485)
@@ -1,3 +1,40 @@
+2016-03-02 Filip Pizlo <fpi...@apple.com>
+
+ 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 Saam barati <sbar...@apple.com>
clean up JSObject::isExtensibleInline and JSObject::setPrototypeOfInline, and rename setPrototypeOf to setPrototype
Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp (197484 => 197485)
--- trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp 2016-03-03 02:04:49 UTC (rev 197484)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp 2016-03-03 02:06:18 UTC (rev 197485)
@@ -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: trunk/Source/_javascript_Core/runtime/JSCJSValue.h (197484 => 197485)
--- trunk/Source/_javascript_Core/runtime/JSCJSValue.h 2016-03-03 02:04:49 UTC (rev 197484)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.h 2016-03-03 02:06:18 UTC (rev 197485)
@@ -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: trunk/Source/_javascript_Core/runtime/JSString.h (197484 => 197485)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2016-03-03 02:04:49 UTC (rev 197484)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2016-03-03 02:06:18 UTC (rev 197485)
@@ -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: trunk/Source/_javascript_Core/runtime/RegExpPrototype.cpp (197484 => 197485)
--- trunk/Source/_javascript_Core/runtime/RegExpPrototype.cpp 2016-03-03 02:04:49 UTC (rev 197484)
+++ trunk/Source/_javascript_Core/runtime/RegExpPrototype.cpp 2016-03-03 02:06:18 UTC (rev 197485)
@@ -99,7 +99,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)
@@ -107,7 +110,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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
