From c0a2998387d1730d38a28cbe296520400a645b25 Mon Sep 17 00:00:00 2001 From: Emily Janzer Date: Fri, 20 Nov 2020 14:31:03 -0800 Subject: [PATCH] Move TurboModuleManager to use RuntimeExecutor instead of jsContext (#30416) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/30416 This diff changes the constructor param for TurboModuleManager from jsContext (a long representing the `jsi::Runtime` pointer) to a RuntimeExecutor. It also updates callsites to use the new RuntimeExecutor created by CatalystInstance. This is only used for installing the TurboModule JSI binding; it's not currently used for JS invocation in TurboModules, which is handled separately by JSCallInvoker. Ultimately we may be able to implement JSCallInvoker *with* the provided RuntimeExecutor, but there's some additional logic in JSCallInvoker that we don't have here yet. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D21338930 fbshipit-source-id: 1480c328f1a1776ddf22752510c0f3b35168a489 --- .../turbomodule/core/TurboModuleManager.java | 8 +- .../react/turbomodule/core/jni/Android.mk | 4 +- .../facebook/react/turbomodule/core/jni/BUCK | 1 + .../jni/ReactCommon/TurboModuleManager.cpp | 158 +++++++++--------- .../core/jni/ReactCommon/TurboModuleManager.h | 8 +- .../react/uiapp/RNTesterApplication.java | 2 +- 6 files changed, 92 insertions(+), 89 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java index 8cd4c670370..000770aec1d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java @@ -15,8 +15,8 @@ import com.facebook.jni.HybridData; import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.bridge.CxxModuleWrapper; import com.facebook.react.bridge.JSIModule; -import com.facebook.react.bridge.JavaScriptContextHolder; import com.facebook.react.bridge.NativeModule; +import com.facebook.react.bridge.RuntimeExecutor; import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder; import com.facebook.react.turbomodule.core.interfaces.TurboModule; @@ -50,14 +50,14 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { private final HybridData mHybridData; public TurboModuleManager( - JavaScriptContextHolder jsContext, + RuntimeExecutor runtimeExecutor, @Nullable final TurboModuleManagerDelegate delegate, CallInvokerHolder jsCallInvokerHolder, CallInvokerHolder nativeCallInvokerHolder) { maybeLoadSoLibrary(); mHybridData = initHybrid( - jsContext.get(), + runtimeExecutor, (CallInvokerHolderImpl) jsCallInvokerHolder, (CallInvokerHolderImpl) nativeCallInvokerHolder, delegate, @@ -291,7 +291,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { } private native HybridData initHybrid( - long jsContext, + RuntimeExecutor runtimeExecutor, CallInvokerHolderImpl jsCallInvokerHolder, CallInvokerHolderImpl nativeCallInvokerHolder, TurboModuleManagerDelegate tmmDelegate, diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/Android.mk b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/Android.mk index 955cc86a8b1..401429a13ac 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/Android.mk +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/Android.mk @@ -19,9 +19,9 @@ LOCAL_EXPORT_C_INCLUDES := $(LOCAL_PATH) LOCAL_CFLAGS += -fexceptions -frtti -std=c++14 -Wall -LOCAL_SHARED_LIBRARIES = libfb libfbjni +LOCAL_SHARED_LIBRARIES = libfb libfbjni libreactnativeutilsjni -LOCAL_STATIC_LIBRARIES = libcallinvoker libreactperfloggerjni +LOCAL_STATIC_LIBRARIES = libcallinvoker libreactperfloggerjni libruntimeexecutor # Name of this module. LOCAL_MODULE := callinvokerholder diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/BUCK b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/BUCK index b0db877eea6..c7f3ec528b1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/BUCK @@ -35,6 +35,7 @@ rn_xplat_cxx_library( ":callinvokerholder", react_native_xplat_shared_library_target("jsi:jsi"), react_native_xplat_target("react/nativemodule/core:core"), + react_native_xplat_target("runtimeexecutor:runtimeexecutor"), react_native_target("java/com/facebook/react/reactperflogger/jni:jni"), ], ) diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp index 81c17fabb95..a6d90b615d8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp @@ -24,13 +24,13 @@ namespace react { TurboModuleManager::TurboModuleManager( jni::alias_ref jThis, - jsi::Runtime *rt, + RuntimeExecutor runtimeExecutor, std::shared_ptr jsCallInvoker, std::shared_ptr nativeCallInvoker, jni::alias_ref delegate, bool enableJSCodegen) : javaPart_(jni::make_global(jThis)), - runtime_(rt), + runtimeExecutor_(runtimeExecutor), jsCallInvoker_(jsCallInvoker), nativeCallInvoker_(nativeCallInvoker), delegate_(jni::make_global(delegate)), @@ -39,7 +39,7 @@ TurboModuleManager::TurboModuleManager( jni::local_ref TurboModuleManager::initHybrid( jni::alias_ref jThis, - jlong jsContext, + jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, jni::alias_ref delegate, @@ -52,7 +52,7 @@ jni::local_ref TurboModuleManager::initHybrid( return makeCxxInstance( jThis, - (jsi::Runtime *)jsContext, + runtimeExecutor->cthis()->get(), jsCallInvoker, nativeCallInvoker, delegate, @@ -68,100 +68,100 @@ void TurboModuleManager::registerNatives() { } void TurboModuleManager::installJSIBindings() { - if (!runtime_ || !jsCallInvoker_) { + if (!jsCallInvoker_) { return; // Runtime doesn't exist when attached to Chrome debugger. } - auto turboModuleProvider = - [turboModuleCache_ = std::weak_ptr(turboModuleCache_), - jsCallInvoker_ = std::weak_ptr(jsCallInvoker_), - nativeCallInvoker_ = std::weak_ptr(nativeCallInvoker_), - delegate_ = jni::make_weak(delegate_), - javaPart_ = jni::make_weak(javaPart_), - runtime_ = runtime_]( - const std::string &name, - const jsi::Value *schema) -> std::shared_ptr { - auto turboModuleCache = turboModuleCache_.lock(); - auto jsCallInvoker = jsCallInvoker_.lock(); - auto nativeCallInvoker = nativeCallInvoker_.lock(); - auto delegate = delegate_.lockLocal(); - auto javaPart = javaPart_.lockLocal(); + runtimeExecutor_([this](jsi::Runtime &runtime) { + auto turboModuleProvider = + [turboModuleCache_ = std::weak_ptr(turboModuleCache_), + jsCallInvoker_ = std::weak_ptr(jsCallInvoker_), + nativeCallInvoker_ = std::weak_ptr(nativeCallInvoker_), + delegate_ = jni::make_weak(delegate_), + javaPart_ = jni::make_weak(javaPart_), + &runtime]( + const std::string &name, + const jsi::Value *schema) -> std::shared_ptr { + auto turboModuleCache = turboModuleCache_.lock(); + auto jsCallInvoker = jsCallInvoker_.lock(); + auto nativeCallInvoker = nativeCallInvoker_.lock(); + auto delegate = delegate_.lockLocal(); + auto javaPart = javaPart_.lockLocal(); - if (!turboModuleCache || !jsCallInvoker || !nativeCallInvoker || - !delegate || !javaPart) { - return nullptr; - } + if (!turboModuleCache || !jsCallInvoker || !nativeCallInvoker || + !delegate || !javaPart) { + return nullptr; + } - const char *moduleName = name.c_str(); + const char *moduleName = name.c_str(); - TurboModulePerfLogger::moduleJSRequireBeginningStart(moduleName); + TurboModulePerfLogger::moduleJSRequireBeginningStart(moduleName); + + auto turboModuleLookup = turboModuleCache->find(name); + if (turboModuleLookup != turboModuleCache->end()) { + TurboModulePerfLogger::moduleJSRequireBeginningCacheHit(moduleName); + TurboModulePerfLogger::moduleJSRequireBeginningEnd(moduleName); + return turboModuleLookup->second; + } - auto turboModuleLookup = turboModuleCache->find(name); - if (turboModuleLookup != turboModuleCache->end()) { - TurboModulePerfLogger::moduleJSRequireBeginningCacheHit(moduleName); TurboModulePerfLogger::moduleJSRequireBeginningEnd(moduleName); - return turboModuleLookup->second; - } - TurboModulePerfLogger::moduleJSRequireBeginningEnd(moduleName); + auto cxxModule = delegate->cthis()->getTurboModule(name, jsCallInvoker); + if (cxxModule) { + turboModuleCache->insert({name, cxxModule}); + return cxxModule; + } - auto cxxModule = delegate->cthis()->getTurboModule(name, jsCallInvoker); - if (cxxModule) { - turboModuleCache->insert({name, cxxModule}); - return cxxModule; - } + static auto getLegacyCxxModule = + javaPart->getClass() + ->getMethod( + const std::string &)>("getLegacyCxxModule"); + auto legacyCxxModule = getLegacyCxxModule(javaPart.get(), name); - static auto getLegacyCxxModule = - javaPart->getClass() - ->getMethod( - const std::string &)>("getLegacyCxxModule"); - auto legacyCxxModule = getLegacyCxxModule(javaPart.get(), name); + if (legacyCxxModule) { + TurboModulePerfLogger::moduleJSRequireEndingStart(moduleName); - if (legacyCxxModule) { - TurboModulePerfLogger::moduleJSRequireEndingStart(moduleName); + auto turboModule = std::make_shared( + legacyCxxModule->cthis()->getModule(), jsCallInvoker); + turboModuleCache->insert({name, turboModule}); - auto turboModule = std::make_shared( - legacyCxxModule->cthis()->getModule(), jsCallInvoker); - turboModuleCache->insert({name, turboModule}); - - TurboModulePerfLogger::moduleJSRequireEndingEnd(moduleName); - return turboModule; - } - - static auto getJavaModule = - javaPart->getClass() - ->getMethod(const std::string &)>( - "getJavaModule"); - auto moduleInstance = getJavaModule(javaPart.get(), name); - - if (moduleInstance) { - TurboModulePerfLogger::moduleJSRequireEndingStart(moduleName); - JavaTurboModule::InitParams params = {.moduleName = name, - .instance = moduleInstance, - .jsInvoker = jsCallInvoker, - .nativeInvoker = nativeCallInvoker}; - - if (schema->isObject() && !schema->isNull()) { - auto turboModule = std::make_shared( - params, TurboModuleSchema::parse(*runtime_, name, *schema)); TurboModulePerfLogger::moduleJSRequireEndingEnd(moduleName); return turboModule; } - auto turboModule = delegate->cthis()->getTurboModule(name, params); - turboModuleCache->insert({name, turboModule}); - TurboModulePerfLogger::moduleJSRequireEndingEnd(moduleName); - return turboModule; - } + static auto getJavaModule = + javaPart->getClass() + ->getMethod(const std::string &)>( + "getJavaModule"); + auto moduleInstance = getJavaModule(javaPart.get(), name); - return nullptr; - }; + if (moduleInstance) { + TurboModulePerfLogger::moduleJSRequireEndingStart(moduleName); + JavaTurboModule::InitParams params = { + .moduleName = name, + .instance = moduleInstance, + .jsInvoker = jsCallInvoker, + .nativeInvoker = nativeCallInvoker}; - jsCallInvoker_->invokeAsync( - [this, turboModuleProvider = std::move(turboModuleProvider)]() -> void { - TurboModuleBinding::install( - *runtime_, std::move(turboModuleProvider), enableJSCodegen_); - }); + if (schema->isObject() && !schema->isNull()) { + auto turboModule = std::make_shared( + params, TurboModuleSchema::parse(runtime, name, *schema)); + TurboModulePerfLogger::moduleJSRequireEndingEnd(moduleName); + return turboModule; + } + + auto turboModule = delegate->cthis()->getTurboModule(name, params); + turboModuleCache->insert({name, turboModule}); + TurboModulePerfLogger::moduleJSRequireEndingEnd(moduleName); + return turboModule; + } + + return nullptr; + }; + + TurboModuleBinding::install( + runtime, std::move(turboModuleProvider), enableJSCodegen_); + }); } } // namespace react diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h index 2a6a95aed2d..92716800d24 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h @@ -9,12 +9,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include @@ -27,7 +29,7 @@ class TurboModuleManager : public jni::HybridClass { "Lcom/facebook/react/turbomodule/core/TurboModuleManager;"; static jni::local_ref initHybrid( jni::alias_ref jThis, - jlong jsContext, + jni::alias_ref runtimeExecutor, jni::alias_ref jsCallInvokerHolder, jni::alias_ref nativeCallInvokerHolder, jni::alias_ref delegate, @@ -38,7 +40,7 @@ class TurboModuleManager : public jni::HybridClass { private: friend HybridBase; jni::global_ref javaPart_; - jsi::Runtime *runtime_; + RuntimeExecutor runtimeExecutor_; std::shared_ptr jsCallInvoker_; std::shared_ptr nativeCallInvoker_; jni::global_ref delegate_; @@ -58,7 +60,7 @@ class TurboModuleManager : public jni::HybridClass { void installJSIBindings(); explicit TurboModuleManager( jni::alias_ref jThis, - jsi::Runtime *rt, + RuntimeExecutor runtimeExecutor, std::shared_ptr jsCallInvoker, std::shared_ptr nativeCallInvoker, jni::alias_ref delegate, diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index 0c2e08f91c3..14700729103 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -140,7 +140,7 @@ public class RNTesterApplication extends Application implements ReactApplication final List packages = reactInstanceManager.getPackages(); return new TurboModuleManager( - jsContext, + reactApplicationContext.getCatalystInstance().getRuntimeExecutor(), new RNTesterTurboModuleManagerDelegate( reactApplicationContext, packages), reactApplicationContext