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

RFC: Well-defined event loop processing model #744

Merged

Conversation

rubennorte
Copy link
Contributor

@rubennorte rubennorte commented Dec 11, 2023

This is a proposal to change and formalize the steps that React Native performs to process and coordinate JavaScript tasks and rendering, aligning React Native closer to the Web specification.

A rendered version of the proposal can be read here

@rubennorte rubennorte marked this pull request as ready for review December 11, 2023 12:07
@acoates-ms
Copy link
Collaborator

Just checking my understanding of the background executor removal. Layout will now happen primarily on the js thread, except when a synchronous event or c++ state change forces an update on the ui thread, at which point the react render and the layout could still both happen on the ui thread.

@rubennorte
Copy link
Contributor Author

rubennorte commented Dec 14, 2023

Just checking my understanding of the background executor removal. Layout will now happen primarily on the js thread, except when a synchronous event or c++ state change forces an update on the ui thread, at which point the react render and the layout could still both happen on the ui thread.

@acoates-ms yes, your understanding is correct :)

@NickGerleman
Copy link

Just checking my understanding of the background executor removal.

Another nuance here is that out-of-the-box apps targeting the new architecture have defaulted to background executor being disabled already. So if you already have an app targeting Fabric, layout is already primarily happening on the JS thread, unless a feature flag is explicitly changed.

rubennorte added a commit to rubennorte/hermes that referenced this pull request Feb 28, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Differential Revision: D54302536
rubennorte added a commit to rubennorte/hermes that referenced this pull request Feb 28, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Differential Revision: D54302536
rubennorte added a commit to rubennorte/react-native that referenced this pull request Feb 28, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Differential Revision: D54302536
rubennorte added a commit to rubennorte/react-native that referenced this pull request Feb 28, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Differential Revision: D54302536
@rubennorte rubennorte merged commit 06ffe89 into react-native-community:main Feb 29, 2024
@rubennorte rubennorte deleted the proposal-event-loop branch February 29, 2024 22:27
rubennorte added a commit to rubennorte/react-native that referenced this pull request Mar 4, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536
rubennorte added a commit to rubennorte/hermes that referenced this pull request Mar 4, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536
rubennorte added a commit to rubennorte/hermes that referenced this pull request Mar 4, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536
facebook-github-bot pushed a commit to facebook/hermes that referenced this pull request Mar 4, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536

fbshipit-source-id: 25f52f91d7ef1a51687c431d2c7562c373dc72a5
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Mar 4, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536

fbshipit-source-id: 25f52f91d7ef1a51687c431d2c7562c373dc72a5
rubennorte added a commit to facebook/react that referenced this pull request Mar 13, 2024
… by RN (#28472)

## Summary

We want to enable the new event loop in React Native
(react-native-community/discussions-and-proposals#744)
for all users in the new architecture (determined by the use of
bridgeless, not by the use of Fabric). In order to leverage that, we
need to also set the flag for the React reconciler to use microtasks for
scheduling (so we'll execute them at the right time in the new event
loop).

This migrates from the previous approach using a dynamic flag (to be
used at Meta) with the check of a global set by React Native. The reason
for doing this is:
1) We still need to determine this dynamically in OSS (based on
Bridgeless, not on Fabric).
2) We still need the ability to configure the behavior at Meta, and for
internal build system reasons we cannot access the flag that enables
microtasks in
[`ReactNativeFeatureFlags`](https://github.com/facebook/react-native/blob/6c28c87c4d5d8a9f5be5e02cd7d3eba5b4aaca8c/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js#L121).

## How did you test this change?

Manually synchronized the changes to React Native and ran all tests for
the new architecture on it. Also tested manually.

> [!NOTE]
> This change depends on
facebook/react-native#43397 which has been
merged already
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
… by RN (facebook#28472)

## Summary

We want to enable the new event loop in React Native
(react-native-community/discussions-and-proposals#744)
for all users in the new architecture (determined by the use of
bridgeless, not by the use of Fabric). In order to leverage that, we
need to also set the flag for the React reconciler to use microtasks for
scheduling (so we'll execute them at the right time in the new event
loop).

This migrates from the previous approach using a dynamic flag (to be
used at Meta) with the check of a global set by React Native. The reason
for doing this is:
1) We still need to determine this dynamically in OSS (based on
Bridgeless, not on Fabric).
2) We still need the ability to configure the behavior at Meta, and for
internal build system reasons we cannot access the flag that enables
microtasks in
[`ReactNativeFeatureFlags`](https://github.com/facebook/react-native/blob/6c28c87c4d5d8a9f5be5e02cd7d3eba5b4aaca8c/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js#L121).

## How did you test this change?

Manually synchronized the changes to React Native and ran all tests for
the new architecture on it. Also tested manually.

> [!NOTE]
> This change depends on
facebook/react-native#43397 which has been
merged already
tungdo194 pushed a commit to tungdo194/rn-test that referenced this pull request Apr 28, 2024
Summary:
X-link: facebook/hermes#1331

Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536
facebook-github-bot pushed a commit to facebook/hermes that referenced this pull request May 6, 2024
Summary:
Original Author: rubennorte@meta.com
Original Git: de9dfe4
Original Reviewed By: neildhar
Original Revision: D54302536

Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: avp

Differential Revision: D56971229

fbshipit-source-id: 3321530b85847e62c804bfa472d8f009bb1cd66b
@shergin
Copy link

shergin commented May 22, 2024

I am certainly late for the party, but allow myself to ask a few Q just for prosperity:

  • Do I understand right that now we allow blocking layout on the JavaScript thread?
  • If so, do we do that all the time or only if some particular kind of useLayoutEffect is detected in Javascript code?
  • Have we thought about the performance implications it will have on RN? The main idea of RN (and pretty much all other modern UI frameworks) always was that layout, paint and other steps are non-blocking (out of JavaScript thread/queue).
  • From a practical standpoint, on most platforms relayout is initiated on the main/UI thread. If we need to block the UI thread on the JS thread, how this proposed solution is different from running Javascript on the main/UI thread?

@rubennorte
Copy link
Contributor Author

I am certainly late for the party, but allow myself to ask a few Q just for prosperity:

@shergin better late than never :P. Good to see you here!

  • Do I understand right that now we allow blocking layout on the JavaScript thread?

In the current iteration, layout happens on the JS thread (it was done on the Fabric background thread before but we moved it to JS and removed that thread) and it's synchronous.

  • If so, do we do that all the time or only if some particular kind of useLayoutEffect is detected in Javascript code?

Right now it's all the time but we can optimize later. For example, we could compute layout in a background thread and only block on it when you call a method that requires layout information. If I'm not mistaken, this is what modern browsers do.

  • Have we thought about the performance implications it will have on RN? The main idea of RN (and pretty much all other modern UI frameworks) always was that layout, paint and other steps are non-blocking (out of JavaScript thread/queue).

We tested it and the impact was relatively small (and we did some optimizations to compensate for it). I'm not sure how other frameworks do, but in browsers layout does happen on the main/JS thread in some situations.

  • From a practical standpoint, on most platforms relayout is initiated on the main/UI thread. If we need to block the UI thread on the JS thread, how this proposed solution is different from running Javascript on the main/UI thread?

If you mean re-layout as a result of window resize, for example, we can do that in the main thread. We run layout synchronously from where the commit was created, but that commit can be done in the main thread as well.

@shergin
Copy link

shergin commented May 24, 2024

Yeah, I checked the code and it looks like the layout is performed on the same thread/queue (here and later these words are used interchangeably) which initiates the changes in the Shadow Tree. In cases where it's called from JavaScript, it's done on the same thread.
However, the mutation of the tree can initiated by other parts of the system, and some of them are happening on the main thread (e.g. external relayout), and some of them might happen on some random thread.

IMO, there are a few problems with it.

  • If we want to have it as-is, it will lead to inconsistent behaviors (sometimes the reaction of relayour will be synchronous, sometimes it will not be). And, as far as I understand the actual goal is to remove the inconsistencies.
  • If we want to block on JS thread all the time we mutate Shadow tree, we will make all the callers actually slower. Currently (as it's implemented) we only block the JS thread for layout; it's usually a few dozen milliseconds on a regular screen without any complex embedded widgets, so it's not that bad. The logic we run on the Javascript thread usually relates to business stuff (not visual, like animations, image drawing, and so on), so small freezes there are not that visible. So, it's not that bad, we only lose some milliseconds that might be spent on React state reconciliation. But if we start blocking the main thread (and others) by JavaScript one, we will see drastic effects (and the rest of the app will suffer). Another terrifying example: imagine we need to run an animation that affects layout; if we need to block on JS thread on every frame, it will not be pretty. 😱

Elaborating on the second point:

  • Yes, browsers are designed this way and it's why they are not the snappiest thing on Earth. As you know, in browsers, accidental access to layout information from JS will make the whole website janky. Even with all the extremely tricky optimizations, it's super easy to accidentally make a website slow, so the product devs must be educated about those quirks.
  • Websites running in browsers are not interoperable with the rest of the OS (the only exception is external relayout). All the websites are running in their containers/threads which can be throttled down. In RN, all the screens are the single app that should be interoperable with the OS (inside and outside - embedded stuff inside, and embedding RN surface inside some other container).
  • Because of these constraints, we designed Fabric as we designed that. That's the reason that the tree is immutable, that's the reason why we have a concurrent, staged rendering pipeline. The goal is/was to draw it as fast as possible no matter what the source of change is, to make the UI snappy. If we are blocking everything on the JS thread, we can remove all this complexity (and give up all the idea of building a framework that can be fast).

I think to resolve that dilemma we need to clarify the priorities we have in mind when we work on the framework. All the technical decisions have side effects, approaches do conflict; without a clear idea of what we want, we will only add complexity, quirkiness, and technical misalignment.
If we believe that reimplementing web-browsers threading modal is the more important goal than the performance and responsiveness of UI, that's fine, but we need to make this decision explicit and share it with the community. This way nobody will complain that RN is not snappy enough; we will always say that we don't promise it. The product dev's decision about choosing a UI framework will also be more educated.
Another view on this: if we want to to design RN the same way as browsers design, what value are we adding? We already have great browsers, we can embed them and run React inside.

From a purely technical perspective, the bummer is that performance is the only feature that's impossible to retrospect. If we optimize for performance, we need to design for the performance from day one, it's a one-way-door decision. The beauty of RN is that it combines imperative with declarative: product code parts that can be declarative are declarative (component structure, styles, prop propagation...), and only parts that can't be expressed in a declarative manner (random business logic) are imperative. The trick is that we can be clever about declarative parts and optimize them well, but optimizing imperative parts is close to impossible. Layout is a classic problem that has sufficient declarative solutions, so I would try hard not to make it imperative.

In general, if we want to find an alignment with the web, I would try to do so by designing React primitives that would make sense for both/all platforms (a naive approach would be to make useLayoutEffect async on all platforms).

@rubennorte
Copy link
Contributor Author

Thanks for the feedback and the nuanced discussion, @shergin! There's a lot to unwrap there, so let me clarify a few things first:

  • One of the main goals of this change is to fix an important part of the React programming model that React Native didn't implement correctly. In this case, that's the semantics of useLayoutEffect. As defined by the React programming model (which all React implementations should follow), useLayoutEffect allows accessing layout information before painting on the screen. Any updates triggered from this hook are also processed synchronously. To fix this behavior we must 1) make layout information available when requested here and 2) block paint until we've fully processed these hooks, in case they trigger another render. To fix 1) we decided to compute layout synchronously in the JS thread, but we have other options in the future to optimize performance a little bit (as I mentioned earlier that browsers do).
  • Re: blocking everything on the JS thread: the only thing that useLayoutEffect blocks (and it's the same as on Web) is paint for changes coming as a result of an update in a React component. There isn't a guarantee that they've run because layout changed (it might not have changed) and it doesn't re-run when we have layout changes coming from other sources (e.g.: other components, or external relayouts like window resize). The API that allows us to do that on Web is ResizeObserver, which we plan to support in RN eventually. When we do, it might be reasonable to run the callbacks synchronously from the main thread when those external relayout happen, but it's something we still need to design.

This change doesn't change the fact that we can potentially commit changes in the Shadow Tree from other threads (without blocking on JS). In fact, we don't do it right now in RN (we always commit from the JS thread), but we're working on supporting it to improve APIs like IntersectionObserver.

If we believe that reimplementing web-browsers threading modal is the more important goal than the performance and responsiveness of UI, that's fine, but we need to make this decision explicit and share it with the community.

We're not reimplementing the threading model from browsers. We're only adopting the parts that make sense (mainly the parts that affect the semantics of the code) in the JS thread. Most importantly, what are the assumptions about when changes coming from the JS thread will be rendered in the screen. This decision has been done very explicitly in this RFC and we're already building on top of these foundations.

Aligning with the Web so we can increase code-sharing is definitely a goal for us now.

This way nobody will complain that RN is not snappy enough; we will always say that we don't promise it. The product dev's decision about choosing a UI framework will also be more educated.

I don't think this is fair or justified. What parts of this execution model do you think are intrinsically slow? The only place where we moved layout to a background thread was internally at Meta to unblock the rollout, but this behavior hasn't been enabled anywhere else and we haven't seen any issues with it.

In general, if we want to find an alignment with the web, I would try to do so by designing React primitives that would make sense for both/all platforms (a naive approach would be to make useLayoutEffect async on all platforms).

I think that's fair and AFAIK that's already our goal. The only problem is about useLayoutEffect specifically, where making it asynchronous would defeat the purpose of the hook itself. We already have an async version of useLayoutEffect, which us useEffect and it's recommended to be used over useLayoutEffect whenever possible.

In general, we want to bring APIs and semantics from Web to React Native, as long as they make sense for the React programming model. For example, we don't recommend making imperative mutations to the DOM on Web, and we'll probably never support that in React Native. But there are legit use cases to access the DOM for read access, and we've already started supporting that in RN (again, because it fits our model).

@shergin
Copy link

shergin commented May 30, 2024

This change doesn't change the fact that we can potentially commit changes in the Shadow Tree from other threads (without blocking on JS). In fact, we don't do it right now in RN (we always commit from the JS thread), but we're working on supporting it to improve APIs like IntersectionObserver.

This sentence highlights the misalignment the most. Here are the problems:

  • This change does change the assumptions around useLayoutEffect which is directly dictated by the threading/blocking model. The only way to consistently make useLayoutEffect work is to block all the Shadow Tree changes on the JS thread. (Otherwise, why would you want useLayoutEffect to be not called when some outside side-effect triggers relayout? If you want it to work consistently, you must to block that side effect on the JS thread.)
  • The current version of RN/Fabric already does have places where shadow three is mutated on the main/random threads. You have to make them block on the JS thread or your model will produce inconsistent results.
  • The performance will suffer if you want to block the JS thread for the relayout; the same way it suffers in browsers. That's the reason we don't use browsers on mobile for "native-like" interfaces and it's the reason why Electron sucks on desktop.

If we design the React model as you propose, we will prioritize some devex convenience (quite minor, IMO) over performance. IMO, this is a one-way decision that makes it impossible for RN to implement the rendering faster and snappier than embedded browsers, which itself will defeat the purpose of the existence of React Native.

So, I believe, if we make such a decision, we should be transparent about the goals, priorities, and side effects.

@rubennorte
Copy link
Contributor Author

This change does change the assumptions around useLayoutEffect which is directly dictated by the threading/blocking model

I think this is an important misconception. The assumptions around useLayoutEffect aren't dictated by the threading model, but by the React programming model. The threading model should support those assumptions, which is what this proposal is doing.

The only way to consistently make useLayoutEffect work is to block all the Shadow Tree changes on the JS thread. (Otherwise, why would you want useLayoutEffect to be not called when some outside side-effect triggers relayout? If you want it to work consistently, you must to block that side effect on the JS thread.)

The current version of RN/Fabric already does have places where shadow three is mutated on the main/random threads. You have to make them block on the JS thread or your model will produce inconsistent results.

The performance will suffer if you want to block the JS thread for the relayout; the same way it suffers in browsers. That's the reason we don't use browsers on mobile for "native-like" interfaces and it's the reason why Electron sucks on desktop.

I'm going to reply to all of these statements together because the answer is that same: that's not how useLayoutEffect works or how it's meant to work. Maybe the misconception is coming from the name, but please look at the definition of the hook. Layout effects don't run when layout changes. They ran when you have a state update in React, and after layout has been applied for those changes.

cipolleschi pushed a commit to facebook/hermes that referenced this pull request Jun 28, 2024
Summary:
Changelog: [internal]

## Context

Microtasks are an important aspect of JavaScript and they will become increasingly important in the hosts where we're currently using JSI.

For example, React Native is going to adopt an event loop processing model similar to the one on the Web, which means it would need the ability to schedule and execute microtasks in every iteration of the loop. See react-native-community/discussions-and-proposals#744 for details.

JSI already has a method to execute all pending microtasks (`drainMicrotasks`) but without a method to schedule microtasks this is incomplete.

We're currently testing microtasks with Hermes using an internal method to schedule microtasks (`HermesInternal.enqueueJob`) but we need a method in JSI so this also works in other runtimes like JSC and V8.

## Changes

This adds the `queueMicrotask` to the Runtime API in JSI so we have symmetric API for microtasks and we can implement the necessary functionality.

The expectation for JSI implementations is to queue microtasks from this method and from built-ins like Promises and async functions in the same queue, and not drain that queue until explicitly done via `drainMicrotasks` in JSI.

This also modifies Hermes and JSC to provide stubs for those methods, and the actual implementation will be done in following diffs.

Reviewed By: neildhar

Differential Revision: D54302536

fbshipit-source-id: 25f52f91d7ef1a51687c431d2c7562c373dc72a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants