From 24e0bad8bea95ef7ddf72e2f00a93ffd47872d5b Mon Sep 17 00:00:00 2001
From: Radek Pietruszewski <radexpl@gmail.com>
Date: Thu, 20 Feb 2020 10:31:12 -0800
Subject: [PATCH] Make JSCRuntime::createValue 35% faster (#27016)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 (https://github.com/Nozbe/WatermelonDB/pull/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: https://github.com/facebook/react-native/pull/27016

Reviewed By: RSNara

Differential Revision: D18769047

Pulled By: mhorowitz

fbshipit-source-id: 9d1ee28840303f7721e065c1b3c347e354cd7fef
---
 ReactCommon/jsi/JSCRuntime.cpp | 66 +++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/ReactCommon/jsi/JSCRuntime.cpp b/ReactCommon/jsi/JSCRuntime.cpp
index 8b88e584a1c2a4..f96cab8d73fa7d 100644
--- a/ReactCommon/jsi/JSCRuntime.cpp
+++ b/ReactCommon/jsi/JSCRuntime.cpp
@@ -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
@@ -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();
+      }
+    }
   }
 }