From 4d9b11b45df0e27d44146561e4f3a5f5a5caec2f Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 12 Apr 2024 02:47:01 -0700 Subject: [PATCH] Introduce new State Alignment Mechanism (#44021) 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 --- .../react/renderer/core/ShadowNode.cpp | 8 +- .../react/renderer/core/ShadowNode.h | 10 ++- .../react/renderer/mounting/ShadowTree.cpp | 81 ++++++++++++++++--- .../tests/StateReconciliationTest.cpp | 17 ++-- 4 files changed, 90 insertions(+), 26 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index 126705685bc..e6f58b78b84 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -287,7 +287,11 @@ void ShadowNode::setMounted(bool mounted) const { family_->eventEmitter_->setEnabled(mounted); } -void ShadowNode::progressStateIfNecessary() { +bool ShadowNode::getHasBeenMounted() const { + return hasBeenMounted_; +} + +bool ShadowNode::progressStateIfNecessary() { if (!hasBeenMounted_ && state_) { ensureUnsealed(); auto mostRecentState = family_->getMostRecentStateIfObsolete(*state_); @@ -297,8 +301,10 @@ void ShadowNode::progressStateIfNecessary() { // Must call ComponentDescriptor::adopt to trigger any side effect // state may have. E.g. adjusting padding. componentDescriptor.adopt(*this); + return true; } } + return false; } const ShadowNodeFamily& ShadowNode::getFamily() const { diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h index 06b9d68f52e..b05831575f4 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h @@ -171,14 +171,22 @@ class ShadowNode : public Sealable, */ void setMounted(bool mounted) const; + /* + * Returns true if the shadow node has been marked as mounted before by + * calling `setMounted`. + */ + bool getHasBeenMounted() const; + /* * Applies the most recent state to the ShadowNode if following conditions are * met: * - ShadowNode has a state. * - ShadowNode has not been mounted before. * - ShadowNode's current state is obsolete. + * + * Returns true if the state was applied, false otherwise. */ - void progressStateIfNecessary(); + bool progressStateIfNecessary(); #pragma mark - DebugStringConvertible diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 1461f894b93..fd317991511 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -25,25 +25,50 @@ namespace facebook::react { using CommitStatus = ShadowTree::CommitStatus; using CommitMode = ShadowTree::CommitMode; -// --- Clone-less progress state algorithm --- +// --- State Alignment Mechanism algorithm --- // Note: Ideally, we don't have to const_cast but our use of constness in // C++ is overly restrictive. We do const_cast here but the only place where // we change ShadowNode is by calling `ShadowNode::progressStateIfNecessary` // where checks are in place to avoid manipulating a sealed ShadowNode. -static void progressStateIfNecessary(ShadowNode& newShadowNode) { - newShadowNode.progressStateIfNecessary(); +static void progressStateIfNecessary( + ShadowNode& newShadowNode, + const ShadowNode& baseShadowNode); - for (const auto& childNode : newShadowNode.getChildren()) { - progressStateIfNecessary(const_cast(*childNode)); +/* + * Looks at the new parent, new child and base child node to determine how to + * reconcile the state. + * + * Only to be called when baseChildNode has trait `ClonedByNativeStateUpdate`. + */ +static void progressStateIfNecessary( + ShadowNode& newShadowNode, + const ShadowNode& newChildNode, + const ShadowNode& baseChildNode, + size_t suggestedIndex) { + auto& shadowNode = const_cast(newChildNode); + if (shadowNode.progressStateIfNecessary()) { + // State was progressed without the need to clone. + // We are done with this node, but need to keep traversing. + progressStateIfNecessary(shadowNode, baseChildNode); + } else if (newChildNode.getHasBeenMounted()) { + // `newShadowNode` was cloned from react and cloned from a native state + // update. This child node was cloned only from a native state update. + // This is branching and it is safe to promote the new branch from + // native state update. + auto clonedChildNode = baseChildNode.clone({}); + newShadowNode.replaceChild(newChildNode, clonedChildNode, suggestedIndex); + } else { + // `newShadowNode` was cloned from react and cloned from a native state + // update. This child node was cloned also by react. + // we can't reason about this on this layer and need to keep traversing. + progressStateIfNecessary(shadowNode, baseChildNode); } } static void progressStateIfNecessary( ShadowNode& newShadowNode, const ShadowNode& baseShadowNode) { - newShadowNode.progressStateIfNecessary(); - auto& newChildren = newShadowNode.getChildren(); auto& baseChildren = baseShadowNode.getChildren(); @@ -63,21 +88,51 @@ static void progressStateIfNecessary( } if (!ShadowNode::sameFamily(newChildNode, baseChildNode)) { - // The nodes are not of the same family. Tree hierarchy has changed - // and we have to fall back to full sub-tree traversal from this point on. + // React has changed the structure of the tree. We will realign the + // structure below. break; } - progressStateIfNecessary( - const_cast(newChildNode), baseChildNode); + if (!baseChildNode.getTraits().check( + ShadowNodeTraits::Trait::ClonedByNativeStateUpdate)) { + // was not cloned with a new state, we can continue. + continue; + } + + progressStateIfNecessary(newShadowNode, newChildNode, baseChildNode, index); } + // === Realigning the tree === + + auto unprocessedBaseChildren = baseChildren.begin(); + std::advance(unprocessedBaseChildren, index); for (; index < newChildrenSize; ++index) { const auto& newChildNode = *newChildren[index]; - progressStateIfNecessary(const_cast(newChildNode)); + auto baseChildNodeIterator = std::find_if( + unprocessedBaseChildren, + baseChildren.end(), + [&newChildNode](auto baseChildNode) { + return ShadowNode::sameFamily(newChildNode, *baseChildNode); + }); + if (baseChildNodeIterator == baseChildren.end()) { + // This must never happen and there is a mismatch between the two trees. + // No way of recover from this, let's just continue. + continue; + } + + auto const& baseChildNode = *(*baseChildNodeIterator); + + if (!baseChildNode.getTraits().check( + ShadowNodeTraits::Trait::ClonedByNativeStateUpdate)) { + // was not cloned with a new state, we can continue. + continue; + } + + progressStateIfNecessary(newShadowNode, newChildNode, baseChildNode, index); } } -// --- End of Clone-less progress state algorithm --- + +// --- End of State Alignment Mechanism algorithm --- /* * Generates (possibly) a new tree where all nodes with non-obsolete `State` diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp index a25a60308f9..1ef49950a95 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp @@ -201,13 +201,11 @@ TEST_P(StateReconciliationTest, testStateReconciliation) { }, {.enableStateReconciliation = true}); - if (!GetParam()) { - EXPECT_EQ( - findDescendantNode(shadowTree, scrollViewFamily) - ->getState() - ->getRevision(), - state3->getRevision()); - } + EXPECT_EQ( + findDescendantNode(shadowTree, scrollViewFamily) + ->getState() + ->getRevision(), + state3->getRevision()); } TEST_P(StateReconciliationTest, testCloneslessStateReconciliationDoesntClone) { @@ -672,10 +670,7 @@ TEST_P(StateReconciliationTest, testScrollViewWithComplexChildrenReorder) { EXPECT_NE(findDescendantNode(shadowTree, childA->getFamily()), nullptr); EXPECT_NE(findDescendantNode(shadowTree, childB->getFamily()), nullptr); - if (!GetParam()) { - EXPECT_EQ( - findDescendantNode(shadowTree, childAFamily)->getState(), newState); - } + EXPECT_EQ(findDescendantNode(shadowTree, childAFamily)->getState(), newState); } TEST_P(StateReconciliationTest, testScrollViewWithChildrenReorder) {