From 9b94a541d818edd501b994d5f1bc67c0c81d6ff2 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 1 Apr 2020 11:36:50 -0700 Subject: [PATCH] Get CallInvokers from the bridge Summary: ## Context For now, assume TurboModules doesn't exist. **What happens when we call an async NativeModule method?** Everytime JS calls an async NativeModule method, we don't immediately execute it. The legacy infra pushes the call into some queue managed by `MessageQueue.js`. This queue is "flushed" or "emptied" by the following events: - **Flushed:** A C++ -> JS call. NativeModule async methods can called with an `onSuccess` and/or `onFail` callback(s). Calling `NativeToJsBridge::invokeCallback` to invoke one of these callbacks is one way for ObjC++/C++/Java to call into JS. Another way is via JSModule method calls, which are initiated by `NativeToJsBridge::callFunction`. - **Flushed:** When `JSIExecutor::flush` is called. Since TurboModules don't exist, this only happens when we call `JSIExecutor::loadApplicationScript`. - **Emptied:** When more than 5 ms have passed, and the queue hasn't been flushed/emptied, on the next async NativeModule method call, we add to the queue. Afterwards, we empty it, and invoke all the NativeModule method calls. **So, what's the difference between flushed and emptied?** > Note: These are two terms I just made up, but the distinction is important. If the queue was "flushed", and it contained at least one NativeModule method call, `JsToNativeBridge` dispatches the `onBatchComplete` event. On Android, the UIManager module is the only module that listens to this event. This `onBatchComplete` event doesn't fire if the queue was "emptied". **Why does any of this matter?** 1. TurboModules exist. 2. We need the TurboModules infra to have `JsToNativeBridge` dispatch `onBatchComplete`, which depends on: - **Problem 1:** The queue being flushed on calls into JS from Java/C++/ObjC++. - **Problem 2:** There being queued up NativeModule async method calls when the queue is flushed. In D14656466, fkgozali fixed Problem 1 by making every C++/Java/Obj -> JS call from TurboModules also execute `JSIExecutor::flush()`. This means that, with TurboModules, we flush the NativeModule async method call queue as often as we do without TurboModules. So far, so good. However, we still have one big problem: As we convert more NativeModules to TurboModules, the average size of the queue of NativeModule method calls will become smaller and smaller, because more NativeModule method calls will be TurboModule method calls. This queue will more often be empty than not. Therefore, we'll end up dispatching the `onBatchComplete` event less often with TurboModules enabled. So, somehow, when we're about to flush the NativeModule method call queue, we need `JsToNativeBridge` to understand that we've executed TurboModule method calls in the batch. These calls would have normally been queued, which would have led the queue size to be non-zero. So if, during a batch, some TurboModule async method calls were executed, `JsToNativeBridge` should dispatch `onBatchComplete`. **So, what does this diff do?** 1. Make `Instance` responsible for creating the JS `CallInvoker`. 2. Make `NativeToJsBridge` responsible for creating the native `CallInvoker`. `Instance` calls into `NativeToJsBridge` to get the native `CallInvoker`. 3. Hook up `CatalystInstanceImpl`, the Android bridge, with the new JS `CallInvoker`, and the new native `CallInvoker`. This fixes `onBatchComplete` on Android. iOS work is pending. Changelog: [Android][Fixed] - Ensure `onBatchComplete` is dispatched correctly with TurboModules Reviewed By: mdvacca Differential Revision: D20717931 fbshipit-source-id: bc3ccbd6c135b7f084edbc6ddb4d1e3c0c7e0875 --- RNTester/Podfile.lock | 72 +++++++++---------- RNTester/RNTester/AppDelegate.mm | 3 +- React/CxxBridge/RCTCxxBridge.mm | 10 +-- .../jni/react/jni/CatalystInstanceImpl.cpp | 15 ++-- .../main/jni/react/jni/CatalystInstanceImpl.h | 1 - ReactCommon/ReactCommon.podspec | 11 +-- ReactCommon/callinvoker/Android.mk | 2 - ReactCommon/callinvoker/BUCK | 5 +- .../callinvoker/React-callinvoker.podspec | 34 +++++++++ .../ReactCommon/BridgeJSCallInvoker.cpp | 26 ------- .../ReactCommon/BridgeJSCallInvoker.h | 42 ----------- .../MessageQueueThreadCallInvoker.cpp | 22 ------ .../MessageQueueThreadCallInvoker.h | 35 --------- ReactCommon/cxxreact/Android.mk | 3 +- ReactCommon/cxxreact/BUCK | 1 + ReactCommon/cxxreact/Instance.cpp | 69 ++++++++++++++++-- ReactCommon/cxxreact/Instance.h | 62 +++++++++++++++- ReactCommon/cxxreact/NativeToJsBridge.cpp | 42 +++++++++-- ReactCommon/cxxreact/NativeToJsBridge.h | 8 +++ ReactCommon/cxxreact/React-cxxreact.podspec | 1 + .../core/platform/ios/RCTTurboModule.h | 12 ++-- scripts/react_native_pods.rb | 2 +- 22 files changed, 266 insertions(+), 212 deletions(-) create mode 100644 ReactCommon/callinvoker/React-callinvoker.podspec delete mode 100644 ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.cpp delete mode 100644 ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.h delete mode 100644 ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.cpp delete mode 100644 ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.h diff --git a/RNTester/Podfile.lock b/RNTester/Podfile.lock index c4e421b421d..6e9dd58e849 100644 --- a/RNTester/Podfile.lock +++ b/RNTester/Podfile.lock @@ -1,6 +1,6 @@ PODS: - boost-for-react-native (1.63.0) - - CocoaAsyncSocket (7.6.3) + - CocoaAsyncSocket (7.6.4) - CocoaLibEvent (1.0.0) - DoubleConversion (1.1.6) - FBLazyVector (1000.0.0) @@ -11,7 +11,7 @@ PODS: - React-Core (= 1000.0.0) - React-jsi (= 1000.0.0) - ReactCommon/turbomodule/core (= 1000.0.0) - - Flipper (0.30.2): + - Flipper (0.33.1): - Flipper-Folly (~> 2.1) - Flipper-RSocket (~> 1.0) - Flipper-DoubleConversion (1.1.7) @@ -25,36 +25,36 @@ PODS: - Flipper-PeerTalk (0.0.4) - Flipper-RSocket (1.0.0): - Flipper-Folly (~> 2.0) - - FlipperKit (0.30.2): - - FlipperKit/Core (= 0.30.2) - - FlipperKit/Core (0.30.2): - - Flipper (~> 0.30.2) + - FlipperKit (0.33.1): + - FlipperKit/Core (= 0.33.1) + - FlipperKit/Core (0.33.1): + - Flipper (~> 0.33.1) - FlipperKit/CppBridge - FlipperKit/FBCxxFollyDynamicConvert - FlipperKit/FBDefines - FlipperKit/FKPortForwarding - - FlipperKit/CppBridge (0.30.2): - - Flipper (~> 0.30.2) - - FlipperKit/FBCxxFollyDynamicConvert (0.30.2): + - FlipperKit/CppBridge (0.33.1): + - Flipper (~> 0.33.1) + - FlipperKit/FBCxxFollyDynamicConvert (0.33.1): - Flipper-Folly (~> 2.1) - - FlipperKit/FBDefines (0.30.2) - - FlipperKit/FKPortForwarding (0.30.2): + - FlipperKit/FBDefines (0.33.1) + - FlipperKit/FKPortForwarding (0.33.1): - CocoaAsyncSocket (~> 7.6) - Flipper-PeerTalk (~> 0.0.4) - - FlipperKit/FlipperKitHighlightOverlay (0.30.2) - - FlipperKit/FlipperKitLayoutPlugin (0.30.2): + - FlipperKit/FlipperKitHighlightOverlay (0.33.1) + - FlipperKit/FlipperKitLayoutPlugin (0.33.1): - FlipperKit/Core - FlipperKit/FlipperKitHighlightOverlay - FlipperKit/FlipperKitLayoutTextSearchable - YogaKit (~> 1.18) - - FlipperKit/FlipperKitLayoutTextSearchable (0.30.2) - - FlipperKit/FlipperKitNetworkPlugin (0.30.2): + - FlipperKit/FlipperKitLayoutTextSearchable (0.33.1) + - FlipperKit/FlipperKitNetworkPlugin (0.33.1): - FlipperKit/Core - - FlipperKit/FlipperKitReactPlugin (0.30.2): + - FlipperKit/FlipperKitReactPlugin (0.33.1): - FlipperKit/Core - - FlipperKit/FlipperKitUserDefaultsPlugin (0.30.2): + - FlipperKit/FlipperKitUserDefaultsPlugin (0.33.1): - FlipperKit/Core - - FlipperKit/SKIOSNetworkPlugin (0.30.2): + - FlipperKit/SKIOSNetworkPlugin (0.33.1): - FlipperKit/Core - FlipperKit/FlipperKitNetworkPlugin - Folly (2020.01.13.00): @@ -91,6 +91,7 @@ PODS: - React-RCTVibration (= 1000.0.0) - React-ART (1000.0.0): - React-Core/ARTHeaders (= 1000.0.0) + - React-callinvoker (1000.0.0) - React-Core (1000.0.0): - Folly (= 2020.01.13.00) - glog @@ -232,6 +233,7 @@ PODS: - DoubleConversion - Folly (= 2020.01.13.00) - glog + - React-callinvoker (= 1000.0.0) - React-jsinspector (= 1000.0.0) - React-jsi (1000.0.0): - boost-for-react-native (= 1.63.0) @@ -308,27 +310,22 @@ PODS: - Folly (= 2020.01.13.00) - React-Core/RCTVibrationHeaders (= 1000.0.0) - ReactCommon/turbomodule/core (= 1000.0.0) - - ReactCommon/callinvoker (1000.0.0): - - DoubleConversion - - Folly (= 2020.01.13.00) - - glog - - React-cxxreact (= 1000.0.0) - ReactCommon/turbomodule/core (1000.0.0): - DoubleConversion - Folly (= 2020.01.13.00) - glog + - React-callinvoker (= 1000.0.0) - React-Core (= 1000.0.0) - React-cxxreact (= 1000.0.0) - React-jsi (= 1000.0.0) - - ReactCommon/callinvoker (= 1000.0.0) - ReactCommon/turbomodule/samples (1000.0.0): - DoubleConversion - Folly (= 2020.01.13.00) - glog + - React-callinvoker (= 1000.0.0) - React-Core (= 1000.0.0) - React-cxxreact (= 1000.0.0) - React-jsi (= 1000.0.0) - - ReactCommon/callinvoker (= 1000.0.0) - ReactCommon/turbomodule/core (= 1000.0.0) - Yoga (1.14.0) - YogaKit (1.18.1): @@ -338,17 +335,18 @@ DEPENDENCIES: - DoubleConversion (from `../third-party-podspecs/DoubleConversion.podspec`) - FBLazyVector (from `../Libraries/FBLazyVector`) - FBReactNativeSpec (from `../Libraries/FBReactNativeSpec`) - - FlipperKit (~> 0.30.2) - - FlipperKit/FlipperKitLayoutPlugin (~> 0.30.2) - - FlipperKit/FlipperKitReactPlugin (~> 0.30.2) - - FlipperKit/FlipperKitUserDefaultsPlugin (~> 0.30.2) - - FlipperKit/SKIOSNetworkPlugin (~> 0.30.2) + - FlipperKit (~> 0.33.1) + - FlipperKit/FlipperKitLayoutPlugin (~> 0.33.1) + - FlipperKit/FlipperKitReactPlugin (~> 0.33.1) + - FlipperKit/FlipperKitUserDefaultsPlugin (~> 0.33.1) + - FlipperKit/SKIOSNetworkPlugin (~> 0.33.1) - Folly (from `../third-party-podspecs/Folly.podspec`) - glog (from `../third-party-podspecs/glog.podspec`) - RCTRequired (from `../Libraries/RCTRequired`) - RCTTypeSafety (from `../Libraries/TypeSafety`) - React (from `../`) - React-ART (from `../Libraries/ART`) + - React-callinvoker (from `../ReactCommon/callinvoker`) - React-Core (from `../`) - React-Core/DevSupport (from `../`) - React-Core/RCTWebSocket (from `../`) @@ -368,7 +366,6 @@ DEPENDENCIES: - React-RCTTest (from `./RCTTest`) - React-RCTText (from `../Libraries/Text`) - React-RCTVibration (from `../Libraries/Vibration`) - - ReactCommon/callinvoker (from `../ReactCommon`) - ReactCommon/turbomodule/core (from `../ReactCommon`) - ReactCommon/turbomodule/samples (from `../ReactCommon`) - Yoga (from `../ReactCommon/yoga`) @@ -407,6 +404,8 @@ EXTERNAL SOURCES: :path: "../" React-ART: :path: "../Libraries/ART" + React-callinvoker: + :path: "../ReactCommon/callinvoker" React-Core: :path: "../" React-CoreModules: @@ -448,18 +447,18 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: boost-for-react-native: 39c7adb57c4e60d6c5479dd8623128eb5b3f0f2c - CocoaAsyncSocket: eafaa68a7e0ec99ead0a7b35015e0bf25d2c8987 + CocoaAsyncSocket: 694058e7c0ed05a9e217d1b3c7ded962f4180845 CocoaLibEvent: 2fab71b8bd46dd33ddb959f7928ec5909f838e3f DoubleConversion: cde416483dac037923206447da6e1454df403714 FBLazyVector: 8ea0285646adaf7fa725c20ed737c49ee5ea680a FBReactNativeSpec: e8f07c749b9cf184c819f5a8ca44b91ab61fca12 - Flipper: 10b225e352595f521be0e5badddd90e241336e89 + Flipper: 6c1f484f9a88d30ab3e272800d53688439e50f69 Flipper-DoubleConversion: 38631e41ef4f9b12861c67d17cb5518d06badc41 Flipper-Folly: 2de3d03e0acc7064d5e4ed9f730e2f217486f162 Flipper-Glog: 1dfd6abf1e922806c52ceb8701a3599a79a200a6 Flipper-PeerTalk: 116d8f857dc6ef55c7a5a75ea3ceaafe878aadc9 Flipper-RSocket: 1260a31c05c238eabfa9bb8a64e3983049048371 - FlipperKit: 88b7f0d0cf907ddc2137b85eeb7f3d4d8d9395c8 + FlipperKit: 6dc9b8f4ef60d9e5ded7f0264db299c91f18832e Folly: b73c3869541e86821df3c387eb0af5f65addfab4 glog: 40a13f7840415b9a77023fbcae0f1e6f43192af3 OpenSSL-Universal: 8b48cc0d10c1b2923617dfe5c178aa9ed2689355 @@ -467,9 +466,10 @@ SPEC CHECKSUMS: RCTTypeSafety: 1ade47a69b092cddf1e4ea21e0c5bdc65cc950b4 React: cafb3c2321f7df55ce90dbf29d513799a79e4418 React-ART: df0460bdff42ef039e28ee3ffd41f50b75644788 + React-callinvoker: 0dada022d38b73e6e15b33e2a96476153f79bbf6 React-Core: 08c69f013e6fd654ea8f9fd84bbd66780a54d886 React-CoreModules: 0b59c833afcc9735e5a0220997fb18876dc9e52c - React-cxxreact: c0246279e902a2a6e71e309e8cdcbaa4877141c6 + React-cxxreact: 091da030e879ed93d970e95dd74fcbacb2a1d661 React-jsi: fe94132da767bfc4801968c2a12abae43e9a833e React-jsiexecutor: 55eff40b2e0696e7a979016e321793ec8b28a2ac React-jsinspector: 7fbf9b42b58b02943a0d89b0ba9fff0070f2de98 @@ -484,7 +484,7 @@ SPEC CHECKSUMS: React-RCTTest: cfe25fcf70b04a747dba4326105db398250caa9a React-RCTText: 6c01963d3e562109f5548262b09b1b2bc260dd60 React-RCTVibration: d218336fa28ade97e99b4ddb935f1de5c670e361 - ReactCommon: d17767bf5744406b4f4be1db7356ffbf1f99180c + ReactCommon: 39e00b754f5e1628804fab28f44146d06280f700 Yoga: f7fa200d8c49f97b54c9421079e781fb900b5cae YogaKit: f782866e155069a2cca2517aafea43200b01fd5a diff --git a/RNTester/RNTester/AppDelegate.mm b/RNTester/RNTester/AppDelegate.mm index 137ab78d979..61bb278efdd 100644 --- a/RNTester/RNTester/AppDelegate.mm +++ b/RNTester/RNTester/AppDelegate.mm @@ -21,7 +21,6 @@ #import #import #import -#import #import @@ -147,7 +146,7 @@ { _turboModuleManager = [[RCTTurboModuleManager alloc] initWithBridge:bridge delegate:self - jsInvoker:std::make_shared(bridge.reactInstance)]; + jsInvoker:bridge.jsCallInvoker]; __weak __typeof(self) weakSelf = self; return std::make_unique([weakSelf, bridge](facebook::jsi::Runtime &runtime) { if (!bridge) { diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index e8861b70bf8..48eb02b5379 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -210,9 +210,9 @@ struct RCTInstanceCallback : public InstanceCallback { return _jsMessageThread; } -- (std::weak_ptr)reactInstance +- (std::shared_ptr)jsCallInvoker { - return _reactInstance; + return _reactInstance ? _reactInstance->getJSCallInvoker() : nullptr; } - (BOOL)isInspectable @@ -1443,10 +1443,10 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithBundleURL __weak __typeof(self) weakSelf = self; [self _runAfterLoad:^{ __strong __typeof(self) strongSelf = weakSelf; - if (strongSelf->_reactInstance == nullptr) { - return; + + if (std::shared_ptr jsInvoker = strongSelf.jsCallInvoker) { + jsInvoker->invokeAsync(std::move(retainedFunc)); } - strongSelf->_reactInstance->invokeAsync(std::move(retainedFunc)); }]; } diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index 924131081d6..3b8bfc201c0 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -14,7 +14,6 @@ #include #include -#include #include #include #include @@ -288,8 +287,8 @@ void CatalystInstanceImpl::handleMemoryPressure(int pressureLevel) { jni::alias_ref CatalystInstanceImpl::getJSCallInvokerHolder() { if (!jsCallInvokerHolder_) { - jsCallInvokerHolder_ = jni::make_global(CallInvokerHolder::newObjectCxxArgs( - std::make_shared(instance_))); + jsCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(instance_->getJSCallInvoker())); } return jsCallInvokerHolder_; @@ -298,10 +297,12 @@ CatalystInstanceImpl::getJSCallInvokerHolder() { jni::alias_ref CatalystInstanceImpl::getNativeCallInvokerHolder() { if (!nativeCallInvokerHolder_) { - nativeCallInvokerHolder_ = - jni::make_global(CallInvokerHolder::newObjectCxxArgs( - std::make_shared( - moduleMessageQueue_))); + nativeCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(instance_->getNativeCallInvoker( + [moduleMessageQueue = + moduleMessageQueue_](std::function &&work) { + moduleMessageQueue->runOnQueue(std::move(work)); + }))); } return nativeCallInvokerHolder_; diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h index 94d50e245fb..32a2eef9178 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h @@ -8,7 +8,6 @@ #include #include -#include #include #include diff --git a/ReactCommon/ReactCommon.podspec b/ReactCommon/ReactCommon.podspec index 62348ea76c7..765ec10e6b8 100644 --- a/ReactCommon/ReactCommon.podspec +++ b/ReactCommon/ReactCommon.podspec @@ -36,17 +36,8 @@ Pod::Spec.new do |s| "USE_HEADERMAP" => "YES", "CLANG_CXX_LANGUAGE_STANDARD" => "c++14" } - s.subspec "callinvoker" do |ss| - ss.source_files = "callinvoker/**/*.{cpp,h}" - - ss.dependency "React-cxxreact", version - ss.dependency "DoubleConversion" - ss.dependency "Folly", folly_version - ss.dependency "glog" - end - s.subspec "turbomodule" do |ss| - ss.dependency "ReactCommon/callinvoker", version + ss.dependency "React-callinvoker", version ss.dependency "React-Core", version ss.dependency "React-cxxreact", version ss.dependency "React-jsi", version diff --git a/ReactCommon/callinvoker/Android.mk b/ReactCommon/callinvoker/Android.mk index a088e119c65..040d5d8ef63 100644 --- a/ReactCommon/callinvoker/Android.mk +++ b/ReactCommon/callinvoker/Android.mk @@ -15,8 +15,6 @@ LOCAL_EXPORT_C_INCLUDES := $(LOCAL_PATH) LOCAL_CFLAGS += -fexceptions -frtti -std=c++14 -Wall -LOCAL_STATIC_LIBRARIES = libreactnative - # Name of this module. LOCAL_MODULE := callinvoker diff --git a/ReactCommon/callinvoker/BUCK b/ReactCommon/callinvoker/BUCK index ed9a63d7818..a9c3fca7e27 100644 --- a/ReactCommon/callinvoker/BUCK +++ b/ReactCommon/callinvoker/BUCK @@ -1,4 +1,4 @@ -load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "APPLE", "react_native_xplat_target", "rn_xplat_cxx_library", "subdir_glob") +load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "APPLE", "rn_xplat_cxx_library", "subdir_glob") rn_xplat_cxx_library( name = "callinvoker", @@ -26,7 +26,4 @@ rn_xplat_cxx_library( visibility = [ "PUBLIC", ], - deps = [ - react_native_xplat_target("cxxreact:bridge"), - ], ) diff --git a/ReactCommon/callinvoker/React-callinvoker.podspec b/ReactCommon/callinvoker/React-callinvoker.podspec new file mode 100644 index 00000000000..e2a90d30943 --- /dev/null +++ b/ReactCommon/callinvoker/React-callinvoker.podspec @@ -0,0 +1,34 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +require "json" + +package = JSON.parse(File.read(File.join(__dir__, "..", "..", "package.json"))) +version = package['version'] + +source = { :git => 'https://github.com/facebook/react-native.git' } +if version == '1000.0.0' + # This is an unpublished version, use the latest commit hash of the react-native repo, which we’re presumably in. + source[:commit] = `git rev-parse HEAD`.strip +else + source[:tag] = "v#{version}" +end + +folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32' +folly_version = '2020.01.13.00' +boost_compiler_flags = '-Wno-documentation' + +Pod::Spec.new do |s| + s.name = "React-callinvoker" + s.version = version + s.summary = "-" # TODO + s.homepage = "https://reactnative.dev/" + s.license = package["license"] + s.author = "Facebook, Inc. and its affiliates" + s.platforms = { :ios => "10.0", :tvos => "10.0" } + s.source = source + s.source_files = "**/*.{cpp,h}" + s.header_dir = "ReactCommon" +end diff --git a/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.cpp b/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.cpp deleted file mode 100644 index 97d9da6cdfa..00000000000 --- a/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.cpp +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include -#include - -namespace facebook { -namespace react { - -BridgeJSCallInvoker::BridgeJSCallInvoker(std::weak_ptr reactInstance) - : reactInstance_(reactInstance) {} - -void BridgeJSCallInvoker::invokeAsync(std::function &&func) { - auto instance = reactInstance_.lock(); - if (instance == nullptr) { - return; - } - instance->invokeAsync(std::move(func)); -} - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.h b/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.h deleted file mode 100644 index 473faa37279..00000000000 --- a/ReactCommon/callinvoker/ReactCommon/BridgeJSCallInvoker.h +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include - -#include - -namespace facebook { -namespace react { - -class Instance; - -/** - * A native-to-JS call invoker that uses the bridge ('Instance'). - * It guarantees that any calls from any thread are queued on the right JS - * thread. - * - * For now, this is a thin-wrapper around existing bridge. Eventually, - * it should be consolidated with Fabric implementation so there's only one - * API to call JS from native, whether synchronously or asynchronously. - * Also, this class should not depend on `Instance` in the future. - */ -class BridgeJSCallInvoker : public CallInvoker { - public: - BridgeJSCallInvoker(std::weak_ptr reactInstance); - - void invokeAsync(std::function &&func) override; - // TODO: add sync support - - private: - std::weak_ptr reactInstance_; -}; - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.cpp b/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.cpp deleted file mode 100644 index ed8db06f811..00000000000 --- a/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.cpp +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include "MessageQueueThreadCallInvoker.h" - -namespace facebook { -namespace react { - -MessageQueueThreadCallInvoker::MessageQueueThreadCallInvoker( - std::shared_ptr moduleMessageQueue) - : moduleMessageQueue_(moduleMessageQueue) {} - -void MessageQueueThreadCallInvoker::invokeAsync(std::function &&func) { - moduleMessageQueue_->runOnQueue(std::move(func)); -} - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.h b/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.h deleted file mode 100644 index cc60348ef2a..00000000000 --- a/ReactCommon/callinvoker/ReactCommon/MessageQueueThreadCallInvoker.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include - -#include -#include - -namespace facebook { -namespace react { - -/** - * Used to schedule async calls on the NativeModuels thread. - */ -class MessageQueueThreadCallInvoker : public CallInvoker { - public: - MessageQueueThreadCallInvoker( - std::shared_ptr moduleMessageQueue); - - void invokeAsync(std::function &&func) override; - // TODO: add sync support - - private: - std::shared_ptr moduleMessageQueue_; -}; - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/cxxreact/Android.mk b/ReactCommon/cxxreact/Android.mk index 5ac26f435c4..553e0a93e5c 100644 --- a/ReactCommon/cxxreact/Android.mk +++ b/ReactCommon/cxxreact/Android.mk @@ -19,13 +19,14 @@ LOCAL_CFLAGS := \ LOCAL_CFLAGS += -fexceptions -frtti -Wno-unused-lambda-capture -LOCAL_STATIC_LIBRARIES := boost jsi +LOCAL_STATIC_LIBRARIES := boost jsi callinvoker LOCAL_SHARED_LIBRARIES := jsinspector libfolly_json glog include $(BUILD_STATIC_LIBRARY) $(call import-module,fb) $(call import-module,folly) +$(call import-module,callinvoker) $(call import-module,jsc) $(call import-module,glog) $(call import-module,jsi) diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index f1d4888fcd7..8ef91ae51b0 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/ReactCommon/cxxreact/BUCK @@ -148,6 +148,7 @@ rn_xplat_cxx_library( "//xplat/folly:headers_only", "//xplat/folly:memory", "//xplat/folly:molly", + react_native_xplat_target("callinvoker:callinvoker"), react_native_xplat_target("jsinspector:jsinspector"), react_native_xplat_target("microprofiler:microprofiler"), "//xplat/folly:optional", diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index 65f1ce171d5..5144921f17e 100644 --- a/ReactCommon/cxxreact/Instance.cpp +++ b/ReactCommon/cxxreact/Instance.cpp @@ -46,9 +46,16 @@ void Instance::initializeBridge( callback_ = std::move(callback); moduleRegistry_ = std::move(moduleRegistry); jsQueue->runOnQueueSync([this, &jsef, jsQueue]() mutable { - nativeToJsBridge_ = std::make_unique( + nativeToJsBridge_ = std::make_shared( jsef.get(), moduleRegistry_, jsQueue, callback_); + /** + * After NativeToJsBridge is created, the jsi::Runtime should exist. + * Also, the JS message queue thread exists. So, it's safe to + * schedule all queued up js Calls. + */ + jsCallInvoker_->setNativeToJsBridgeAndFlushCalls(nativeToJsBridge_); + std::lock_guard lock(m_syncMutex); m_syncReady = true; m_syncCV.notify_all(); @@ -215,12 +222,60 @@ void Instance::handleMemoryPressure(int pressureLevel) { nativeToJsBridge_->handleMemoryPressure(pressureLevel); } -void Instance::invokeAsync(std::function &&func) { - nativeToJsBridge_->runOnExecutorQueue( - [func = std::move(func)](JSExecutor *executor) { - func(); - executor->flush(); - }); +std::shared_ptr Instance::getJSCallInvoker() { + return std::static_pointer_cast(jsCallInvoker_); +} + +std::shared_ptr Instance::getNativeCallInvoker( + std::function &&work)> &&scheduleWork) { + return nativeToJsBridge_->getNativeCallInvoker(std::move(scheduleWork)); +} + +void Instance::JSCallInvoker::setNativeToJsBridgeAndFlushCalls( + std::weak_ptr nativeToJsBridge) { + std::lock_guard guard(m_mutex); + + m_shouldBuffer = false; + m_nativeToJsBridge = nativeToJsBridge; + while (m_workBuffer.size() > 0) { + scheduleAsync(std::move(m_workBuffer.front())); + m_workBuffer.pop_front(); + } +} + +void Instance::JSCallInvoker::invokeAsync(std::function &&work) { + std::lock_guard guard(m_mutex); + + /** + * Why is is necessary to queue up async work? + * + * 1. TurboModuleManager must be created synchronously after the Instance, + * before we load the source code. This is when the NativeModule system + * is initialized. RCTDevLoadingView shows bundle download progress. + * 2. TurboModuleManager requires a JS CallInvoker. + * 3. The JS CallInvoker requires the NativeToJsBridge, which is created on + * the JS thread in Instance::initializeBridge. + * + * Therefore, although we don't anyone to call invokeAsync before the JS + * bundle is executed, this buffering is implemented anyways to ensure + * that work isn't discarded. + */ + if (m_shouldBuffer) { + m_workBuffer.push_back(std::move(work)); + return; + } + + scheduleAsync(std::move(work)); +} + +void Instance::JSCallInvoker::scheduleAsync(std::function &&work) { + if (auto strongNativeToJsBridge = m_nativeToJsBridge.lock()) { + strongNativeToJsBridge->runOnExecutorQueue( + [work = std::move(work)](JSExecutor *executor) { + work(); + executor->flush(); + }); + } } } // namespace react diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index ee2ebc44b73..7aabee19abc 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -8,7 +8,9 @@ #pragma once #include +#include #include +#include #include @@ -86,7 +88,45 @@ class RN_EXPORT Instance { void handleMemoryPressure(int pressureLevel); - void invokeAsync(std::function &&func); + /** + * JS CallInvoker is used by TurboModules to schedule work on the JS thread. + * + * Why is the bridge creating JS CallInvoker? + * + * - After every Native -> JS call in the TurboModule system, the bridge + * needs to flush all queued NativeModule method calls. The bridge must + * also dispatch onBatchComplete if the queue of NativeModule method calls + * was not empty. + */ + std::shared_ptr getJSCallInvoker(); + + /** + * Native CallInvoker is used by TurboModules to schedule work on the + * NativeModule thread(s). + * + * Why is the bridge creating JS CallInvoker? + * + * - The bridge must be informed of all TurboModule async method calls. Why? + * When all queued NativeModule method calls are flushed by a call from + * Native -> JS, if that queue was non-zero in size, JsToNativeBridge + * dispatches onBatchComplete. When we turn our NativeModules to + * TurboModuels, there will be less and less pending NativeModule method + * calls, so onBatchComplete will not fire as often. Therefore, the bridge + * needs to know how many TurboModule async method calls have been completed + * since the last time the bridge was flushed. If this number is non-zero, + * we fire onBatchComplete. + * + * Why must we pass in a scheduleWork function? + * + * - On Android, we have one NativeModule thread. That thread is created and + * managed outisde of NativeToJsBridge. On iOS, we have one MethodQueue per + * module. Those MethodQueues are also created and managed outside of + * NativeToJsBridge. Therefore, we need to pass in a function that schedules + * work on the respective thread. + * + */ + std::shared_ptr getNativeCallInvoker( + std::function &&work)> &&scheduleWork); private: void callNativeModules(folly::dynamic &&calls, bool isEndOfBatch); @@ -100,12 +140,30 @@ class RN_EXPORT Instance { std::string startupScriptSourceURL); std::shared_ptr callback_; - std::unique_ptr nativeToJsBridge_; + std::shared_ptr nativeToJsBridge_; std::shared_ptr moduleRegistry_; std::mutex m_syncMutex; std::condition_variable m_syncCV; bool m_syncReady = false; + + class JSCallInvoker : public CallInvoker { + private: + std::weak_ptr m_nativeToJsBridge; + std::mutex m_mutex; + bool m_shouldBuffer = true; + std::list> m_workBuffer; + + void scheduleAsync(std::function &&work); + + public: + void setNativeToJsBridgeAndFlushCalls( + std::weak_ptr nativeToJsBridge); + void invokeAsync(std::function &&work) override; + }; + + std::shared_ptr jsCallInvoker_ = + std::make_shared(); }; } // namespace react diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index 0e34f942a85..a34041e5a1a 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -7,6 +7,7 @@ #include "NativeToJsBridge.h" +#include #include #include #include @@ -42,7 +43,7 @@ class JsToNativeBridge : public react::ExecutorDelegate { } bool isBatchActive() { - return m_batchHadNativeModuleCalls; + return m_batchHadNativeModuleOrTurboModuleCalls; } void callNativeModules( @@ -51,7 +52,8 @@ class JsToNativeBridge : public react::ExecutorDelegate { bool isEndOfBatch) override { CHECK(m_registry || calls.empty()) << "native module calls cannot be completed with no native modules"; - m_batchHadNativeModuleCalls = m_batchHadNativeModuleCalls || !calls.empty(); + m_batchHadNativeModuleOrTurboModuleCalls = + m_batchHadNativeModuleOrTurboModuleCalls || !calls.empty(); // An exception anywhere in here stops processing of the batch. This // was the behavior of the Android bridge, and since exception handling @@ -65,9 +67,9 @@ class JsToNativeBridge : public react::ExecutorDelegate { // decrementPendingJSCalls will be called sync. Be aware that the bridge // may still be processing native calls when the bridge idle signaler // fires. - if (m_batchHadNativeModuleCalls) { + if (m_batchHadNativeModuleOrTurboModuleCalls) { m_callback->onBatchComplete(); - m_batchHadNativeModuleCalls = false; + m_batchHadNativeModuleOrTurboModuleCalls = false; } m_callback->decrementPendingJSCalls(); } @@ -82,13 +84,17 @@ class JsToNativeBridge : public react::ExecutorDelegate { moduleId, methodId, std::move(args)); } + void recordTurboModuleAsyncMethodCall() { + m_batchHadNativeModuleOrTurboModuleCalls = true; + } + private: // These methods are always invoked from an Executor. The NativeToJsBridge // keeps a reference to the executor, and when destroy() is called, the // executor is destroyed synchronously on its queue. std::shared_ptr m_registry; std::shared_ptr m_callback; - bool m_batchHadNativeModuleCalls = false; + bool m_batchHadNativeModuleOrTurboModuleCalls = false; }; NativeToJsBridge::NativeToJsBridge( @@ -291,5 +297,31 @@ void NativeToJsBridge::runOnExecutorQueue( }); } +std::shared_ptr NativeToJsBridge::getNativeCallInvoker( + std::function &&work)> &&scheduleWork) { + class NativeCallInvoker : public CallInvoker { + private: + std::weak_ptr m_jsToNativeBridge; + std::function &&work)> m_scheduleWork; + + public: + NativeCallInvoker( + std::weak_ptr jsToNativeBridge, + std::function &&work)> &&scheduleWork) + : m_jsToNativeBridge(jsToNativeBridge), + m_scheduleWork(std::move(scheduleWork)) {} + + void invokeAsync(std::function &&func) override { + if (auto strongJsToNativeBridge = m_jsToNativeBridge.lock()) { + strongJsToNativeBridge->recordTurboModuleAsyncMethodCall(); + } + m_scheduleWork(std::move(func)); + } + }; + + return std::make_shared( + m_delegate, std::move(scheduleWork)); +} + } // namespace react } // namespace facebook diff --git a/ReactCommon/cxxreact/NativeToJsBridge.h b/ReactCommon/cxxreact/NativeToJsBridge.h index e94bb21f06e..0a4f6d400a0 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.h +++ b/ReactCommon/cxxreact/NativeToJsBridge.h @@ -12,6 +12,7 @@ #include #include +#include #include namespace folly { @@ -93,6 +94,13 @@ class NativeToJsBridge { void runOnExecutorQueue(std::function task); + /** + * Native CallInvoker is used by TurboModules to schedule work on the + * NativeModule thread(s). + */ + std::shared_ptr getNativeCallInvoker( + std::function &&work)> &&scheduleWork); + private: // This is used to avoid a race condition where a proxyCallback gets queued // after ~NativeToJsBridge(), on the same thread. In that case, the callback diff --git a/ReactCommon/cxxreact/React-cxxreact.podspec b/ReactCommon/cxxreact/React-cxxreact.podspec index bba625c8439..cb6835be464 100644 --- a/ReactCommon/cxxreact/React-cxxreact.podspec +++ b/ReactCommon/cxxreact/React-cxxreact.podspec @@ -40,4 +40,5 @@ Pod::Spec.new do |s| s.dependency "Folly", folly_version s.dependency "glog" s.dependency "React-jsinspector", version + s.dependency "React-callinvoker", version end diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h index 9eda3b989d9..8b008fac2ac 100644 --- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h +++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.h @@ -212,9 +212,13 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { @end -// TODO: Consolidate this extension with the one in RCTSurfacePresenter. +/** + * These methods are all implemented by RCTCxxBridge, which subclasses RCTBridge. Hence, they must only be used in + * contexts where the concrete class of an RCTBridge instance is RCTCxxBridge. This happens, for example, when + * [RCTCxxBridgeDelegate jsExecutorFactoryForBridge:(RCTBridge *)] is invoked by RCTCxxBridge. + * + * TODO: Consolidate this extension with the one in RCTSurfacePresenter. + */ @interface RCTBridge () - -- (std::weak_ptr)reactInstance; - +- (std::shared_ptr)jsCallInvoker; @end diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb index e8988cb6121..e1afb436b90 100644 --- a/scripts/react_native_pods.rb +++ b/scripts/react_native_pods.rb @@ -40,7 +40,7 @@ def use_react_native! (options={}) pod 'React-jsi', :path => "#{prefix}/ReactCommon/jsi" pod 'React-jsiexecutor', :path => "#{prefix}/ReactCommon/jsiexecutor" pod 'React-jsinspector', :path => "#{prefix}/ReactCommon/jsinspector" - pod 'ReactCommon/callinvoker', :path => "#{prefix}/ReactCommon" + pod 'React-callinvoker', :path => "#{prefix}/ReactCommon/callinvoker" pod 'ReactCommon/turbomodule/core', :path => "#{prefix}/ReactCommon" pod 'Yoga', :path => "#{prefix}/ReactCommon/yoga", :modular_headers => true