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

Clone free state progression #39357

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,20 @@ void ShadowNode::setMounted(bool mounted) const {
family_->eventEmitter_->setEnabled(mounted);
}

void ShadowNode::progressStateIfNecessary() {
if (!hasBeenMounted_ && state_) {
ensureUnsealed();
auto mostRecentState = family_->getMostRecentStateIfObsolete(*state_);
if (mostRecentState) {
state_ = mostRecentState;
const auto& componentDescriptor = family_->componentDescriptor_;
// Must call ComponentDescriptor::adopt to trigger any side effect
// state may have. E.g. adjusting padding.
componentDescriptor.adopt(*this);
}
}
}

const ShadowNodeFamily& ShadowNode::getFamily() const {
return *family_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ class ShadowNode : public Sealable,
*/
void setMounted(bool mounted) 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.
*/
void progressStateIfNecessary();

#pragma mark - DebugStringConvertible

#if RN_DEBUG_STRING_CONVERTIBLE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,60 @@ namespace facebook::react {
using CommitStatus = ShadowTree::CommitStatus;
using CommitMode = ShadowTree::CommitMode;

// --- Clone-less progress state 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();

for (const auto& childNode : newShadowNode.getChildren()) {
progressStateIfNecessary(const_cast<ShadowNode&>(*childNode));
}
}

static void progressStateIfNecessary(
ShadowNode& newShadowNode,
const ShadowNode& baseShadowNode) {
newShadowNode.progressStateIfNecessary();

auto& newChildren = newShadowNode.getChildren();
auto& baseChildren = baseShadowNode.getChildren();

auto newChildrenSize = newChildren.size();
auto baseChildrenSize = baseChildren.size();
auto index = size_t{0};

for (index = 0; index < newChildrenSize && index < baseChildrenSize;
++index) {
const auto& newChildNode = *newChildren[index];
const auto& baseChildNode = *baseChildren[index];

if (&newChildNode == &baseChildNode) {
// Nodes are identical. They are shared between `newShadowNode` and
// `baseShadowNode` and it is safe to skipping.
continue;
}

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.
break;
}

progressStateIfNecessary(
const_cast<ShadowNode&>(newChildNode), baseChildNode);
}

for (; index < newChildrenSize; ++index) {
const auto& newChildNode = *newChildren[index];
progressStateIfNecessary(const_cast<ShadowNode&>(newChildNode));
}
}
// --- End of Clone-less progress state algorithm ---

/*
* Generates (possibly) a new tree where all nodes with non-obsolete `State`
* objects. If all `State` objects in the tree are not obsolete for the moment
Expand Down Expand Up @@ -344,11 +398,15 @@ CommitStatus ShadowTree::tryCommit(
}

if (commitOptions.enableStateReconciliation) {
auto updatedNewRootShadowNode =
progressState(*newRootShadowNode, *oldRootShadowNode);
if (updatedNewRootShadowNode) {
newRootShadowNode =
std::static_pointer_cast<RootShadowNode>(updatedNewRootShadowNode);
if (CoreFeatures::enableClonelessStateProgression) {
progressStateIfNecessary(*newRootShadowNode, *oldRootShadowNode);
} else {
auto updatedNewRootShadowNode =
progressState(*newRootShadowNode, *oldRootShadowNode);
if (updatedNewRootShadowNode) {
newRootShadowNode =
std::static_pointer_cast<RootShadowNode>(updatedNewRootShadowNode);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,45 +62,35 @@ const ShadowNode* findDescendantNode(
}
} // namespace

TEST(StateReconciliationTest, testStateReconciliation) {
auto builder = simpleComponentBuilder();
class StateReconciliationTest : public ::testing::TestWithParam<bool> {
public:
StateReconciliationTest() : builder_(simpleComponentBuilder()) {
CoreFeatures::enableClonelessStateProgression = GetParam();
}

ComponentBuilder builder_;
};

TEST_P(StateReconciliationTest, testStateReconciliation) {
auto shadowNodeA = std::shared_ptr<RootShadowNode>{};
auto shadowNodeAA = std::shared_ptr<ViewShadowNode>{};
auto shadowNodeAB = std::shared_ptr<ScrollViewShadowNode>{};
auto shadowNodeABA = std::shared_ptr<ViewShadowNode>{};
auto shadowNodeABB = std::shared_ptr<ViewShadowNode>{};
auto shadowNodeABC = std::shared_ptr<ViewShadowNode>{};

// clang-format off
auto element =
Element<RootShadowNode>()
.reference(shadowNodeA)
.finalize([](RootShadowNode &shadowNode){
shadowNode.sealRecursive();
})
.children({
Element<ViewShadowNode>()
.reference(shadowNodeAA),
Element<ScrollViewShadowNode>()
.reference(shadowNodeAB)
.children({
Element<ViewShadowNode>()
.children({
Element<ViewShadowNode>()
.reference(shadowNodeABA),
Element<ViewShadowNode>()
.reference(shadowNodeABB),
Element<ViewShadowNode>()
.reference(shadowNodeABC)
})
})
});
// clang-format on

ContextContainer contextContainer{};

auto shadowNode = builder.build(element);
auto shadowNode = builder_.build(element);

auto rootShadowNodeState1 = shadowNode->ShadowNode::clone({});

Expand Down Expand Up @@ -184,5 +174,128 @@ TEST(StateReconciliationTest, testStateReconciliation) {
},
{true});

EXPECT_EQ(findDescendantNode(shadowTree, family)->getState(), state3);
// 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.
if (!GetParam()) {
EXPECT_EQ(findDescendantNode(shadowTree, family)->getState(), state3);
} else {
EXPECT_EQ(findDescendantNode(shadowTree, family)->getState(), state2);
}
}

TEST_P(StateReconciliationTest, testCloneslessStateReconciliationDoesntClone) {
auto shadowNodeA = std::shared_ptr<RootShadowNode>{};
auto shadowNodeAA = std::shared_ptr<ViewShadowNode>{};
auto shadowNodeAB = std::shared_ptr<ScrollViewShadowNode>{};

// clang-format off
auto element =
Element<RootShadowNode>()
.reference(shadowNodeA)
.children({
Element<ViewShadowNode>()
.reference(shadowNodeAA),
Element<ScrollViewShadowNode>()
.reference(shadowNodeAB)
});
// clang-format on

ContextContainer contextContainer{};

auto rootShadowNode1 = builder_.build(element);

auto& scrollViewComponentDescriptor = shadowNodeAB->getComponentDescriptor();
auto& family = shadowNodeAB->getFamily();
auto state1 = shadowNodeAB->getState();
auto shadowTreeDelegate = DummyShadowTreeDelegate{};
ShadowTree shadowTree{
SurfaceId{11},
LayoutConstraints{},
LayoutContext{},
shadowTreeDelegate,
contextContainer};

shadowTree.commit(
[&](const RootShadowNode& /*oldRootShadowNode*/) {
return std::static_pointer_cast<RootShadowNode>(rootShadowNode1);
},
{true});

EXPECT_EQ(state1->getMostRecentState(), state1);

EXPECT_EQ(findDescendantNode(*rootShadowNode1, family)->getState(), state1);

auto state2 = scrollViewComponentDescriptor.createState(
family, std::make_shared<const ScrollViewState>());

auto rootShadowNode2 =
rootShadowNode1->cloneTree(family, [&](const ShadowNode& oldShadowNode) {
return oldShadowNode.clone(
{ShadowNodeFragment::propsPlaceholder(),
ShadowNodeFragment::childrenPlaceholder(),
state2});
});

EXPECT_EQ(findDescendantNode(*rootShadowNode2, family)->getState(), state2);
EXPECT_EQ(state1->getMostRecentState(), state1);

shadowTree.commit(
[&](const RootShadowNode& /*oldRootShadowNode*/) {
return std::static_pointer_cast<RootShadowNode>(rootShadowNode2);
},
{true});

EXPECT_EQ(state1->getMostRecentState(), state2);
EXPECT_EQ(state2->getMostRecentState(), state2);

ShadowNode::Unshared newlyClonedShadowNode;

auto rootShadowNodeClonedFromReact =
rootShadowNode2->cloneTree(family, [&](const ShadowNode& oldShadowNode) {
newlyClonedShadowNode = oldShadowNode.clone({});
return newlyClonedShadowNode;
});

auto state3 = scrollViewComponentDescriptor.createState(
family, std::make_shared<const ScrollViewState>());

auto rootShadowNodeClonedFromStateUpdate =
rootShadowNode2->cloneTree(family, [&](const ShadowNode& oldShadowNode) {
return oldShadowNode.clone(
{ShadowNodeFragment::propsPlaceholder(),
ShadowNodeFragment::childrenPlaceholder(),
state3});
});

shadowTree.commit(
[&](const RootShadowNode& /*oldRootShadowNode*/) {
return std::static_pointer_cast<RootShadowNode>(
rootShadowNodeClonedFromStateUpdate);
},
{});

shadowTree.commit(
[&](const RootShadowNode& /*oldRootShadowNode*/) {
return std::static_pointer_cast<RootShadowNode>(
rootShadowNodeClonedFromReact);
},
{true});

auto scrollViewShadowNode = findDescendantNode(shadowTree, family);

EXPECT_EQ(scrollViewShadowNode->getState(), state3);

if (GetParam()) {
// Checking that newlyClonedShadowNode was not cloned unnecessarly by state
// progression. This fails with the old algorithm.
EXPECT_EQ(scrollViewShadowNode, newlyClonedShadowNode.get());
}
}
INSTANTIATE_TEST_SUITE_P(
StateReconciliationTestInstantiation,
StateReconciliationTest,
testing::Values(false, true));
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ bool CoreFeatures::enableCleanParagraphYogaNode = false;
bool CoreFeatures::disableScrollEventThrottleRequirement = false;
bool CoreFeatures::enableGranularShadowTreeStateReconciliation = false;
bool CoreFeatures::enableDefaultAsyncBatchedPriority = false;
bool CoreFeatures::enableClonelessStateProgression = false;

} // namespace facebook::react
3 changes: 3 additions & 0 deletions packages/react-native/ReactCommon/react/utils/CoreFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ class CoreFeatures {

// Default state updates and events to async batched priority.
static bool enableDefaultAsyncBatchedPriority;

// When enabled, Fabric will avoid cloning notes to perform state progression.
static bool enableClonelessStateProgression;
};

} // namespace facebook::react