Diff properties in the commit phase instead of generating an update p…

…ayload (#26583) This removes the concept of `prepareUpdate()`, behind a flag. React Native already does everything in the commit phase, but generates a temporary update payload before applying it. React Fabric does it both in the render phase. Now it just moves it to a single host config. For DOM I forked updateProperties into one that does diffing and updating in one pass vs just applying a pre-diffed updatePayload. There are a few downsides of this approach: - If only "children" has changed, we end up scheduling an update to be done in the commit phase. Since we traverse through it anyway, it's probably not much extra. - It does more work in the commit phase so for a large tree that is mostly unchanged, it'll stall longer. - It does some extra work for special cases since that work happens if anything has changed. We no longer have a deep bailout. - The special cases now have to each replicate the "clean up old props" loop, leading to extra code. The benefit is that this doesn't allocate temporary extra objects (possibly multiple per element if the array has to resize). It's less work overall. It also gives us an option to reuse this function for a sync render optimization. Another benefit is that if we do the loop in the commit phase I can do further optimizations by reading all props that I need for special cases in that loop instead of polymorphic reads from props. This is what I'd like to do in future refactors that would be stacked on top of this change. DiffTrain build for [ca41adb](ca41adb)
facebook · Apr 10, 2023 · 8ae1b02 · 8ae1b02
1 parent 2e48109
commit 8ae1b02
Show file tree

Hide file tree

Showing 18 changed files with 7,300 additions and 2,763 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-ffb8eaca5966fc7733bd0a23f4055e26d2cc59d7
+ca41adb8c1b256705f73d1fb657421a03dfad82c
diff --git a/compiled/facebook-www/React-dev.modern.js b/compiled/facebook-www/React-dev.modern.js
@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-www-modern-0ed50ada";
+var ReactVersion = "18.3.0-www-modern-82c50b6a";
 
 // ATTENTION
 // When adding new symbols to this file,

diff --git a/compiled/facebook-www/ReactART-dev.classic.js b/compiled/facebook-www/ReactART-dev.classic.js
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-classic-b7529c47";
+var ReactVersion = "18.3.0-www-classic-26b068f9";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -177,7 +177,8 @@ var revertRemovalOfSiblingPrerendering =
   enableUnifiedSyncLane = dynamicFeatureFlags.enableUnifiedSyncLane,
   enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
   enableDeferRootSchedulingToMicrotask =
-    dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask; // On WWW, false is used for a new modern build.
+    dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask,
+  diffInCommitPhase = dynamicFeatureFlags.diffInCommitPhase; // On WWW, false is used for a new modern build.
 var enableProfilerTimer = true;
 var enableProfilerCommitHooks = true;
 var enableProfilerNestedUpdatePhase = true;
@@ -17583,18 +17584,23 @@ function updateHostComponent(
       // In mutation mode, this is sufficient for a bailout because
       // we won't touch this node even if children changed.
       return;
-    } // If we get updated because one of our children updated, we don't
-    getHostContext(); // TODO: Experiencing an error where oldProps is null. Suggests a host
-    // component is hitting the resume path. Figure out why. Possibly
-    // related to `hidden`.
+    }
+
+    if (diffInCommitPhase) {
+      markUpdate(workInProgress);
+    } else {
+      // component is hitting the resume path. Figure out why. Possibly
+      // related to `hidden`.
 
-    var updatePayload = prepareUpdate(); // TODO: Type this specific to this type of component.
+      getHostContext();
+      var updatePayload = prepareUpdate(); // TODO: Type this specific to this type of component.
 
-    workInProgress.updateQueue = updatePayload; // If the update payload indicates that there is a change or if there
-    // is a new ref we mark this as an update. All the work is done in commitWork.
+      workInProgress.updateQueue = updatePayload; // If the update payload indicates that there is a change or if there
+      // is a new ref we mark this as an update. All the work is done in commitWork.
 
-    if (updatePayload) {
-      markUpdate(workInProgress);
+      if (updatePayload) {
+        markUpdate(workInProgress);
+      }
     }
   }
 } // This function must be called at the very end of the complete phase, because
@@ -21225,7 +21231,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
             var _updatePayload = finishedWork.updateQueue;
             finishedWork.updateQueue = null;
 
-            if (_updatePayload !== null) {
+            if (_updatePayload !== null || diffInCommitPhase) {
               try {
                 commitUpdate(
                   _instance2,

diff --git a/compiled/facebook-www/ReactART-dev.modern.js b/compiled/facebook-www/ReactART-dev.modern.js
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-modern-71e9d049";
+var ReactVersion = "18.3.0-www-modern-ae1f3bca";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -177,7 +177,8 @@ var revertRemovalOfSiblingPrerendering =
   enableUnifiedSyncLane = dynamicFeatureFlags.enableUnifiedSyncLane,
   enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
   enableDeferRootSchedulingToMicrotask =
-    dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask; // On WWW, true is used for a new modern build.
+    dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask,
+  diffInCommitPhase = dynamicFeatureFlags.diffInCommitPhase; // On WWW, true is used for a new modern build.
 var enableProfilerTimer = true;
 var enableProfilerCommitHooks = true;
 var enableProfilerNestedUpdatePhase = true;
@@ -17277,18 +17278,23 @@ function updateHostComponent(
       // In mutation mode, this is sufficient for a bailout because
       // we won't touch this node even if children changed.
       return;
-    } // If we get updated because one of our children updated, we don't
-    getHostContext(); // TODO: Experiencing an error where oldProps is null. Suggests a host
-    // component is hitting the resume path. Figure out why. Possibly
-    // related to `hidden`.
+    }
+
+    if (diffInCommitPhase) {
+      markUpdate(workInProgress);
+    } else {
+      // component is hitting the resume path. Figure out why. Possibly
+      // related to `hidden`.
 
-    var updatePayload = prepareUpdate(); // TODO: Type this specific to this type of component.
+      getHostContext();
+      var updatePayload = prepareUpdate(); // TODO: Type this specific to this type of component.
 
-    workInProgress.updateQueue = updatePayload; // If the update payload indicates that there is a change or if there
-    // is a new ref we mark this as an update. All the work is done in commitWork.
+      workInProgress.updateQueue = updatePayload; // If the update payload indicates that there is a change or if there
+      // is a new ref we mark this as an update. All the work is done in commitWork.
 
-    if (updatePayload) {
-      markUpdate(workInProgress);
+      if (updatePayload) {
+        markUpdate(workInProgress);
+      }
     }
   }
 } // This function must be called at the very end of the complete phase, because
@@ -20890,7 +20896,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
             var _updatePayload = finishedWork.updateQueue;
             finishedWork.updateQueue = null;
 
-            if (_updatePayload !== null) {
+            if (_updatePayload !== null || diffInCommitPhase) {
               try {
                 commitUpdate(
                   _instance2,

diff --git a/compiled/facebook-www/ReactART-prod.classic.js b/compiled/facebook-www/ReactART-prod.classic.js
@@ -73,6 +73,7 @@ var ReactSharedInternals =
   enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
   enableDeferRootSchedulingToMicrotask =
     dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask,
+  diffInCommitPhase = dynamicFeatureFlags.diffInCommitPhase,
   REACT_ELEMENT_TYPE = Symbol.for("react.element"),
   REACT_PORTAL_TYPE = Symbol.for("react.portal"),
   REACT_FRAGMENT_TYPE = Symbol.for("react.fragment"),
@@ -5240,6 +5241,9 @@ function getChildContextValues(context) {
     collectNearestChildContextValues(currentFiber, context, childContextValues);
   return childContextValues;
 }
+function markUpdate(workInProgress) {
+  workInProgress.flags |= 4;
+}
 function scheduleRetryEffect(workInProgress, retryQueue) {
   null !== retryQueue
     ? (workInProgress.flags |= 4)
@@ -5359,8 +5363,10 @@ function completeWork(current, workInProgress, renderLanes) {
       renderLanes = workInProgress.type;
       if (null !== current && null != workInProgress.stateNode)
         current.memoizedProps !== newProps &&
-          (workInProgress.updateQueue = UPDATE_SIGNAL) &&
-          (workInProgress.flags |= 4),
+          (diffInCommitPhase
+            ? markUpdate(workInProgress)
+            : (workInProgress.updateQueue = UPDATE_SIGNAL) &&
+              markUpdate(workInProgress)),
           current.ref !== workInProgress.ref &&
             (workInProgress.flags |= 2097664);
       else {
@@ -5427,7 +5433,7 @@ function completeWork(current, workInProgress, renderLanes) {
       return null;
     case 6:
       if (current && null != workInProgress.stateNode)
-        current.memoizedProps !== newProps && (workInProgress.flags |= 4);
+        current.memoizedProps !== newProps && markUpdate(workInProgress);
       else {
         if ("string" !== typeof newProps && null === workInProgress.stateNode)
           throw Error(formatProdErrorMessage(166));
@@ -5597,8 +5603,8 @@ function completeWork(current, workInProgress, renderLanes) {
             }),
             shim$1(),
             null !== workInProgress.ref &&
-              ((workInProgress.flags |= 2097664), (workInProgress.flags |= 4)))
-          : (null !== workInProgress.ref && (workInProgress.flags |= 4),
+              ((workInProgress.flags |= 2097664), markUpdate(workInProgress)))
+          : (null !== workInProgress.ref && markUpdate(workInProgress),
             current.ref !== workInProgress.ref &&
               (workInProgress.flags |= 2097664)),
         bubbleProperties(workInProgress),
@@ -6665,7 +6671,7 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
         current = null !== current ? current.memoizedProps : newProps;
         var updatePayload = finishedWork.updateQueue;
         finishedWork.updateQueue = null;
-        if (null !== updatePayload)
+        if (null !== updatePayload || diffInCommitPhase)
           try {
             flags._applyProps(flags, newProps, current);
           } catch (error$108) {
@@ -10072,7 +10078,7 @@ var slice = Array.prototype.slice,
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-classic-0d8af644",
+    version: "18.3.0-www-classic-9e45a6d4",
     rendererPackageName: "react-art"
   };
 var internals$jscomp$inline_1344 = {
@@ -10103,7 +10109,7 @@ var internals$jscomp$inline_1344 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-classic-0d8af644"
+  reconcilerVersion: "18.3.0-www-classic-9e45a6d4"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1345 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

diff --git a/compiled/facebook-www/ReactART-prod.modern.js b/compiled/facebook-www/ReactART-prod.modern.js
@@ -73,6 +73,7 @@ var ReactSharedInternals =
   enableTransitionTracing = dynamicFeatureFlags.enableTransitionTracing,
   enableDeferRootSchedulingToMicrotask =
     dynamicFeatureFlags.enableDeferRootSchedulingToMicrotask,
+  diffInCommitPhase = dynamicFeatureFlags.diffInCommitPhase,
   REACT_ELEMENT_TYPE = Symbol.for("react.element"),
   REACT_PORTAL_TYPE = Symbol.for("react.portal"),
   REACT_FRAGMENT_TYPE = Symbol.for("react.fragment"),
@@ -4995,6 +4996,9 @@ function getChildContextValues(context) {
     collectNearestChildContextValues(currentFiber, context, childContextValues);
   return childContextValues;
 }
+function markUpdate(workInProgress) {
+  workInProgress.flags |= 4;
+}
 function scheduleRetryEffect(workInProgress, retryQueue) {
   null !== retryQueue
     ? (workInProgress.flags |= 4)
@@ -5108,8 +5112,10 @@ function completeWork(current, workInProgress, renderLanes) {
       renderLanes = workInProgress.type;
       if (null !== current && null != workInProgress.stateNode)
         current.memoizedProps !== newProps &&
-          (workInProgress.updateQueue = UPDATE_SIGNAL) &&
-          (workInProgress.flags |= 4),
+          (diffInCommitPhase
+            ? markUpdate(workInProgress)
+            : (workInProgress.updateQueue = UPDATE_SIGNAL) &&
+              markUpdate(workInProgress)),
           current.ref !== workInProgress.ref &&
             (workInProgress.flags |= 2097664);
       else {
@@ -5176,7 +5182,7 @@ function completeWork(current, workInProgress, renderLanes) {
       return null;
     case 6:
       if (current && null != workInProgress.stateNode)
-        current.memoizedProps !== newProps && (workInProgress.flags |= 4);
+        current.memoizedProps !== newProps && markUpdate(workInProgress);
       else {
         if ("string" !== typeof newProps && null === workInProgress.stateNode)
           throw Error(formatProdErrorMessage(166));
@@ -5342,8 +5348,8 @@ function completeWork(current, workInProgress, renderLanes) {
             }),
             shim$1(),
             null !== workInProgress.ref &&
-              ((workInProgress.flags |= 2097664), (workInProgress.flags |= 4)))
-          : (null !== workInProgress.ref && (workInProgress.flags |= 4),
+              ((workInProgress.flags |= 2097664), markUpdate(workInProgress)))
+          : (null !== workInProgress.ref && markUpdate(workInProgress),
             current.ref !== workInProgress.ref &&
               (workInProgress.flags |= 2097664)),
         bubbleProperties(workInProgress),
@@ -6401,7 +6407,7 @@ function commitMutationEffectsOnFiber(finishedWork, root) {
         current = null !== current ? current.memoizedProps : newProps;
         var updatePayload = finishedWork.updateQueue;
         finishedWork.updateQueue = null;
-        if (null !== updatePayload)
+        if (null !== updatePayload || diffInCommitPhase)
           try {
             flags._applyProps(flags, newProps, current);
           } catch (error$107) {
@@ -9737,7 +9743,7 @@ var slice = Array.prototype.slice,
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-modern-77b45d24",
+    version: "18.3.0-www-modern-f0a7ed1e",
     rendererPackageName: "react-art"
   };
 var internals$jscomp$inline_1324 = {
@@ -9768,7 +9774,7 @@ var internals$jscomp$inline_1324 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-modern-77b45d24"
+  reconcilerVersion: "18.3.0-www-modern-f0a7ed1e"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1325 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		ffb8eaca5966fc7733bd0a23f4055e26d2cc59d7
		ca41adb8c1b256705f73d1fb657421a03dfad82c