Skip to content

Commit

Permalink
Make JSCRuntime::createValue 35% faster (#27016)
Browse files Browse the repository at this point in the history
Summary:
JSC does some sort of thread safety locking on every single JSC API call, which makes them ridiculously expensive for a large number of calls (such as when passing a large array over the RN bridge). It would be great if we could lock and unlock once for an entire sequence of JSC calls… but in the meantime, the less we call JSC the better.

![unknown](https://user-images.githubusercontent.com/183747/67624956-08bd6e00-f838-11e9-8f65-e077693f113d.png)

In my benchmark environment (Nozbe/WatermelonDB#541), the time spent in JSCRuntime::createValue went down from 1.07s to 0.69s by changing JSValueIsXXXX calls to a single JSValueGetType call.

The underlying implementation does the same thing: https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/API/JSValueRef.cpp#L58

## Changelog

[General] [Fixed] - Make JSCRuntime::createValue faster
Pull Request resolved: #27016

Reviewed By: RSNara

Differential Revision: D18769047

Pulled By: mhorowitz

fbshipit-source-id: 9d1ee28840303f7721e065c1b3c347e354cd7fef
  • Loading branch information
radex authored and facebook-github-bot committed Feb 20, 2020
1 parent a793ed7 commit 24e0bad
Showing 1 changed file with 42 additions and 24 deletions.
66 changes: 42 additions & 24 deletions ReactCommon/jsi/JSCRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,18 @@ bool JSCRuntime::isInspectable() {
namespace {

bool smellsLikeES6Symbol(JSGlobalContextRef ctx, JSValueRef ref) {
// Empirically, an es6 Symbol is not an object, but its type is
// Since iOS 13, JSValueGetType will return kJSTypeSymbol
// Before: Empirically, an es6 Symbol is not an object, but its type is
// object. This makes no sense, but we'll run with it.
return (
!JSValueIsObject(ctx, ref) && JSValueGetType(ctx, ref) == kJSTypeObject);
// https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/API/JSValueRef.cpp#L79-L82

JSType type = JSValueGetType(ctx, ref);

if (type == /* kJSTypeSymbol */ 6) {
return true;
}

return (!JSValueIsObject(ctx, ref) && type == kJSTypeObject);
}

} // namespace
Expand Down Expand Up @@ -1339,27 +1347,37 @@ jsi::Object JSCRuntime::createObject(JSObjectRef obj) const {
}

jsi::Value JSCRuntime::createValue(JSValueRef value) const {
if (JSValueIsNumber(ctx_, value)) {
return jsi::Value(JSValueToNumber(ctx_, value, nullptr));
} else if (JSValueIsBoolean(ctx_, value)) {
return jsi::Value(JSValueToBoolean(ctx_, value));
} else if (JSValueIsNull(ctx_, value)) {
return jsi::Value(nullptr);
} else if (JSValueIsUndefined(ctx_, value)) {
return jsi::Value();
} else if (JSValueIsString(ctx_, value)) {
JSStringRef str = JSValueToStringCopy(ctx_, value, nullptr);
auto result = jsi::Value(createString(str));
JSStringRelease(str);
return result;
} else if (JSValueIsObject(ctx_, value)) {
JSObjectRef objRef = JSValueToObject(ctx_, value, nullptr);
return jsi::Value(createObject(objRef));
} else if (smellsLikeES6Symbol(ctx_, value)) {
return jsi::Value(createSymbol(value));
} else {
// WHAT ARE YOU
abort();
JSType type = JSValueGetType(ctx_, value);

switch (type) {
case kJSTypeNumber:
return jsi::Value(JSValueToNumber(ctx_, value, nullptr));
case kJSTypeBoolean:
return jsi::Value(JSValueToBoolean(ctx_, value));
case kJSTypeNull:
return jsi::Value(nullptr);
case kJSTypeUndefined:
return jsi::Value();
case kJSTypeString: {
JSStringRef str = JSValueToStringCopy(ctx_, value, nullptr);
auto result = jsi::Value(createString(str));
JSStringRelease(str);
return result;
}
case kJSTypeObject: {
JSObjectRef objRef = JSValueToObject(ctx_, value, nullptr);
return jsi::Value(createObject(objRef));
}
// TODO: Uncomment this when all supported JSC versions have this symbol
// case kJSTypeSymbol:
default: {
if (smellsLikeES6Symbol(ctx_, value)) {
return jsi::Value(createSymbol(value));
} else {
// WHAT ARE YOU
abort();
}
}
}
}

Expand Down

0 comments on commit 24e0bad

Please sign in to comment.