Skip to content

Commit

Permalink
Migrate from native CallInvoker to NativeMethodCallInvoker (#37473)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #37473

## Context
The TurboModule system uses a CallInvoker interface to schedule JavaScript → native, and native → JavaScript calls.

## Problem
JavaScript → native and native → JavaScript calls are different. They can have different behaviours/properties. And, in the future, we might want to evolve them differently. e.g:
- JavaScript → native **always** have a method name. Native → JavaScript calls don't.
- Native → JavaScript can have priorities, since D41492849. JavaScript → native don't.

## Changes
Instead of tying both types of calls to the CallInvoker abstraction, this diff creates a **separate** abstraction for Native → JavaScript calls: NativeMethodCallInvoker.

This way, we can evolve both abstractions separately over time:
- We can evolve the CallInvoker abstraction to suit the needs of JavaScript → native calls.
- We can evolve the NativeMethodCallInvoker abstraction to suite the needs of native → JavaScript calls.

This ultimately makes TurboModule system more extensible.

## Motivation
For the TurboModule interop layer on iOS, React Native needs to execute the "constantsToExport" method on the main queue, when the module requires main queue setup. (implementation: D45924977).

The simplest way to implement this behaviour is to introduce a `methodName` to CallInvoker, and customize the legacy module's CallInvoker::invokeSync method, like so:

```
  void invokeSync(std::string methodName, std::function<void()> &&work) override
  {
    if (requiresMainQueueSetup_ && methodName == "getConstants") {
      __block auto retainedWork = std::move(work);
      RCTUnsafeExecuteOnMainQueueSync(^{
        retainedWork();
      });
      return;
    }

    work();
  }
```

But, customizing CallInvoker to introduce a `methodName` parameter doesn't make sense: Native → JavaScript calls don't necessarily have method names. So, this diff forks CallInvoker into NativeMethodCallInvoker. That way, we can customize NativeMethodCallInvoker to introduce a method name (which does make sense) and resolve this problem. For the full solution, see D45924977.

NOTE: Now that NativeMethodCallInvoker is different from CallInvoker, it might make sense to re-name CallInvoker back to JSCallInvoker.

Changelog:
[Category][Breaking] - Introduce NativeMethodCallInvoker to replace the TurboModule system's native CallInvoker

Reviewed By: javache

Differential Revision: D45891627

fbshipit-source-id: 39c3f450a290ad396b715288a50858d33ce78441
  • Loading branch information
RSNara authored and facebook-github-bot committed May 23, 2023
1 parent d470dee commit b70f186
Show file tree
Hide file tree
Showing 29 changed files with 275 additions and 120 deletions.
5 changes: 3 additions & 2 deletions packages/react-native/React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1635,9 +1635,10 @@ - (void)invokeAsync:(std::function<void()> &&)func
return _reactInstance ? _reactInstance->getJSCallInvoker() : nullptr;
}

- (std::shared_ptr<CallInvoker>)decorateNativeCallInvoker:(std::shared_ptr<CallInvoker>)nativeInvoker
- (std::shared_ptr<NativeMethodCallInvoker>)decorateNativeMethodCallInvoker:
(std::shared_ptr<NativeMethodCallInvoker>)nativeInvoker
{
return _reactInstance ? _reactInstance->getDecoratedNativeCallInvoker(nativeInvoker) : nullptr;
return _reactInstance ? _reactInstance->getDecoratedNativeMethodCallInvoker(nativeInvoker) : nullptr;
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ private ReactApplicationContext createReactContext(
catalystInstance.getRuntimeExecutor(),
tmmDelegate,
catalystInstance.getJSCallInvokerHolder(),
catalystInstance.getNativeCallInvokerHolder());
catalystInstance.getNativeMethodCallInvokerHolder());

catalystInstance.setTurboModuleManager(turboModuleManager);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.facebook.react.bridge.queue.ReactQueueConfiguration;
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder;
import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHolder;
import java.util.Collection;
import java.util.List;

Expand Down Expand Up @@ -122,10 +123,10 @@ public interface CatalystInstance
CallInvokerHolder getJSCallInvokerHolder();

/**
* Returns a hybrid object that contains a pointer to a Native CallInvoker, which is used to
* Returns a hybrid object that contains a pointer to a NativeMethodCallInvoker, which is used to
* schedule work on the NativeModules thread. Required for TurboModuleManager initialization.
*/
CallInvokerHolder getNativeCallInvokerHolder();
NativeMethodCallInvokerHolder getNativeMethodCallInvokerHolder();

/**
* For the time being, we want code relying on the old infra to also work with TurboModules.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.turbomodule.core.CallInvokerHolderImpl;
import com.facebook.react.turbomodule.core.NativeMethodCallInvokerHolderImpl;
import com.facebook.react.turbomodule.core.interfaces.TurboModuleRegistry;
import com.facebook.systrace.Systrace;
import com.facebook.systrace.TraceListener;
Expand Down Expand Up @@ -111,7 +112,7 @@ public String toString() {

public native CallInvokerHolderImpl getJSCallInvokerHolder();

public native CallInvokerHolderImpl getNativeCallInvokerHolder();
public native NativeMethodCallInvokerHolderImpl getNativeMethodCallInvokerHolder();

private CatalystInstanceImpl(
final ReactQueueConfigurationSpec reactQueueConfigurationSpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.facebook.react.modules.core.JavaTimerManager;
import com.facebook.react.modules.core.ReactChoreographer;
import com.facebook.react.turbomodule.core.CallInvokerHolderImpl;
import com.facebook.react.turbomodule.core.NativeMethodCallInvokerHolderImpl;
import com.facebook.react.turbomodule.core.TurboModuleManager;
import com.facebook.react.turbomodule.core.TurboModuleManagerDelegate;
import com.facebook.react.uimanager.ComponentNameResolver;
Expand Down Expand Up @@ -193,7 +194,7 @@ public void onHostDestroy() {
unbufferedRuntimeExecutor,
turboModuleManagerDelegate,
getJSCallInvokerHolder(),
getNativeCallInvokerHolder());
getNativeMethodCallInvokerHolder());

// Eagerly initialize TurboModules
for (String moduleName : mTurboModuleManager.getEagerInitModuleNames()) {
Expand Down Expand Up @@ -397,7 +398,7 @@ private native HybridData initHybrid(

private native CallInvokerHolderImpl getJSCallInvokerHolder();

private native CallInvokerHolderImpl getNativeCallInvokerHolder();
private native NativeMethodCallInvokerHolderImpl getNativeMethodCallInvokerHolder();

private native RuntimeExecutor getUnbufferedRuntimeExecutor();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.turbomodule.core;

import com.facebook.jni.HybridData;
import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHolder;
import com.facebook.soloader.SoLoader;

/**
* NativeMethodCallInvokerHolder is created at a different time/place (i.e: in CatalystInstance)
* than TurboModuleManager. Therefore, we need to wrap NativeMethodCallInvokerHolder within a hybrid
* class so that we may pass it from CatalystInstance, through Java, to
* TurboModuleManager::initHybrid.
*/
public class NativeMethodCallInvokerHolderImpl implements NativeMethodCallInvokerHolder {
private static volatile boolean sIsSoLibraryLoaded;

private final HybridData mHybridData;

private NativeMethodCallInvokerHolderImpl(HybridData hd) {
maybeLoadSoLibrary();
mHybridData = hd;
}

// Prevents issues with initializer interruptions. See T38996825 and D13793825 for more context.
private static synchronized void maybeLoadSoLibrary() {
if (!sIsSoLibraryLoaded) {
SoLoader.loadLibrary("turbomodulejsijni");
sIsSoLibraryLoaded = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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.NativeMethodCallInvokerHolder;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
import com.facebook.react.turbomodule.core.interfaces.TurboModuleRegistry;
import com.facebook.soloader.SoLoader;
Expand Down Expand Up @@ -61,14 +62,14 @@ public TurboModuleManager(
RuntimeExecutor runtimeExecutor,
@Nullable final TurboModuleManagerDelegate delegate,
CallInvokerHolder jsCallInvokerHolder,
CallInvokerHolder nativeCallInvokerHolder) {
NativeMethodCallInvokerHolder nativeMethodCallInvokerHolder) {
maybeLoadSoLibrary();
mDelegate = delegate;
mHybridData =
initHybrid(
runtimeExecutor,
(CallInvokerHolderImpl) jsCallInvokerHolder,
(CallInvokerHolderImpl) nativeCallInvokerHolder,
(NativeMethodCallInvokerHolderImpl) nativeMethodCallInvokerHolder,
delegate);
installJSIBindings(shouldCreateLegacyModules());

Expand Down Expand Up @@ -405,7 +406,7 @@ public static void logError(String message) {
private native HybridData initHybrid(
RuntimeExecutor runtimeExecutor,
CallInvokerHolderImpl jsCallInvokerHolder,
CallInvokerHolderImpl nativeCallInvokerHolder,
NativeMethodCallInvokerHolderImpl nativeMethodCallInvoker,
TurboModuleManagerDelegate tmmDelegate);

private native void installJSIBindings(boolean shouldCreateLegacyModules);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.turbomodule.core.interfaces;

/**
* This interface represents the opaque Java object that contains a pointer to and instance of
* NativeMethodCallInvoker.
*/
public interface NativeMethodCallInvokerHolder {}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <jsi/jsi.h>
#include <jsireact/JSIExecutor.h>
#include <react/bridgeless/BridgelessJSCallInvoker.h>
#include <react/bridgeless/BridgelessNativeCallInvoker.h>
#include <react/bridgeless/BridgelessNativeMethodCallInvoker.h>
#include <react/common/mapbuffer/JReadableMapBuffer.h>
#include <react/jni/JRuntimeExecutor.h>
#include <react/jni/JSLogging.h>
Expand Down Expand Up @@ -90,10 +90,12 @@ JReactInstance::JReactInstance(
std::make_unique<BridgelessJSCallInvoker>(unbufferedRuntimeExecutor);
jsCallInvokerHolder_ = jni::make_global(
CallInvokerHolder::newObjectCxxArgs(std::move(jsInvoker)));
auto nativeInvoker = std::make_unique<BridgelessNativeCallInvoker>(
sharedNativeMessageQueueThread);
nativeCallInvokerHolder_ = jni::make_global(
CallInvokerHolder::newObjectCxxArgs(std::move(nativeInvoker)));
auto nativeMethodCallInvoker =
std::make_unique<BridgelessNativeMethodCallInvoker>(
sharedNativeMessageQueueThread);
nativeMethodCallInvokerHolder_ =
jni::make_global(NativeMethodCallInvokerHolder::newObjectCxxArgs(
std::move(nativeMethodCallInvoker)));

// Storing this here to make sure the Java reference doesn't get destroyed
unbufferedRuntimeExecutor_ = jni::make_global(
Expand Down Expand Up @@ -156,9 +158,9 @@ JReactInstance::getJSCallInvokerHolder() {
return jsCallInvokerHolder_;
}

jni::alias_ref<CallInvokerHolder::javaobject>
JReactInstance::getNativeCallInvokerHolder() {
return nativeCallInvokerHolder_;
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
JReactInstance::getNativeMethodCallInvokerHolder() {
return nativeMethodCallInvokerHolder_;
}

jni::global_ref<JJSTimerExecutor::javaobject>
Expand Down Expand Up @@ -211,8 +213,8 @@ void JReactInstance::registerNatives() {
makeNativeMethod(
"getJSCallInvokerHolder", JReactInstance::getJSCallInvokerHolder),
makeNativeMethod(
"getNativeCallInvokerHolder",
JReactInstance::getNativeCallInvokerHolder),
"getNativeMethodCallInvokerHolder",
JReactInstance::getNativeMethodCallInvokerHolder),
makeNativeMethod(
"callFunctionOnModule", JReactInstance::callFunctionOnModule),
makeNativeMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#pragma once

#include <ReactCommon/CallInvokerHolder.h>
#include <ReactCommon/NativeMethodCallInvokerHolder.h>
#include <ReactCommon/RuntimeExecutor.h>
#include <fb/fbjni.h>
#include <jni.h>
Expand Down Expand Up @@ -92,14 +93,16 @@ class JReactInstance : public jni::HybridClass<JReactInstance> {
bool isProfiling) noexcept;

jni::alias_ref<CallInvokerHolder::javaobject> getJSCallInvokerHolder();
jni::alias_ref<CallInvokerHolder::javaobject> getNativeCallInvokerHolder();
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
getNativeMethodCallInvokerHolder();

std::unique_ptr<ReactInstance> instance_;
jni::global_ref<JRuntimeExecutor::javaobject> unbufferedRuntimeExecutor_;
jni::global_ref<JRuntimeExecutor::javaobject> bufferedRuntimeExecutor_;
jni::global_ref<JRuntimeScheduler::javaobject> runtimeScheduler_;
jni::global_ref<CallInvokerHolder::javaobject> jsCallInvokerHolder_;
jni::global_ref<CallInvokerHolder::javaobject> nativeCallInvokerHolder_;
jni::global_ref<NativeMethodCallInvokerHolder::javaobject>
nativeMethodCallInvokerHolder_;
jni::global_ref<JReactExceptionManager::javaobject> jReactExceptionManager_;
jni::global_ref<JBindingsInstaller::javaobject> jBindingsInstaller_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ void CatalystInstanceImpl::registerNatives() {
"getJSCallInvokerHolder",
CatalystInstanceImpl::getJSCallInvokerHolder),
makeNativeMethod(
"getNativeCallInvokerHolder",
CatalystInstanceImpl::getNativeCallInvokerHolder),
"getNativeMethodCallInvokerHolder",
CatalystInstanceImpl::getNativeMethodCallInvokerHolder),
makeNativeMethod(
"jniHandleMemoryPressure",
CatalystInstanceImpl::handleMemoryPressure),
Expand Down Expand Up @@ -358,36 +358,41 @@ CatalystInstanceImpl::getJSCallInvokerHolder() {
return jsCallInvokerHolder_;
}

jni::alias_ref<CallInvokerHolder::javaobject>
CatalystInstanceImpl::getNativeCallInvokerHolder() {
if (!nativeCallInvokerHolder_) {
class NativeThreadCallInvoker : public CallInvoker {
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
CatalystInstanceImpl::getNativeMethodCallInvokerHolder() {
if (!nativeMethodCallInvokerHolder_) {
class NativeMethodCallInvokerImpl : public NativeMethodCallInvoker {
private:
std::shared_ptr<JMessageQueueThread> messageQueueThread_;

public:
NativeThreadCallInvoker(
NativeMethodCallInvokerImpl(
std::shared_ptr<JMessageQueueThread> messageQueueThread)
: messageQueueThread_(messageQueueThread) {}
void invokeAsync(std::function<void()> &&work) override {
void invokeAsync(
const std::string &methodName,
std::function<void()> &&work) override {
messageQueueThread_->runOnQueue(std::move(work));
}
void invokeSync(std::function<void()> &&work) override {
void invokeSync(
const std::string &methodName,
std::function<void()> &&work) override {
messageQueueThread_->runOnQueueSync(std::move(work));
}
};

std::shared_ptr<CallInvoker> nativeInvoker =
std::make_shared<NativeThreadCallInvoker>(moduleMessageQueue_);
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker =
std::make_shared<NativeMethodCallInvokerImpl>(moduleMessageQueue_);

std::shared_ptr<CallInvoker> decoratedNativeInvoker =
instance_->getDecoratedNativeCallInvoker(nativeInvoker);
std::shared_ptr<NativeMethodCallInvoker> decoratedNativeMethodCallInvoker =
instance_->getDecoratedNativeMethodCallInvoker(nativeMethodCallInvoker);

nativeCallInvokerHolder_ = jni::make_global(
CallInvokerHolder::newObjectCxxArgs(decoratedNativeInvoker));
nativeMethodCallInvokerHolder_ =
jni::make_global(NativeMethodCallInvokerHolder::newObjectCxxArgs(
decoratedNativeMethodCallInvoker));
}

return nativeCallInvokerHolder_;
return nativeMethodCallInvokerHolder_;
}

jni::alias_ref<JRuntimeExecutor::javaobject>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <string>

#include <ReactCommon/CallInvokerHolder.h>
#include <ReactCommon/NativeMethodCallInvokerHolder.h>
#include <ReactCommon/RuntimeExecutor.h>
#include <fbjni/fbjni.h>

Expand Down Expand Up @@ -96,7 +97,8 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
NativeArray *arguments);
void jniCallJSCallback(jint callbackId, NativeArray *arguments);
jni::alias_ref<CallInvokerHolder::javaobject> getJSCallInvokerHolder();
jni::alias_ref<CallInvokerHolder::javaobject> getNativeCallInvokerHolder();
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
getNativeMethodCallInvokerHolder();
jni::alias_ref<JRuntimeExecutor::javaobject> getRuntimeExecutor();
jni::alias_ref<JRuntimeScheduler::javaobject> getRuntimeScheduler();
void setGlobalVariable(std::string propName, std::string &&jsonValue);
Expand All @@ -111,7 +113,8 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
std::shared_ptr<ModuleRegistry> moduleRegistry_;
std::shared_ptr<JMessageQueueThread> moduleMessageQueue_;
jni::global_ref<CallInvokerHolder::javaobject> jsCallInvokerHolder_;
jni::global_ref<CallInvokerHolder::javaobject> nativeCallInvokerHolder_;
jni::global_ref<NativeMethodCallInvokerHolder::javaobject>
nativeMethodCallInvokerHolder_;
jni::global_ref<JRuntimeExecutor::javaobject> runtimeExecutor_;
jni::global_ref<JRuntimeScheduler::javaobject> runtimeScheduler_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ add_library(
callinvokerholder
STATIC
ReactCommon/CallInvokerHolder.cpp
ReactCommon/NativeMethodCallInvokerHolder.cpp
)

target_include_directories(callinvokerholder
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#include "NativeMethodCallInvokerHolder.h"

namespace facebook::react {

NativeMethodCallInvokerHolder::NativeMethodCallInvokerHolder(
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker)
: _nativeMethodCallInvoker(nativeMethodCallInvoker) {}

std::shared_ptr<NativeMethodCallInvoker>
NativeMethodCallInvokerHolder::getNativeMethodCallInvoker() {
return _nativeMethodCallInvoker;
}

void NativeMethodCallInvokerHolder::registerNatives() {}

} // namespace facebook::react
Loading

0 comments on commit b70f186

Please sign in to comment.