Buffer async CallInvoker work with module calls - #58313
Open
javache wants to merge 1 commit into
Open
Conversation
Summary: In bridgeless, native reaches JS by two routes that end in the same `RuntimeScheduler` queue but get there differently. `callFunctionOnModule` goes through the instance's `BufferedRuntimeExecutor`; the `CallInvoker` goes straight to `scheduleTask`. The CallInvoker therefore skips the buffer entirely and can reach the runtime while a module call issued earlier is still parked, unflushed, because the bundle is mid-evaluation. Native code that issues both cannot rely on the order it issued them in, and `Task` is a min-heap on `now() + timeout(priority)` with no insertion tiebreak, so equal priorities do not settle it either. Gives the two channels the same buffering. `BufferedRuntimeExecutor` gains a priority-carrying `execute`, so work routed through it keeps the scheduler priority it was submitted with instead of collapsing to the executor default, and buffered work from both overloads stays in one submission-ordered stream. `BufferedCallInvoker` sits on that executor and becomes the bridgeless `jsCallInvoker` on Android, iOS and macOS. `invokeSync` deliberately keeps going straight to the scheduler: a synchronous call cannot wait for a flush that only happens once the bundle has run. Behind `enableBufferedCallInvoker`, default true. `ReactInstance` picks between the buffered invoker and the existing `RuntimeSchedulerCallInvoker` in one place, so the platform call sites are identical either way and the change is revertible at runtime — it moves when native-issued async work first reaches JS during startup, which is the intended contract but affects every native module. One lifetime hazard this surfaces, worth knowing about beyond this diff: `BufferedRuntimeExecutor` reaches the scheduler through a raw pointer captured at construction, which is safe only while the owning instance is alive. A CallInvoker is routinely held across instance teardown, so `BufferedCallInvoker` guards every async dispatch on a weak reference to the scheduler and drops the work when it has expired — the same contract `RuntimeSchedulerCallInvoker` has. Without that guard this reliably segfaults on a reload. Changelog: [General][Changed] - Async `CallInvoker` work is now buffered alongside callable module calls, so it no longer runs before the JS bundle has finished evaluating Differential Revision: D118456662
|
@javache has exported this pull request. If you are a Meta employee, you can view the originating Diff in D118456662. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
In bridgeless, native reaches JS by two routes that end in the same
RuntimeSchedulerqueue but get there differently.callFunctionOnModulegoesthrough the instance's
BufferedRuntimeExecutor; theCallInvokergoesstraight to
scheduleTask. The CallInvoker therefore skips the buffer entirelyand can reach the runtime while a module call issued earlier is still parked,
unflushed, because the bundle is mid-evaluation. Native code that issues both
cannot rely on the order it issued them in, and
Taskis a min-heap onnow() + timeout(priority)with no insertion tiebreak, so equal priorities donot settle it either.
Gives the two channels the same buffering.
BufferedRuntimeExecutorgains apriority-carrying
execute, so work routed through it keeps the schedulerpriority it was submitted with instead of collapsing to the executor default,
and buffered work from both overloads stays in one submission-ordered stream.
BufferedCallInvokersits on that executor and becomes the bridgelessjsCallInvokeron Android, iOS and macOS.invokeSyncdeliberately keeps going straight to the scheduler: a synchronouscall cannot wait for a flush that only happens once the bundle has run.
Behind
enableBufferedCallInvoker, default true.ReactInstancepicks betweenthe buffered invoker and the existing
RuntimeSchedulerCallInvokerin oneplace, so the platform call sites are identical either way and the change is
revertible at runtime — it moves when native-issued async work first reaches JS
during startup, which is the intended contract but affects every native module.
One lifetime hazard this surfaces, worth knowing about beyond this diff:
BufferedRuntimeExecutorreaches the scheduler through a raw pointer capturedat construction, which is safe only while the owning instance is alive. A
CallInvoker is routinely held across instance teardown, so
BufferedCallInvokerguards every async dispatch on a weak reference to the scheduler and drops the
work when it has expired — the same contract
RuntimeSchedulerCallInvokerhas.Without that guard this reliably segfaults on a reload.
Changelog:
[General][Changed] - Async
CallInvokerwork is now buffered alongside callable module calls, so it no longer runs before the JS bundle has finished evaluatingDifferential Revision: D118456662