From ea3e2446686c8fca4268ff62f33f2797fa51589c Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 28 Sep 2021 09:21:55 -0700 Subject: [PATCH] Add option to use RuntimeScheduler in TurboModules Summary: changelog: [internal] Reviewed By: RSNara Differential Revision: D31145372 fbshipit-source-id: b1d9473d5006d055d1116f71f65899293fb85c56 --- .../facebook/react/ReactInstanceManager.java | 4 -- .../react/bridge/CatalystInstance.java | 2 - .../react/bridge/CatalystInstanceImpl.java | 15 +++++-- .../react/config/ReactFeatureFlags.java | 2 + .../jni/react/jni/CatalystInstanceImpl.cpp | 43 ++++++++++++------- .../main/jni/react/jni/CatalystInstanceImpl.h | 15 +++++-- .../RuntimeSchedulerCallInvoker.cpp | 6 ++- .../RuntimeSchedulerCallInvoker.h | 6 ++- 8 files changed, 61 insertions(+), 32 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 5ca75bdab9c..559ebb01a23 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -1368,10 +1368,6 @@ public class ReactInstanceManager { } } - if (ReactFeatureFlags.enableRuntimeScheduler) { - catalystInstance.installRuntimeScheduler(); - } - if (mJSIModulePackage != null) { catalystInstance.addJSIModules( mJSIModulePackage.getJSIModules( diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java index 09eb2c70183..ddd1bee4f02 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -113,8 +113,6 @@ public interface CatalystInstance RuntimeScheduler getRuntimeScheduler(); - void installRuntimeScheduler(); - void addJSIModules(List jsiModules); /** diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index d5a9b97571b..74992ad17ab 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -108,7 +108,8 @@ public class CatalystInstanceImpl implements CatalystInstance { // C++ parts private final HybridData mHybridData; - private static native HybridData initHybrid(); + private static native HybridData initHybrid( + boolean enableRuntimeScheduler, boolean enableRuntimeSchedulerInTurboModule); public native CallInvokerHolderImpl getJSCallInvokerHolder(); @@ -123,7 +124,15 @@ public class CatalystInstanceImpl implements CatalystInstance { FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge."); Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstanceImpl"); - mHybridData = initHybrid(); + if (ReactFeatureFlags.enableRuntimeSchedulerInTurboModule + && !ReactFeatureFlags.enableRuntimeScheduler) { + Assertions.assertUnreachable(); + } + + mHybridData = + initHybrid( + ReactFeatureFlags.enableRuntimeScheduler, + ReactFeatureFlags.enableRuntimeSchedulerInTurboModule); mReactQueueConfiguration = ReactQueueConfigurationImpl.create( @@ -560,8 +569,6 @@ public class CatalystInstanceImpl implements CatalystInstance { public native RuntimeScheduler getRuntimeScheduler(); - public native void installRuntimeScheduler(); - @Override public void addJSIModules(List jsiModules) { mJSIModuleRegistry.registerModules(jsiModules); diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 2ecfdaf7639..bb625143c26 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -72,6 +72,8 @@ public class ReactFeatureFlags { public static boolean enableRuntimeScheduler = false; + public static boolean enableRuntimeSchedulerInTurboModule = false; + /** Enables a more aggressive cleanup during destruction of ReactContext */ public static boolean enableReactContextCleanupFix = false; diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index e5b36b30dd1..00d947649af 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -92,12 +93,21 @@ class JInstanceCallback : public InstanceCallback { } // namespace jni::local_ref -CatalystInstanceImpl::initHybrid(jni::alias_ref) { - return makeCxxInstance(); +CatalystInstanceImpl::initHybrid( + jni::alias_ref, + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule) { + return makeCxxInstance( + enableRuntimeScheduler, enableRuntimeSchedulerInTurboModule); } -CatalystInstanceImpl::CatalystInstanceImpl() - : instance_(std::make_unique()) {} +CatalystInstanceImpl::CatalystInstanceImpl( + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule) + : instance_(std::make_unique()), + enableRuntimeScheduler_(enableRuntimeScheduler), + enableRuntimeSchedulerInTurboModule_( + enableRuntimeScheduler && enableRuntimeSchedulerInTurboModule) {} void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() { CxxNativeModule::setShouldWarnOnUse(true); @@ -140,9 +150,6 @@ void CatalystInstanceImpl::registerNatives() { "getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor), makeNativeMethod( "getRuntimeScheduler", CatalystInstanceImpl::getRuntimeScheduler), - makeNativeMethod( - "installRuntimeScheduler", - CatalystInstanceImpl::installRuntimeScheduler), makeNativeMethod( "warnOnLegacyNativeModuleSystemUse", CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse), @@ -347,10 +354,18 @@ void CatalystInstanceImpl::handleMemoryPressure(int pressureLevel) { jni::alias_ref CatalystInstanceImpl::getJSCallInvokerHolder() { if (!jsCallInvokerHolder_) { - jsCallInvokerHolder_ = jni::make_global( - CallInvokerHolder::newObjectCxxArgs(instance_->getJSCallInvoker())); + if (enableRuntimeSchedulerInTurboModule_) { + auto runtimeScheduler = getRuntimeScheduler(); + auto runtimeSchedulerCallInvoker = + std::make_shared( + runtimeScheduler->cthis()->get()); + jsCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(runtimeSchedulerCallInvoker)); + } else { + jsCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(instance_->getJSCallInvoker())); + } } - return jsCallInvokerHolder_; } @@ -397,11 +412,7 @@ CatalystInstanceImpl::getRuntimeExecutor() { jni::alias_ref CatalystInstanceImpl::getRuntimeScheduler() { - return runtimeScheduler_; -} - -void CatalystInstanceImpl::installRuntimeScheduler() { - if (!runtimeScheduler_) { + if (enableRuntimeScheduler_ && !runtimeScheduler_) { auto runtimeExecutor = instance_->getRuntimeExecutor(); auto runtimeScheduler = std::make_shared(runtimeExecutor); @@ -413,6 +424,8 @@ void CatalystInstanceImpl::installRuntimeScheduler() { runtime, runtimeScheduler); }); } + + return runtimeScheduler_; } } // namespace react diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h index 66daec3e34b..219669714df 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h @@ -37,7 +37,10 @@ class CatalystInstanceImpl : public jni::HybridClass { static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/CatalystInstanceImpl;"; - static jni::local_ref initHybrid(jni::alias_ref); + static jni::local_ref initHybrid( + jni::alias_ref, + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule); static void registerNatives(); @@ -48,7 +51,9 @@ class CatalystInstanceImpl : public jni::HybridClass { private: friend HybridBase; - CatalystInstanceImpl(); + CatalystInstanceImpl( + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule); void initializeBridge( jni::alias_ref callback, @@ -100,11 +105,12 @@ class CatalystInstanceImpl : public jni::HybridClass { jni::alias_ref getNativeCallInvokerHolder(); jni::alias_ref getRuntimeExecutor(); jni::alias_ref getRuntimeScheduler(); - void installRuntimeScheduler(); void setGlobalVariable(std::string propName, std::string &&jsonValue); jlong getJavaScriptContext(); void handleMemoryPressure(int pressureLevel); + void createAndInstallRuntimeSchedulerIfNecessary(); + // This should be the only long-lived strong reference, but every C++ class // will have a weak reference. std::shared_ptr instance_; @@ -114,6 +120,9 @@ class CatalystInstanceImpl : public jni::HybridClass { jni::global_ref nativeCallInvokerHolder_; jni::global_ref runtimeExecutor_; jni::global_ref runtimeScheduler_; + + bool const enableRuntimeScheduler_; + bool const enableRuntimeSchedulerInTurboModule_; }; } // namespace react diff --git a/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp b/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp index 42922e4be40..70f02ca132e 100644 --- a/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp @@ -7,7 +7,8 @@ #include "RuntimeSchedulerCallInvoker.h" -namespace facebook::react { +namespace facebook { +namespace react { RuntimeSchedulerCallInvoker::RuntimeSchedulerCallInvoker( std::weak_ptr runtimeScheduler) @@ -27,4 +28,5 @@ void RuntimeSchedulerCallInvoker::invokeSync(std::function &&func) { } } -} // namespace facebook::react +} // namespace react +} // namespace facebook diff --git a/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.h b/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.h index 35206ccd505..b40e0cad727 100644 --- a/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.h +++ b/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.h @@ -10,7 +10,8 @@ #include #include -namespace facebook::react { +namespace facebook { +namespace react { /* * Exposes RuntimeScheduler to native modules. All calls invonked on JavaScript @@ -31,4 +32,5 @@ class RuntimeSchedulerCallInvoker : public CallInvoker { std::weak_ptr runtimeScheduler_; }; -} // namespace facebook::react +} // namespace react +} // namespace facebook