Skip to content

Commit

Permalink
Set up test to reduce the amount of failed commits in ShadowTree (fac…
Browse files Browse the repository at this point in the history
…ebook#38706)

Summary:
Pull Request resolved: facebook#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>
  • Loading branch information
4 people authored and facebook-github-bot committed Aug 14, 2023
1 parent e29b5bb commit e44fdfe
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <react/renderer/mounting/ShadowTreeRevision.h>
#include <react/renderer/mounting/ShadowViewMutation.h>
#include <react/renderer/telemetry/TransactionTelemetry.h>
#include <react/utils/CoreFeatures.h>

#include "ShadowTreeDelegate.h"

Expand Down Expand Up @@ -249,6 +250,8 @@ ShadowTree::ShadowTree(
currentRevision_ = ShadowTreeRevision{
rootShadowNode, INITIAL_REVISION, TransactionTelemetry{}};

lastRevisionNumberWithNewState_ = currentRevision_.number;

mountingCoordinator_ =
std::make_shared<MountingCoordinator const>(currentRevision_);
}
Expand Down Expand Up @@ -322,12 +325,14 @@ CommitStatus ShadowTree::tryCommit(
CommitMode commitMode;
auto oldRevision = ShadowTreeRevision{};
auto newRevision = ShadowTreeRevision{};
ShadowTreeRevision::Number lastRevisionNumberWithNewState;

{
// Reading `currentRevision_` in shared manner.
std::shared_lock lock(commitMutex_);
commitMode = commitMode_;
oldRevision = currentRevision_;
lastRevisionNumberWithNewState = lastRevisionNumberWithNewState_;
}

auto const &oldRootShadowNode = oldRevision.rootShadowNode;
Expand Down Expand Up @@ -377,11 +382,21 @@ CommitStatus ShadowTree::tryCommit(
return CommitStatus::Cancelled;
}

if (currentRevision_.number != oldRevision.number) {
return CommitStatus::Failed;
if (CoreFeatures::enableGranularShadowTreeStateReconciliation) {
auto lastRevisionNumberWithNewStateChanged =
lastRevisionNumberWithNewState != lastRevisionNumberWithNewState_;
// Commit should only fail if we propagated the wrong state.
if (commitOptions.enableStateReconciliation &&
lastRevisionNumberWithNewStateChanged) {
return CommitStatus::Failed;
}
} else {
if (currentRevision_.number != oldRevision.number) {
return CommitStatus::Failed;
}
}

auto newRevisionNumber = oldRevision.number + 1;
auto newRevisionNumber = currentRevision_.number + 1;

{
std::scoped_lock dispatchLock(EventEmitter::DispatchMutex());
Expand All @@ -398,6 +413,9 @@ CommitStatus ShadowTree::tryCommit(
std::move(newRootShadowNode), newRevisionNumber, telemetry};

currentRevision_ = newRevision;
if (!commitOptions.enableStateReconciliation) {
lastRevisionNumberWithNewState_ = newRevisionNumber;
}
}

emitLayoutEvents(affectedLayoutableNodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class ShadowTree final {
};

struct CommitOptions {
// When set to true, Shadow Node state from current revision will be applied
// to the new revision. For more details see
// https://reactnative.dev/architecture/render-pipeline#react-native-renderer-state-updates
bool enableStateReconciliation{false};

// Indicates if mounting will be triggered synchronously and React will
Expand Down Expand Up @@ -144,6 +147,8 @@ class ShadowTree final {
mutable CommitMode commitMode_{
CommitMode::Normal}; // Protected by `commitMutex_`.
mutable ShadowTreeRevision currentRevision_; // Protected by `commitMutex_`.
mutable ShadowTreeRevision::Number
lastRevisionNumberWithNewState_; // Protected by `commitMutex_`.
MountingCoordinator::Shared mountingCoordinator_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ Scheduler::Scheduler(
CoreFeatures::cacheLastTextMeasurement =
reactNativeConfig_->getBool("react_fabric:enable_text_measure_cache");

CoreFeatures::enableGranularShadowTreeStateReconciliation =
reactNativeConfig_->getBool(
"react_fabric:enable_granular_shadow_tree_state_reconciliation");

if (animationDelegate != nullptr) {
animationDelegate->setComponentDescriptorRegistry(
componentDescriptorRegistry_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ bool CoreFeatures::enableMountHooks = false;
bool CoreFeatures::doNotSwapLeftAndRightOnAndroidInLTR = false;
bool CoreFeatures::enableCleanParagraphYogaNode = false;
bool CoreFeatures::disableScrollEventThrottleRequirement = false;
bool CoreFeatures::enableGranularShadowTreeStateReconciliation = false;

} // namespace facebook::react
4 changes: 4 additions & 0 deletions packages/react-native/ReactCommon/react/utils/CoreFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ class CoreFeatures {
// Fire `onScroll` events continuously on iOS without a `scrollEventThrottle`
// props, and provide continuous `onScroll` upates like other platforms.
static bool disableScrollEventThrottleRequirement;

// When enabled, the renderer would only fail commits when they propagate
// state and the last commit that updated state changed before committing.
static bool enableGranularShadowTreeStateReconciliation;
};

} // namespace facebook::react

0 comments on commit e44fdfe

Please sign in to comment.