Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce native api to access RuntimeExecutor #42882

Closed
wants to merge 1 commit into from

Conversation

philIip
Copy link
Contributor

@philIip philIip commented Feb 6, 2024

Summary:
Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of PR#42758.

From PR#42758

The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, i created an async api that will guarantee the runtime executor will be ready.

Differential Revision: D53461821

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 6, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

philIip added a commit to philIip/react-native that referenced this pull request Feb 6, 2024
Summary:

Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of [PR#42758](facebook#42758).

From [PR#42758](facebook#42758)

> The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

> However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, i created an async api that will guarantee the runtime executor will be ready.

Differential Revision: D53461821
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

@analysis-bot
Copy link

analysis-bot commented Feb 6, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,233,692 -7
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,600,119 -1
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: e37da1e
Branch: main

Comment on lines 133 to 141

reactContext.executeRuntimeExecutor(
new ReactContext.ReactRuntimeExecutor() {
@Override
public void execute(RuntimeExecutor runtimeExecutor) {
// do something
}
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reactContext.executeRuntimeExecutor(
new ReactContext.ReactRuntimeExecutor() {
@Override
public void execute(RuntimeExecutor runtimeExecutor) {
// do something
}
});

It doesn't do anything 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh whoops this was my testing code hehe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a different module for this test.

At meta: FrescoModule is eagerly initialized when the ReactInstance is created.

So when this initialize method is executed, the ReactContext and the ReactHost don't have access to the ReactInstance reference (and hence the runtime executor reference), because it's being created.

That's probably why you had to create an async api to access the RuntimeExecutor?

Comment on lines 572 to 585
void executeRuntimeExecutor(ReactRuntimeExecutor reactRuntimeExecutor) {
mReactInstanceTaskRef
.get()
.onSuccess(
task -> {
final ReactInstance reactInstance = task.getResult();
if (reactInstance == null) {
return false;
}
reactRuntimeExecutor.execute(reactInstance.getBufferedRuntimeExecutor());
return true;
},
mBGExecutor);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to the runtime executer will be obtained asynchronously. Currently, in our flow, module initialization starts when native constants are received. It is uncertain whether this will be invoked in time before the JS continues executing.

Copy link
Contributor

@RSNara RSNara Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukmccall, the code is very different now.

NativeModules can now access the runtime by calling ReactContext.getRuntimeExecutor().

This method should work for all cases except for this one: when ReactHost's ReactInstance is/becomes null.

This happens in two scenarios:

  1. The native module is created eagerly (i.e: while ReactHost is creating the ReactInstance).
  2. The native module outlives the ReactInstance (i.e: ReactHost kills the ReactInstance, but something else keeps the native module alive).

Aside from that, I think the concern you mentioned about timing still exists.

I talked with Phillip offline: Maybe we could continue with this solution for now, and iterate on it, if we see any problems? I think we want to replace this solution with something more reliable down the line anyway.

Comment on lines 572 to 573
void executeRuntimeExecutor(ReactRuntimeExecutor reactRuntimeExecutor) {
mReactInstanceTaskRef
.get()
.onSuccess(
task -> {
final ReactInstance reactInstance = task.getResult();
if (reactInstance == null) {
return false;
}
reactRuntimeExecutor.execute(reactInstance.getBufferedRuntimeExecutor());
return true;
},
mBGExecutor);
}

/* package */
@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void executeRuntimeExecutor(ReactRuntimeExecutor reactRuntimeExecutor) {
mReactInstanceTaskRef
.get()
.onSuccess(
task -> {
final ReactInstance reactInstance = task.getResult();
if (reactInstance == null) {
return false;
}
reactRuntimeExecutor.execute(reactInstance.getBufferedRuntimeExecutor());
return true;
},
mBGExecutor);
}
/* package */
@Nullable
@Deprecated
RuntimeExecutor getRuntimeExecutor() {
return mReactInstanceTaskRef.get().getResult().getBufferedRuntimeExecutor();
}
/* package */
@Nullable

Comment on lines 158 to 166
@Override
public void executeRuntimeExecutor(ReactRuntimeExecutor runtimeExecutor) {
mReactHost.executeRuntimeExecutor(runtimeExecutor);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Override
public void executeRuntimeExecutor(ReactRuntimeExecutor runtimeExecutor) {
mReactHost.executeRuntimeExecutor(runtimeExecutor);
}
@Override
public RuntimeExecutor getRuntimeExecutor() {
return mReactHost.getRuntimeExecutor();
}

Copy link
Contributor Author

@philIip philIip Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea this was my first version but i ran into the issue with FrescoModule T_T but what you're saying for needsEagerInit for the FrescoModule makes sense

Comment on lines 133 to 141

reactContext.executeRuntimeExecutor(
new ReactContext.ReactRuntimeExecutor() {
@Override
public void execute(RuntimeExecutor runtimeExecutor) {
// do something
}
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a different module for this test.

At meta: FrescoModule is eagerly initialized when the ReactInstance is created.

So when this initialize method is executed, the ReactContext and the ReactHost don't have access to the ReactInstance reference (and hence the runtime executor reference), because it's being created.

That's probably why you had to create an async api to access the RuntimeExecutor?

philIip added a commit to philIip/react-native that referenced this pull request Feb 7, 2024
Summary:

Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of [PR#42758](facebook#42758).

From [PR#42758](facebook#42758)

> The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

> However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, i created an async api that will guarantee the runtime executor will be ready.

Differential Revision: D53461821
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

philIip added a commit to philIip/react-native that referenced this pull request Feb 7, 2024
Summary:

Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of [PR#42758](facebook#42758).

From [PR#42758](facebook#42758)

> The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

> However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, this can still return null. however, the callsite should be cognizant of when this will happen. in the case of expomodules, the runtime should be ready when the module is init, unless it is a eager initialized module

Differential Revision: D53461821
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

philIip added a commit to philIip/react-native that referenced this pull request Feb 8, 2024
Summary:

Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of [PR#42758](facebook#42758).

From [PR#42758](facebook#42758)

> The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

> However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, this can still return null. however, the callsite should be cognizant of when this will happen. in the case of expomodules, the runtime should be ready when the module is init, unless it is a eager initialized module

Differential Revision: D53461821
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

Summary:

Changelog: [Android][Added] - introduce native api to access RuntimeExecutor

This is the android equivalent of [PR#42758](facebook#42758).

From [PR#42758](facebook#42758)

> The goal of this API is to provide a safe way to access the jsi::runtime in bridgeless mode. The decision to limit access to the runtime in bridgeless was a conscious one - the runtime pointer is not thread-safe and its lifecycle must be managed correctly by owners.

> However, interacting with the runtime is an advanced use case we would want to support. Our recommended ways to access the runtime in bridgeless mode is either 1) via the RuntimeExecutor, or 2) via a C++ TurboModule.

This diff introduces the API that would allow for 1). because react context can be non-null before react instance is ready, this can still return null. however, the callsite should be cognizant of when this will happen. in the case of expomodules, the runtime should be ready when the module is init, unless it is a eager initialized module

Differential Revision: D53461821
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53461821

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in d7dce97.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants