Set up test to reduce the amount of failed commits in ShadowTree (#38706)

rubennorte · sammy-SC · tomekzaw · facebook-github-bot · commit e44fdfed60c0 · 2023-08-14T08:49:02.000-07:00
Summary: Pull Request resolved: #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>
diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp
@@ -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"
 
@@ -249,6 +250,8 @@ ShadowTree::ShadowTree(
   currentRevision_ = ShadowTreeRevision{
       rootShadowNode, INITIAL_REVISION, TransactionTelemetry{}};
 
+  lastRevisionNumberWithNewState_ = currentRevision_.number;
+
   mountingCoordinator_ =
       std::make_shared<MountingCoordinator const>(currentRevision_);
 }
@@ -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;
@@ -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());
@@ -398,6 +413,9 @@ CommitStatus ShadowTree::tryCommit(
         std::move(newRootShadowNode), newRevisionNumber, telemetry};
 
     currentRevision_ = newRevision;
+    if (!commitOptions.enableStateReconciliation) {
+      lastRevisionNumberWithNewState_ = newRevisionNumber;
+    }
   }
 
   emitLayoutEvents(affectedLayoutableNodes);
diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h
@@ -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
@@ -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_;
 };
 
diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp
@@ -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_);
diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp b/packages/react-native/ReactCommon/react/utils/CoreFeatures.cpp
@@ -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
diff --git a/packages/react-native/ReactCommon/react/utils/CoreFeatures.h b/packages/react-native/ReactCommon/react/utils/CoreFeatures.h
@@ -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
