Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44098
This sets `QualiferAlignment` so that code is automatically formatted to west const.
I did a pass at this before, but now that we are on new Clang Format, we can enforce it automatically, and I think a couple more cases not previously changed now are.
Changelog: [Internal]
Reviewed By: christophpurrer
Differential Revision: D56143678
fbshipit-source-id: 8f12b288476ea6019fd7d7a93a39b4fe2e75af14
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44058
## Changelog:
[Internal] -
This was revealed when running an ASAN build on MacOS - we were doing an unsafe downcast from `LayoutableShadowNode->ViewShadowNode` inside `ShadowTree::emitLayoutEvents`.
Which, even though incidentally worked, is generally unsafe, as we may get e.g. `ImageShadowNode` there, which doesn't inherit from `ViewShadowNode`.
That downcast to `ViewShadowNode` wasn't even required, to begin with, as all the needed information can be already extracted from the `LayoutableShadowNode` itself.
Reviewed By: christophpurrer, javache
Differential Revision: D56062334
fbshipit-source-id: 08d5b3f5e0c57dc51b051d23506c7933581fea29
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44022
changelog: [internal]
move the new state reconciliation algorithm to the unified feature flag system.
Reviewed By: rubennorte
Differential Revision: D55965530
fbshipit-source-id: 3edde0858a670e86dc2d1cb561f03f584ff21896
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44021
changelog: [internal]
This is an evolution of cloneless state progression, introduced in D49012353.
# Problem
## When React clones the wrong node revision
Whenever React wants to commit a new change, it first needs to clone shadow nodes. React sometimes clones from the wrong revision. This has mostly been fine, Fabric does state reconciliation to pass newest state forward. State reconciliation is needed, as we need to keep native state in the shadow tree.
However, when React clones a node that has never been through layout step, it will clone a node without any layout information and its yoga node is dirtied. Even though there might be a subsequent revision of the node with layout information already calculated. As a result, Yoga needs to traverse bigger parts of the tree, even though layout has been calculated before. It is just cached on a different revision that was used as a source.
There are two main sources (there is more but they don't help to paint the picture) when this can happen. Background Executor and State Progression. Let's start with the simpler one but less severe: Background Executor.
Background Executor moves layout from JavaScript thread. React can start cloning nodes right away, even though they might not have layout information calculated yet. This is a race condition and depending on when the node is cloned, we can see different results. In this case, React eventually clones node from the correct revision with the layout cache. It will be in a correct state in the end. This case is not as bad as far as I can tell but I included it here because it better illustrates what is going on.
State Progression is where things get worse. In this scenario, React will never clone from the correct revision and will never recover from this. Anytime React clones node with a state that needs to be progressed, it will get cloned one more time during commit but React will hold the wrong revision. Depending on where this node is located in the view hierarchy, it may lead to expensive layout calculations.
Example:
Let's use notation A/r1 as node of family A revision 1.
- React calls create node. Node A/r1 is created and React holds reference to this. It will later use it to clone it.
Node A has native state that was updated. New revision A/r2 is created. Now React and RN do not observe the same node anymore (this is sometimes necessary).
- React now clones node A to create A/r3. This revision may have the wrong yoga cache. Now this might sound like one off but let's explore what happens next.
- During commit, Fabric must do state progression to give node A/r3 state from A/r2. This requires cloning and new revision A/r4 is created. React has again a wrong node that does not have Yoga cache and can't recover from this state.
The blast radius of this varies depending on where in the tree the node is.
# Solution - State Alignment Mechanism
The main principle for new state progression is to make sure React references the correct shadow node after commit to avoid layout cache miss on subsequent commit.
Agenda for the diagrams below:
- Black colour: node was not cloned.
- Blue colour: node was cloned by React.
- Orange colour: node was cloned by host platform.
- Blue and Orange colour: node was cloned by both React and host platform.
## Simple cases
### Base case
{F1483309510}
### React Cloned
{F1483308354}
### React and host platform clone the same node
{F1483309324}
## Medium difficulty
### React clones a different branch than host platform
{F1483349393}
### React deletes a branch that was cloned by host platform
{F1483349259}
### React changes structure of the tree, node cloned by host platform remains
{F1483349758}
### React reorders nodes that were cloned by host platform
{F1483350283}
Reviewed By: rubennorte
Differential Revision: D53405702
fbshipit-source-id: c7d4b0772c144c86d72e39965e9626a2daefa6fd
Summary:
## Stack
These can suss out some real bugs, and helps further avoid mismatch with downstream MSVC on /W4 as used by MSFT.
I enabled the families of warnings, but suppressed some major individual warnings that weren't clean. But I did clean some up, notably, missing initializer, and shortening 64 bit to 32 bit. We can do some of the rest incrementally (e.g. `-Wunused-parameter` has a fixit).
This change illuminates that MapBuffer is missing 64 bit integer support, but we often pass 64 bit counters to it, which is a bug. For now I just left TODOs around those.
`rn_xplat_cxx_library` is used for external libraries interfacing with RN, which we probably don't want to police, so I structured these stricter warnings as an opt-in flag, only enabled for our own rules.
## Diff
This fixes up source code to avoid emitting the extra warnings now enforced. Of what is enabled, this is mostly shortening 64 to 32, or missing field in initializer.
Changelog: [Internal]
Reviewed By: christophpurrer
Differential Revision: D52589303
fbshipit-source-id: 11cb778d065799fd0ead3ae706934146d13500bb
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39795
X-link: https://github.com/facebook/yoga/pull/1412
Android NDK 25 uses a version of libc++ that is more than three years old, missing a lot of basic features of C++ 20. This is rectified in NDK 26 (latest LTS NDK), which brings us up to date with latest Clang (17, released this year), and adds a new policy where future NDK versions will bump libc++ as part of bumping LLVM/Clang.
This requires an a beta AGP version (and corresponding Android Studio Preview). Based on how far we are historically, it wouldn't be a surprise if we see the stable release this month (well before the RN 0.74/Yoga 3.0 cut, even in the worse case).
Changelog:
[Android][Changed] - Use NDK 26
Reviewed By: yungsters
Differential Revision: D49895949
fbshipit-source-id: 37bb4d1fdf81137be7f14f6675b4e079c6f861e4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39357
changelog: [internal]
# Problem
## When React clones the wrong node revision
Whenever React wants to commit a new change, it first needs to clone shadow nodes. React sometimes clones from the wrong revision. This has mostly been fine, Fabric does state reconciliation to pass newest state forward. State reconciliation is needed, as we need to keep native state in the shadow tree.
However, when React clones a node that has never been through layout step, it will clone a node without any layout information and its yoga node is dirtied. Even though there might be a subsequent revision of the node with layout information already calculated. As a result, Yoga needs to traverse bigger parts of the tree, even though layout has been calculated before. It is just cached on a different revision that was used as a source.
There are two main sources (there is more but they don't help to paint the picture) when this can happen. Background Executor and State Progression. Let's start with the simpler one but less severe: Background Executor.
Background Executor moves layout from JavaScript thread. React can start cloning nodes right away, even though they might not have layout information calculated yet. This is a race condition and depending on when the node is cloned, we can see different results. In this case, React eventually clones node from the correct revision with the layout cache. It will be in a correct state in the end. This case is not as bad as far as I can tell but I included it here because it better illustrates what is going on.
State Progression is where things get worse. In this scenario, React will never clone from the correct revision and will never recover from this. Anytime React clones node with a state that needs to be progressed, it will get cloned one more time during commit but React will hold the wrong revision. Depending on where this node is located in the view hierarchy, it may lead to expensive layout calculations.
Example:
Let's use notation A/r1 as node of family A revision 1.
- React calls create node. Node A/r1 is created and React holds reference to this. It will later use it to clone it.
Node A has native state that was updated. New revision A/r2 is created. Now React and RN do not observe the same node anymore (this is sometimes necessary).
- React now clones node A to create A/r3. This revision may have the wrong yoga cache. Now this might sound like one off but let's explore what happens next.
- During commit, Fabric must do state progression to give node A/r3 state from A/r2. This requires cloning and new revision A/r4 is created. React has again a wrong node that does not have Yoga cache and can't recover from this state.
The blast radius of this varies depending on where in the tree the node is.
# Solution
Clone-less state progression. In this algorithm, state progression never clones. It checks if a ShadowNode has ever been mounted via `ShadowNode::hasBeenMounted_` to check if a node can be safely mutated without the need to clone and checks if current state the node is holding is obsolete (like the previous algorithm).
The clone-less state progression depends on the fact that any shadow node cloned from React is still unsealed and is not exposed to a multithreaded environment. This makes it safe to mutate it in place, without the need to clone.
WARNING: there is important semantic difference with the approach. With the old algorithm, you couldn't go back to a shadow node with old state. New state was always enforced when state reconciliation was enabled. The clone-less algorithm does not support that, because it can't mutate such a node in place.
Reviewed By: javache
Differential Revision: D49012353
fbshipit-source-id: 2d322f0587042c13761c83f7e916d1623159773d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39355
changelog: [internal]
We want to seal node just before it is promoted.
Reviewed By: javache, NickGerleman
Differential Revision: D49054011
fbshipit-source-id: 92550c13280c68bf66d505872d86d775cf115f5d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39205
This diff adds affected layoutable nodes count from Yoga in the telemetry for commit revision. This is used for performance monitoring next to the timestamp information we already have.
Changelog:
[Internal] - Add affected layoutable nodes count information in Fabric commit telemetry.
Reviewed By: rshest
Differential Revision: D48671209
fbshipit-source-id: b054e5dcd465122f01500fb54cba6f6d250cd256
Summary:
This changes Clang format config to enforce left pointer alignment instead of right, in accordance with https://www.internalfb.com/intern/wiki/Cpp/CppStyle/
Changelog: [Internal]
bypass-github-export-checks
Reviewed By: sammy-SC
Differential Revision: D48952040
fbshipit-source-id: 108329b2f11d2041a31dee3334c7801d69a3f1ad
Summary:
(reland of D48761722)
Pull Request resolved: https://github.com/facebook/react-native/pull/39217
React Native uses an inconsistent mix of "west const" and "east const". E.g. `const auto &` in 74 files, but `auto const &` in 60. Sometimes they are mixed from one line to the next: {F1079102436}
Clang format 14 adds a QualifierAlignment option, but fbsource is still on 12, so we cannot use it in our config until the [world is updated]()https://fb.workplace.com/groups/toolchain.fndn/posts/24006558685624673/?comment_id=24009214565359085&reply_comment_id=24009455088668366. This diff just runs a local version of Clang format locally first to fix QualifierAlignment, then reformats with the fbsource version, to fix any other output differences unrelated to that. This will not continually enforce a style, but will make the world more consistent, and hopefully encourage a consistent style until we can set it.
West const seems more popular in `//xplat` so I just picked left alignment somewhat arbitrarily, but we could also maybe take a poll on this.
Changelog: [Internal]
bypass-github-export-checks
Reviewed By: shwanton
Differential Revision: D48852450
fbshipit-source-id: 1789aa0db43948169f482188cb8b5e8f0f0246b8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38706
Fabric has a mechanism to resolve conflicts when there are race conditions committing new versions in different threads. This mechanism forces commits to be done in order, retrying commits when they didn't have time to finish before another concurrent commit.
{F1061612667}
This mechanism has an important drawback. If we have a continuous stream of short commits (e.g.: animations via Reanimated) and we want to do a large commit (e.g.: coming from React), the large commit could exhaust all attempts to finish and ultimately fail. Every time we tried to commit the large one, another small one would have had a chance to finish, forcing another retry.
{F1061614717}
In most of the cases where this happens, we didn't even need to fail the commits in the first place!
**The only reason why we retry commits is so we make sure we are propagating the last state** (when state propagation is enabled in that commit). We don't reconcile the contents of the commits themselves (the tree we retry is exactly the same, if we exclude state).
We can reduce the number of reconciliation failures (and completely remove them in the case of animations) if instead of checking if the base revision for a commit changed, we checked if the version of the state we propagated changed.
We would have 3 scenarios when doing that:
1. Commit creating state vs. commit reusing state. If the commit creating the state went first, we would have to retry the commit reusing the state (to make sure we're reusing the new state). If the commit reusing the state went first, we wouldn't have conflicts, and we would just apply the new state on top of it. {F1061617730}
2. Commit reusing state vs. another commit reusing state. In this case the order doesn't matter and we don't need to trigger a conflict. Commits would be updated in the order in which they are applied. {F1061618406}
3. Commit creating state vs. commit creating state. In this case, we are not propagating state, so retrying the commits wouldn't do anything. We can ignore the conflicts in this case too. {F1061618689}
Changelog: [internal]
Reviewed By: sammy-SC
Differential Revision: D47915384
fbshipit-source-id: bb4510c59bf32ca342c02f3bdb7029b545f56d99
Co-authored-by: Samuel Susla <samuel.susla@gmail.com>
Co-authored-by: David Vacca
Co-authored-by: Tomek Zawadzki <tomasz.zawadzki@swmansion.com>
Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
Summary:
Hello! This PR is a fix for one merged some time ago (https://github.com/facebook/react-native/pull/36216). In the PR check for `nullptr` value of `newRootShadowNode` just after performing commit hooks was overlooked. This PR restores previous behaviour of conditional commit cancellation after commit hook execution.
## Changelog:
[INTERNAL] [FIXED] - Restore checking shadow tree commit cancellation after commit hook execution
Pull Request resolved: https://github.com/facebook/react-native/pull/38715
Test Plan: Just register a commit hook that return `nullptr`. In that case current code crashes due to `nullptr` dereference.
Reviewed By: sammy-SC
Differential Revision: D47972245
Pulled By: ryancat
fbshipit-source-id: 7599ad11ed4b2dcaf25e53f676ec4530e37410d5
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37932
`static_cast<Derived &>(*sharedBase)` is preferred over `*std::static_pointer_cast<Derived>(sharedBase)`, since we don't need to copy the underlying shared_ptr to do so. The same applies for `std::const_pointer_cast`.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D46766558
fbshipit-source-id: 5d0b660107b2a60340952e2b5ec2792e3ed1832a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37553
## Context
I added some getters in EventTarget/EventEmitter to access the React instance handle for a given shadow node/shadow node family in https://github.com/facebook/react-native/commit/43864a1d6025bfc4483dded115d51f7613018199 (D44022477), so I could implement DOM traversal methods easily.
I wanted to reuse that method to implement `MutationObserver` and `IntersectionObserver`, but unfortunately `EventEmitter`/`EventTarget` only allow access to the instance handle as long as the shadow node is mounted. That makes sense for events, but not for this use case, as with `MutationObserver` we intentionally want to have access to unmounted nodes when creating the list of `addedNodes` and `removedNodes` for the `MutationRecord` (depending on when exactly we generate this list, either `addedNodes` or `removedNodes` wouldn't be mounted).
## Changes
This reverts my original change and adds a new field in `ShadowNodeFamily` to provide access to the `InstanceHandle` at any point (even if the node isn't mounted).
This provides the same guarantees as the original method, keeping weak references to the handles to avoid memory leaks.
Changelog: [internal]
Reviewed By: sammy-SC
Differential Revision: D46149084
fbshipit-source-id: f76abae50134a5d55a98cab42eebeb62084024f9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37609
changelog: [internal]
Moving LayoutMetrics and LayoutPrimitives from core to graphics module.
This is to enable different implementation for different platforms.
Reviewed By: rubennorte
Differential Revision: D45904748
fbshipit-source-id: a4e666d7c7390e87abdb09235f96655b63f451f9
Nick Gerleman