Skip to content

Implement UI consistency mechanism for JS thread#43581

Closed
rubennorte wants to merge 1 commit into
facebook:mainfrom
rubennorte:export-D55024832
Closed

Implement UI consistency mechanism for JS thread#43581
rubennorte wants to merge 1 commit into
facebook:mainfrom
rubennorte:export-D55024832

Conversation

@rubennorte
Copy link
Copy Markdown
Contributor

@rubennorte rubennorte commented Mar 20, 2024

Summary:
Changelog: [internal]

This implements a mechanism to ensure that JavaScript tasks have a consistent view of the state of the UI during their execution.

Context

Fabric allows committing new revisions of the ShadowTree from any thread, but we don't make use of this capability and instead always commit them from the JS thread (e.g.: when we schedule Fabric state updates to update the offset of a list on scroll). This was done to make sure that JS work didn't see changes in the state of the tree at random points during its execution. E.g.:

useEffect(() => {
  const rect = ref.current.getBoundingClientRect();
  // do something
  const newRect = ref.current.getBoundingClientRect();
  // `rect` and `newRect` should always be the same
}, []);

This isn't used by Reanimated at the moment, which means JS can inadvertently see the result of animations in non-specific times during execution.

You can find additional context about this in the RFC for DOM Traversal & Layout APIs in RN.

This works correctly at the moment, but we introduce a limitation in the execution model to prevent updating the tree synchronously from the main thread. One of the main problems this introduces is that computing intersections (for IntersectionObserver) relies on the information in the shadow tree, but this is updated asynchronously on scroll.

There are 2 potential solutions for that problem:

  1. Send the timestamp of the scroll even with the state update to backdate the timestamps of the intersections. This could work but introduces more complexity and possibly accuracy problems due to batching those state updates with other changes (e.g.: what happens if we update the state and commit another tree in the same task? should we use the backdated timestamp or wait for mount?).
  2. (Preferred/ this diff) Allow committing new revisions from any thread, but lock the JS thread into seeing a specific revision, which would only update/progress in specific moments when it's safe. Some of those moments would be:
    1. When we start a new JS task.
    2. When we commit a new tree from React (JS).

Changes

This implements the solution outlined in 2), creating a few abstractions to handle what's the current tree that should be visible to JS and to lock/unlock it in specific moments.

More specifically:

  • Creates ShadowTreeRevisionProvider as an abstract class for APIs consuming the visible revision of the ShadowTree (mainly DOM APIs and layout methods like measure, etc.).
  • Creates ShadowTreeRevisionConsistencyManager as an abstract class to handle what trees are visible (with a lockRevision and unlockRevision to be called from RuntimeScheduler at the beginning and end of each JS task).
  • Creates 2 different implementations of these abstractions:
    • One that preserves the current behavior (LatestShadowTreeRevisionProvider, which just returns the last committed revision at the time of the call).
    • One that locks revisions lazily (the first time they're accessed) (LazyShadowTreeRevisionConsistencyManager).

Differential Revision: D55024832

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Mar 20, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Mar 21, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,433,905 +262,557
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,803,148 +262,666
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 274faf4
Branch: main

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

Summary:
Pull Request resolved: facebook#43581

Changelog: [internal]

This implements a mechanism to ensure that JavaScript tasks have a consistent view of the state of the UI during their execution.

## Context

Fabric allows committing new revisions of the ShadowTree from any thread, but we don't make use of this capability and instead always commit them from the JS thread (e.g.: when we schedule Fabric state updates to update the offset of a list on scroll). This was done to make sure that JS work didn't see changes in the state of the tree at random points during its execution. E.g.:

```
useEffect(() => {
  const rect = ref.current.getBoundingClientRect();
  // do something
  const newRect = ref.current.getBoundingClientRect();
  // `rect` and `newRect` should always be the same
}, []);
```

This isn't used by Reanimated at the moment, which means JS can inadvertently see the result of animations in non-specific times during execution.

You can find additional context about this in the [RFC for DOM Traversal & Layout APIs in RN](https://github.com/react-native-community/discussions-and-proposals/blob/main/proposals/0607-dom-traversal-and-layout-apis.md#consistency-and-updates).

This works correctly at the moment, but we introduce a limitation in the execution model to prevent updating the tree synchronously from the main thread. One of the main problems this introduces is that computing intersections (for `IntersectionObserver`) relies on the information in the shadow tree, but this is updated asynchronously on scroll.

There are 2 potential solutions for that problem:
1) Send the timestamp of the scroll even with the state update to backdate the timestamps of the intersections. This could work but introduces more complexity and possibly accuracy problems due to batching those state updates with other changes (e.g.: what happens if we update the state and commit another tree in the same task? should we use the backdated timestamp or wait for mount?).
2) (**Preferred**/ this diff) Allow committing new revisions from any thread, but lock the JS thread into seeing a specific revision, which would only update/progress in specific moments when it's safe. Some of those moments would be:
    1) When we start a new JS task.
    2) When we commit a new tree from React (JS).

## Changes

This implements the solution outlined in 2), creating a few abstractions to handle what's the current tree that should be visible to JS and to lock/unlock it in specific moments.

More specifically:
* Creates `ShadowTreeRevisionProvider` as an abstract class for APIs consuming the visible revision of the ShadowTree (mainly DOM APIs and layout methods like `measure`, etc.).
* Creates `ShadowTreeRevisionConsistencyManager` as an abstract class to handle what trees are visible (with a `lockRevision` and `unlockRevision` to be called from `RuntimeScheduler` at the beginning and end of each JS task).
* Creates 2 different implementations of these abstractions:
  * One that preserves the current behavior (`LatestShadowTreeRevisionProvider`, which just returns the last committed revision at the time of the call).
  * One that locks revisions lazily (the first time they're accessed) (`LazyShadowTreeRevisionConsistencyManager`).

Reviewed By: sammy-SC

Differential Revision: D55024832
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 25, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in 332355e.

facebook-github-bot pushed a commit that referenced this pull request Dec 2, 2024
Summary:
Running app with static linking fails to compile. Probably during #43581 adding that code was overlooked since the analogous thing seems to be added: https://github.com/facebook/react-native/pull/43581/files#diff-6680f6849631e3dcc5897ee3961a9d6d2bc57aff3eccb79a9d9c634183276202R566.

cc rubennorte since you made the linked PR.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[IOS] [FIXED] - Add missing subpsec to React-Fabric podspec.

Pull Request resolved: #48023

Test Plan: Run https://github.com/WoLewicki/reproducer-react-native/tree/%40wolewicki/static-linking-with-live-markdown and see that it won't compile without this change.

Reviewed By: cipolleschi

Differential Revision: D66600486

Pulled By: cortinico

fbshipit-source-id: 9de64541e49d27cdf00f37942adab6620476f15a
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.

3 participants