Follow-up to #59414: cache each Java TurboModule method's ID with its signature, by name

packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js

@@ -54,8 +54,7 @@ const HostFunctionTemplate = ({
   jsReturnType: JSReturnType,
 }>) => {
   return `static facebook::jsi::Value __hostFunction_${hasteModuleName}SpecJSI_${propertyName}(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
-  static jmethodID cachedMethodId = nullptr;
-  return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, ${jsReturnType}, "${propertyName}", "${jniSignature}", args, count, cachedMethodId);
+  return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, ${jsReturnType}, "${propertyName}", "${jniSignature}", args, count);
 }`;
 };
 

packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaInteropTurboModule.cpp

@@ -42,7 +42,6 @@ JavaInteropTurboModule::JavaInteropTurboModule(
     const std::vector<MethodDescriptor>& methodDescriptors)
     : JavaTurboModule(params),
       methodDescriptors_(methodDescriptors),
-      methodIDs_(methodDescriptors.size()),
       constantsCache_(jsi::Value::undefined()) {
   for (const auto& methodDescriptor : methodDescriptors) {
     methodMap_[methodDescriptor.methodName] = MethodMetadata{
@@ -73,11 +72,10 @@ jsi::Value JavaInteropTurboModule::create(
               jsi::Value ret = this->invokeJavaMethod(
                   rt,
                   this->methodDescriptors_[i].jsiReturnKind,
-                  this->methodDescriptors_[i].methodName,
-                  this->methodDescriptors_[i].jniSignature,
+                  this->methodDescriptors_[i].methodName.c_str(),
+                  this->methodDescriptors_[i].jniSignature.c_str(),
                   args,
-                  count,
-                  this->methodIDs_[i]);
+                  count);
 
               bool isRetValid = ret.isUndefined() || ret.isNull() ||
                   (ret.isObject() && !ret.asObject(rt).isFunction(rt));
@@ -112,11 +110,10 @@ jsi::Value JavaInteropTurboModule::create(
             return this->invokeJavaMethod(
                 rt,
                 this->methodDescriptors_[i].jsiReturnKind,
-                this->methodDescriptors_[i].methodName,
-                this->methodDescriptors_[i].jniSignature,
+                this->methodDescriptors_[i].methodName.c_str(),
+                this->methodDescriptors_[i].jniSignature.c_str(),
                 args,
-                count,
-                this->methodIDs_[i]);
+                count);
           });
     }
   }

packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaInteropTurboModule.h

@@ -40,7 +40,6 @@ class JSI_EXPORT JavaInteropTurboModule : public JavaTurboModule {
 
  private:
   std::vector<MethodDescriptor> methodDescriptors_;
-  std::vector<jmethodID> methodIDs_;
   jsi::Value constantsCache_;
 
   const jsi::Value &getConstants(jsi::Runtime &runtime);

packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp

@@ -311,7 +311,7 @@ int32_t getUniqueId() {
 JNIArgs convertJSIArgsToJNIArgs(
     JNIEnv* env,
     jsi::Runtime& rt,
-    const std::string& methodName,
+    const char* methodName,
     const std::vector<std::string>& methodArgTypes,
     const jsi::Value* args,
     size_t count,
@@ -588,34 +588,57 @@ void rejectWithException(
 
 } // namespace
 
-const JavaTurboModule::MethodInfo& JavaTurboModule::getMethodInfo(
-    jmethodID methodID,
-    const std::string& methodSignature) {
+const JavaTurboModule::MethodInfo* JavaTurboModule::getMethodInfo(
+    JNIEnv* env,
+    const char* methodName,
+    const char* methodSignature) {
   std::lock_guard<std::mutex> lock(methodInfosMutex_);
-  auto entry = methodInfos_.find(methodID);
+  auto entry = methodInfos_.find(std::string_view(methodName));
   if (entry == methodInfos_.end()) {
-    entry =
-        methodInfos_
-            .emplace(
-                methodID,
-                MethodInfo{
-                    .argTypes = getMethodArgTypesFromSignature(methodSignature),
-                    .returnType = methodSignature.substr(
-                        methodSignature.find_last_of(')') + 1)})
-            .first;
+    jclass cls = env->GetObjectClass(instance_.get());
+    jmethodID methodID = env->GetMethodID(cls, methodName, methodSignature);
+    env->DeleteLocalRef(cls);
+    if (methodID == nullptr) {
+      return nullptr;
+    }
+    std::string signature(methodSignature);
+    entry = methodInfos_
+                .emplace(
+                    methodName,
+                    MethodInfo{
+                        .methodID = methodID,
+                        .argTypes = getMethodArgTypesFromSignature(signature),
+                        .returnType =
+                            signature.substr(signature.find_last_of(')') + 1)})
+                .first;
   }
-  return entry->second;
+  return &entry->second;
 }
 
 jsi::Value JavaTurboModule::invokeJavaMethod(
     jsi::Runtime& runtime,
     TurboModuleMethodValueKind valueKind,
-    const std::string& methodNameStr,
+    const std::string& methodName,
     const std::string& methodSignature,
     const jsi::Value* args,
     size_t argCount,
-    jmethodID& methodID) {
-  const char* methodName = methodNameStr.c_str();
+    jmethodID& /*cachedMethodID*/) {
+  return invokeJavaMethod(
+      runtime,
+      valueKind,
+      methodName.c_str(),
+      methodSignature.c_str(),
+      args,
+      argCount);
+}
+
+jsi::Value JavaTurboModule::invokeJavaMethod(
+    jsi::Runtime& runtime,
+    TurboModuleMethodValueKind valueKind,
+    const char* methodName,
+    const char* methodSignature,
+    const jsi::Value* args,
+    size_t argCount) {
   const char* moduleName = name_.c_str();
 
   bool isMethodSync = isSyncMethod(valueKind);
@@ -679,17 +702,21 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
     }
   };
 
-  if (methodID == nullptr) {
-    jclass cls = env->GetObjectClass(instance);
-    methodID = env->GetMethodID(cls, methodName, methodSignature.c_str());
-
+  const MethodInfo* methodInfo =
+      getMethodInfo(env, methodName, methodSignature);
+  if (methodInfo == nullptr) {
     // If the method signature doesn't match, show a redbox here instead of
     // crashing later.
     checkJNIErrorForMethodCall();
+    throw jsi::JSError(
+        runtime,
+        "Unable to find method " + std::string(methodName) + "(" +
+            methodSignature + ") on " + name_);
   }
+  jmethodID methodID = methodInfo->methodID;
 
   // TODO(T43933641): Refactor to remove this special-casing
-  if (methodNameStr == "getConstants") {
+  if (std::strcmp(methodName, "getConstants") == 0) {
     TMPL::syncMethodCallArgConversionEnd(moduleName, methodName);
     TMPL::syncMethodCallExecutionStart(moduleName, methodName);
 
@@ -708,13 +735,11 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
     return returnValue;
   }
 
-  const MethodInfo& methodInfo = getMethodInfo(methodID, methodSignature);
-
   JNIArgs jniArgs = convertJSIArgsToJNIArgs(
       env,
       runtime,
-      methodNameStr,
-      methodInfo.argTypes,
+      methodName,
+      methodInfo->argTypes,
       args,
       argCount,
       jsInvoker_,
@@ -730,7 +755,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
 
   switch (valueKind) {
     case BooleanKind: {
-      const std::string& returnType = methodInfo.returnType;
+      const std::string& returnType = methodInfo->returnType;
       if (returnType == "Ljava/lang/Boolean;") {
         auto returnObject =
             env->CallObjectMethodA(instance, methodID, jargs.data());
@@ -766,7 +791,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
       }
     }
     case NumberKind: {
-      const std::string& returnType = methodInfo.returnType;
+      const std::string& returnType = methodInfo->returnType;
       if (returnType == "Ljava/lang/Double;" ||
           returnType == "Ljava/lang/Float;" ||
           returnType == "Ljava/lang/Integer;") {
@@ -907,7 +932,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
            methodID,
            instance_ = jni::make_weak(instance_),
            moduleNameStr = name_,
-           methodNameStr,
+           methodNameStr = std::string(methodName),
            id = getUniqueId()]() mutable {
             TraceSection s(
                 "JavaTurboModuleAsyncMethodInvocation",
@@ -1005,8 +1030,6 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
                 .utf8(runtime);
       }
 
-      const char* moduleName = name_.c_str();
-      const char* methodName = methodNameStr.c_str();
       TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName);
       TMPL::asyncMethodCallDispatch(moduleName, methodName);
       nativeMethodCallInvoker_->invokeAsync(
@@ -1017,7 +1040,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
            methodID,
            instance_ = jni::make_weak(instance_),
            moduleNameStr = name_,
-           methodNameStr,
+           methodNameStr = std::string(methodName),
            id = getUniqueId()]() mutable {
             TraceSection s(
                 "JavaTurboModuleAsyncMethodInvocation",
@@ -1073,7 +1096,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
         if (!returnRef->isInstanceOf(JArrayBuffer::javaClassStatic())) {
           throw jsi::JSError(
               runtime,
-              "JavaTurboModule::invokeJavaMethod: expected " + methodNameStr +
+              "JavaTurboModule::invokeJavaMethod: expected " +
+                  std::string(methodName) +
                   " to return a com.facebook.react.bridge.ArrayBuffer.");
         }
 
@@ -1091,7 +1115,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
     }
     default:
       throw std::runtime_error(
-          "Unable to find method module: " + methodNameStr + "(" +
+          "Unable to find method module: " + std::string(methodName) + "(" +
           methodSignature + ")");
   }
 }

packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h

@@ -9,8 +9,10 @@
 
 #include <react/cxxstableapi/UmbrellaGuard.h>
 
+#include <functional>
 #include <mutex>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <vector>
 
@@ -43,6 +45,19 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
   JavaTurboModule(const InitParams &params);
   virtual ~JavaTurboModule();
 
+  // Looks the method up on its first call and caches it by name.
+  jsi::Value invokeJavaMethod(
+      jsi::Runtime &runtime,
+      TurboModuleMethodValueKind valueKind,
+      const char *methodName,
+      const char *methodSignature,
+      const jsi::Value *args,
+      size_t argCount);
+
+  // Kept for code generated by earlier versions of codegen and for modules
+  // that call it directly. cachedMethodID is neither read nor written: the
+  // method is cached by name.
+  [[deprecated("Use the overload without cachedMethodID")]]
   jsi::Value invokeJavaMethod(
       jsi::Runtime &runtime,
       TurboModuleMethodValueKind valueKind,
@@ -65,14 +80,25 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
   jni::global_ref<jobject> instance_;
   std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker_;
 
-  // A method's JNI signature, parsed on its first call since it doesn't change.
+  // A method's ID and parsed JNI signature, looked up on its first call since they don't change.
   struct MethodInfo {
+    jmethodID methodID;
     std::vector<std::string> argTypes;
     std::string returnType;
   };
-  std::unordered_map<jmethodID, MethodInfo> methodInfos_;
+  struct MethodNameHash {
+    using is_transparent = void;
+    size_t operator()(std::string_view methodName) const
+    {
+      return std::hash<std::string_view>{}(methodName);
+    }
+  };
+  // Keyed by the method's name, which is unique in a TurboModule; looked up
+  // without allocating a std::string.
+  std::unordered_map<std::string, MethodInfo, MethodNameHash, std::equal_to<>> methodInfos_;
   std::mutex methodInfosMutex_;
-  const MethodInfo &getMethodInfo(jmethodID methodID, const std::string &methodSignature);
+  // Returns nullptr, with a pending Java exception, if the method doesn't exist.
+  const MethodInfo *getMethodInfo(JNIEnv *env, const char *methodName, const char *methodSignature);
 };
 
 } // namespace facebook::react

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

@@ -1694,6 +1694,7 @@ class facebook::react::JavaTurboModule : public facebook::react::TurboModule {
   protected void configureEventEmitterCallback();
   protected void setEventEmitterCallback(jni::alias_ref<jobject>);
   public JavaTurboModule(const facebook::react::JavaTurboModule::InitParams& params);
+  public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const char* methodName, const char* methodSignature, const facebook::jsi::Value* args, size_t argCount);
   public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const std::string& methodName, const std::string& methodSignature, const facebook::jsi::Value* args, size_t argCount, jmethodID& cachedMethodID);
   public virtual ~JavaTurboModule();
 }

scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api

@@ -2499,6 +2499,7 @@ class facebook::react::JavaTurboModule : public facebook::react::TurboModule {
   protected void configureEventEmitterCallback();
   protected void setEventEmitterCallback(jni::alias_ref<jobject>);
   public JavaTurboModule(const facebook::react::JavaTurboModule::InitParams& params);
+  public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const char* methodName, const char* methodSignature, const facebook::jsi::Value* args, size_t argCount);
   public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const std::string& methodName, const std::string& methodSignature, const facebook::jsi::Value* args, size_t argCount, jmethodID& cachedMethodID);
   public virtual ~JavaTurboModule();
 }

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

@@ -1693,6 +1693,7 @@ class facebook::react::JavaTurboModule : public facebook::react::TurboModule {
   protected void configureEventEmitterCallback();
   protected void setEventEmitterCallback(jni::alias_ref<jobject>);
   public JavaTurboModule(const facebook::react::JavaTurboModule::InitParams& params);
+  public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const char* methodName, const char* methodSignature, const facebook::jsi::Value* args, size_t argCount);
   public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const std::string& methodName, const std::string& methodSignature, const facebook::jsi::Value* args, size_t argCount, jmethodID& cachedMethodID);
   public virtual ~JavaTurboModule();
 }

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

@@ -1693,6 +1693,7 @@ class facebook::react::JavaTurboModule : public facebook::react::TurboModule {
   protected void configureEventEmitterCallback();
   protected void setEventEmitterCallback(jni::alias_ref<jobject>);
   public JavaTurboModule(const facebook::react::JavaTurboModule::InitParams& params);
+  public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const char* methodName, const char* methodSignature, const facebook::jsi::Value* args, size_t argCount);
   public facebook::jsi::Value invokeJavaMethod(facebook::jsi::Runtime& runtime, facebook::react::TurboModuleMethodValueKind valueKind, const std::string& methodName, const std::string& methodSignature, const facebook::jsi::Value* args, size_t argCount, jmethodID& cachedMethodID);
   public virtual ~JavaTurboModule();
 }