From 2e2e5f1fc976f15b781a6fe52643b721a3a6de49 Mon Sep 17 00:00:00 2001 From: Shubh Porwal Date: Sat, 26 Sep 2026 13:21:00 +0530 Subject: [PATCH 1/3] Avoid copying the children list twice when cloning a path to the root `ShadowNode::cloneTree` and `UIManager::updateShadowTree` copy the parent's children into a local vector, replace the updated child, and then copy that local again into the `std::make_shared` passed to `clone`. The second copy bumps and drops the refcount of every sibling. The local is dead after the call, so move it instead. `addAncestorsToUpdateList` also copied the whole children vector just to read a single element; take it by reference. Added `cloneTree` coverage to `ShadowNodeTest`. Changelog: [General][Changed] - Avoid a redundant copy of the children list in `ShadowNode::cloneTree` and `UIManager::updateShadowTree` Co-Authored-By: Claude Opus 5.5 (1M context) --- .../react/renderer/core/ShadowNode.cpp | 2 +- .../renderer/core/tests/ShadowNodeTest.cpp | 43 +++++++++++++++++++ .../uimanager/UIManagerUpdateShadowTree.cpp | 4 +- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index c147f3707356..5ed77b7b1e81 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -411,7 +411,7 @@ std::shared_ptr ShadowNode::cloneTree( childNode = parentNode.clone( {.children = std::make_shared>>( - children)}); + std::move(children))}); } return std::const_pointer_cast(childNode); diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp index cc53b101e115..abb64a5d47e6 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp @@ -429,3 +429,46 @@ TEST_F(ShadowNodeTest, cloneMultipleWithEmptyFamilySet) { EXPECT_EQ(result, nullptr); } + +TEST_F(ShadowNodeTest, cloneTree) { + auto newProps = std::make_shared(); + auto newRoot = nodeA_->cloneTree( + nodeABB_->getFamily(), [&](const ShadowNode& oldShadowNode) { + return oldShadowNode.clone({.props = newProps}); + }); + + EXPECT_EQ(newRoot->getTag(), nodeA_->getTag()); + EXPECT_NE(newRoot.get(), nodeA_.get()); + EXPECT_EQ(newRoot->getChildren().size(), 3); + EXPECT_EQ(newRoot->getChildren()[0]->getTag(), nodeAA_->getTag()); + EXPECT_EQ(newRoot->getChildren()[0]->getProps(), nodeAA_->getProps()); + EXPECT_EQ(newRoot->getChildren()[2]->getTag(), nodeAC_->getTag()); + EXPECT_EQ(newRoot->getChildren()[2]->getProps(), nodeAC_->getProps()); + + auto newNodeAB = newRoot->getChildren()[1]; + EXPECT_EQ(newNodeAB->getTag(), nodeAB_->getTag()); + EXPECT_NE(newNodeAB.get(), nodeAB_.get()); + EXPECT_EQ(newNodeAB->getChildren().size(), 2); + EXPECT_EQ(newNodeAB->getChildren()[0]->getTag(), nodeABA_->getTag()); + EXPECT_EQ(newNodeAB->getChildren()[0]->getProps(), nodeABA_->getProps()); + + auto newNodeABB = newNodeAB->getChildren()[1]; + EXPECT_EQ(newNodeABB->getTag(), nodeABB_->getTag()); + EXPECT_NE(newNodeABB.get(), nodeABB_.get()); + EXPECT_EQ(newNodeABB->getProps(), newProps); + + // The original tree is left untouched. + EXPECT_EQ(nodeA_->getChildren().size(), 3); + EXPECT_EQ(nodeA_->getChildren()[1].get(), nodeAB_.get()); + EXPECT_EQ(nodeAB_->getChildren().size(), 2); + EXPECT_EQ(nodeAB_->getChildren()[1].get(), nodeABB_.get()); +} + +TEST_F(ShadowNodeTest, cloneTreeReturnsNullptrWhenFamilyHasNoPathToRoot) { + auto result = nodeA_->cloneTree( + nodeZ_->getFamily(), [&](const ShadowNode& oldShadowNode) { + return oldShadowNode.clone({}); + }); + + EXPECT_EQ(result, nullptr); +} diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp index c691d5a897ca..d08abe3a4873 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp @@ -44,7 +44,7 @@ void addAncestorsToUpdateList( while (!currentShadowNode->getChildren().empty() && currentShadowNode->getTag() != shadowNode->getTag()) { ancestorShadowNodesShared[ancestorIndex] = currentShadowNode; - auto children = currentShadowNode->getChildren(); + const auto& children = currentShadowNode->getChildren(); auto childIndex = ancestors[ancestorIndex].second; currentShadowNode = children[childIndex]; ancestorIndex++; @@ -201,7 +201,7 @@ void UIManager::updateShadowTree( auto cloned = oldShadowNode->clone( {.props = newProps, .children = std::make_shared< - std::vector>>(children)}); + std::vector>>(std::move(children))}); clonedShadowNodes.insert({oldShadowNode->getTag(), std::move(cloned)}); } else { LOG(ERROR) << "oldShadowNode is null"; From d97c664607ce0e84006f917ef6e874bdb5cf3560 Mon Sep 17 00:00:00 2001 From: Shubh Porwal Date: Sat, 26 Sep 2026 13:30:20 +0530 Subject: [PATCH 2/3] Apply clang-format Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp | 5 ++--- .../react/renderer/uimanager/UIManagerUpdateShadowTree.cpp | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp index abb64a5d47e6..2eba9ac1e0cf 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp @@ -466,9 +466,8 @@ TEST_F(ShadowNodeTest, cloneTree) { TEST_F(ShadowNodeTest, cloneTreeReturnsNullptrWhenFamilyHasNoPathToRoot) { auto result = nodeA_->cloneTree( - nodeZ_->getFamily(), [&](const ShadowNode& oldShadowNode) { - return oldShadowNode.clone({}); - }); + nodeZ_->getFamily(), + [&](const ShadowNode& oldShadowNode) { return oldShadowNode.clone({}); }); EXPECT_EQ(result, nullptr); } diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp index d08abe3a4873..8fb2e50ad313 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerUpdateShadowTree.cpp @@ -201,7 +201,8 @@ void UIManager::updateShadowTree( auto cloned = oldShadowNode->clone( {.props = newProps, .children = std::make_shared< - std::vector>>(std::move(children))}); + std::vector>>( + std::move(children))}); clonedShadowNodes.insert({oldShadowNode->getTag(), std::move(cloned)}); } else { LOG(ERROR) << "oldShadowNode is null"; From 96dfd1b172ada124dfa20fc211c73df6ff29e8a4 Mon Sep 17 00:00:00 2001 From: Shubh Porwal Date: Sat, 26 Sep 2026 13:34:00 +0530 Subject: [PATCH 3/3] Drop unrelated cloneTree test Co-Authored-By: Claude Opus 5.5 (1M context) --- .../react/renderer/core/tests/ShadowNodeTest.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp index 2eba9ac1e0cf..9b627c113a8e 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp @@ -463,11 +463,3 @@ TEST_F(ShadowNodeTest, cloneTree) { EXPECT_EQ(nodeAB_->getChildren().size(), 2); EXPECT_EQ(nodeAB_->getChildren()[1].get(), nodeABB_.get()); } - -TEST_F(ShadowNodeTest, cloneTreeReturnsNullptrWhenFamilyHasNoPathToRoot) { - auto result = nodeA_->cloneTree( - nodeZ_->getFamily(), - [&](const ShadowNode& oldShadowNode) { return oldShadowNode.clone({}); }); - - EXPECT_EQ(result, nullptr); -}