Skip to content

Commit 281daf1

Browse files
RSNarafacebook-github-bot
authored andcommittedJun 2, 2021
Ship bridge RuntimeExecutor JSIExecutor flushing
Summary: The RuntimeExecutor that Fabric gets from the bridge doesn't call JSIExecutor::flush(). In the legacy NativeModule system, we're supposed to flush the queue of NativeModule calls after every call into JavaScript. The lack of this flushing means that we execute NativeModule calls less frequently with Fabric enabled, and TurboModules disabled. It also means that [the microtask checkpoints we placed inside JSIExecutor::flush()](https://www.internalfb.com/code/fbsource/[62f69606ae81530f7d6f0cba8466ac604934c901]/xplat/js/react-native-github/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp?lines=427%2C445) won't be executed as frequently, with Fabric enabled. Changelog: [Android][Fixed] - Flush NativeModule calls with Fabric on Android, on every Native -> JS call. Reviewed By: JoshuaGross, mdvacca Differential Revision: D28620982 fbshipit-source-id: ae4d1c16c62b6d4a5089e63104ad97f4ed44c440
1 parent 502b819 commit 281daf1

File tree

6 files changed

+11
-41
lines changed

6 files changed

+11
-41
lines changed
 

‎ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

+1-6
Original file line numberDiff line numberDiff line change
@@ -549,12 +549,7 @@ public JavaScriptContextHolder getJavaScriptContextHolder() {
549549
return mJavaScriptContextHolder;
550550
}
551551

552-
@Override
553-
public RuntimeExecutor getRuntimeExecutor() {
554-
return getRuntimeExecutor(ReactFeatureFlags.enableRuntimeExecutorFlushing());
555-
}
556-
557-
public native RuntimeExecutor getRuntimeExecutor(boolean shouldFlush);
552+
public native RuntimeExecutor getRuntimeExecutor();
558553

559554
@Override
560555
public void addJSIModules(List<JSIModuleSpec> jsiModules) {

‎ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java

-21
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818
*/
1919
@DoNotStripAny
2020
public class ReactFeatureFlags {
21-
22-
/** An interface used to compute flags on demand. */
23-
public interface FlagProvider {
24-
boolean get();
25-
}
26-
2721
/**
2822
* Should this application use TurboModules? If yes, then any module that inherits {@link
2923
* com.facebook.react.turbomodule.core.interfaces.TurboModule} will NOT be passed in to C++
@@ -59,13 +53,6 @@ public interface FlagProvider {
5953
/** Feature flag to configure eager initialization of MapBuffer So file */
6054
public static boolean enableEagerInitializeMapBufferSoFile = false;
6155

62-
/** Should the RuntimeExecutor call JSIExecutor::flush()? */
63-
private static FlagProvider enableRuntimeExecutorFlushingProvider = null;
64-
65-
public static void setEnableRuntimeExecutorFlushingFlagProvider(FlagProvider provider) {
66-
enableRuntimeExecutorFlushingProvider = provider;
67-
}
68-
6956
private static boolean mapBufferSerializationEnabled = false;
7057

7158
/** Enables or disables MapBuffer Serialization */
@@ -77,14 +64,6 @@ public static boolean isMapBufferSerializationEnabled() {
7764
return mapBufferSerializationEnabled;
7865
}
7966

80-
public static boolean enableRuntimeExecutorFlushing() {
81-
if (enableRuntimeExecutorFlushingProvider != null) {
82-
return enableRuntimeExecutorFlushingProvider.get();
83-
}
84-
85-
return false;
86-
}
87-
8867
/** Enables Fabric for LogBox */
8968
public static boolean enableFabricInLogBox = false;
9069

‎ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,10 @@ CatalystInstanceImpl::getNativeCallInvokerHolder() {
346346
}
347347

348348
jni::alias_ref<JRuntimeExecutor::javaobject>
349-
CatalystInstanceImpl::getRuntimeExecutor(bool shouldFlush) {
349+
CatalystInstanceImpl::getRuntimeExecutor() {
350350
if (!runtimeExecutor_) {
351-
runtimeExecutor_ = jni::make_global(JRuntimeExecutor::newObjectCxxArgs(
352-
instance_->getRuntimeExecutor(shouldFlush)));
351+
runtimeExecutor_ = jni::make_global(
352+
JRuntimeExecutor::newObjectCxxArgs(instance_->getRuntimeExecutor()));
353353
}
354354
return runtimeExecutor_;
355355
}

‎ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
9393
void jniCallJSCallback(jint callbackId, NativeArray *arguments);
9494
jni::alias_ref<CallInvokerHolder::javaobject> getJSCallInvokerHolder();
9595
jni::alias_ref<CallInvokerHolder::javaobject> getNativeCallInvokerHolder();
96-
jni::alias_ref<JRuntimeExecutor::javaobject> getRuntimeExecutor(
97-
bool shouldFlush);
96+
jni::alias_ref<JRuntimeExecutor::javaobject> getRuntimeExecutor();
9897
void setGlobalVariable(std::string propName, std::string &&jsonValue);
9998
jlong getJavaScriptContext();
10099
void handleMemoryPressure(int pressureLevel);

‎ReactCommon/cxxreact/Instance.cpp

+5-8
Original file line numberDiff line numberDiff line change
@@ -246,23 +246,20 @@ std::shared_ptr<CallInvoker> Instance::getJSCallInvoker() {
246246
return std::static_pointer_cast<CallInvoker>(jsCallInvoker_);
247247
}
248248

249-
RuntimeExecutor Instance::getRuntimeExecutor(bool shouldFlush) {
249+
RuntimeExecutor Instance::getRuntimeExecutor() {
250250
std::weak_ptr<NativeToJsBridge> weakNativeToJsBridge = nativeToJsBridge_;
251251

252252
auto runtimeExecutor =
253-
[weakNativeToJsBridge,
254-
shouldFlush](std::function<void(jsi::Runtime & runtime)> &&callback) {
253+
[weakNativeToJsBridge](
254+
std::function<void(jsi::Runtime & runtime)> &&callback) {
255255
if (auto strongNativeToJsBridge = weakNativeToJsBridge.lock()) {
256256
strongNativeToJsBridge->runOnExecutorQueue(
257-
[callback = std::move(callback),
258-
shouldFlush](JSExecutor *executor) {
257+
[callback = std::move(callback)](JSExecutor *executor) {
259258
jsi::Runtime *runtime =
260259
(jsi::Runtime *)executor->getJavaScriptContext();
261260
try {
262261
callback(*runtime);
263-
if (shouldFlush) {
264-
executor->flush();
265-
}
262+
executor->flush();
266263
} catch (jsi::JSError &originalError) {
267264
handleJSError(*runtime, originalError, true);
268265
}

‎ReactCommon/cxxreact/Instance.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class RN_EXPORT Instance {
134134
/**
135135
* RuntimeExecutor is used by Fabric to access the jsi::Runtime.
136136
*/
137-
RuntimeExecutor getRuntimeExecutor(bool shouldFlush);
137+
RuntimeExecutor getRuntimeExecutor();
138138

139139
private:
140140
void callNativeModules(folly::dynamic &&calls, bool isEndOfBatch);

0 commit comments

Comments
 (0)
Please sign in to comment.