From d93c8faadac0a1cea4ba2a3c1333e78314a6e61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 24 Jul 2020 12:31:30 -0400 Subject: [PATCH] Move Persistent Mode Optimization away from firstEffect (#19381) Persistent mode needs to clone a parent and add its children if a child has changed. We have an optimization in persistent mode where we don't do that if no child could've changed. If there are no effects scheduled for any child then there couldn't have been changes. Instead of checking for this on firstEffect, we now check this on the children's effectTag and subtreeTags. This is quite unfortunate because if we could just do this check a little bit later we would've already gotten it transferred to the completed work's subtreeTag. Now we have to loop over all the children and if any of them changed, we have to loop over them again. Doing at least two loops per parent. --- .../src/ReactFiberCompleteWork.new.js | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index f74b3d6da5..69eccc53d0 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -65,7 +65,9 @@ import { NoEffect, DidCapture, Snapshot, + MutationMask, } from './ReactSideEffectTags'; +import {NoEffect as NoSubtreeTag, Mutation} from './ReactSubtreeTags'; import invariant from 'shared/invariant'; import { @@ -154,6 +156,25 @@ function markRef(workInProgress: Fiber) { workInProgress.effectTag |= Ref; } +function hadNoMutationsEffects(current: null | Fiber, completedWork: Fiber) { + const didBailout = current !== null && current.child === completedWork.child; + if (didBailout) { + return true; + } + + let child = completedWork.child; + while (child !== null) { + if ((child.effectTag & MutationMask) !== NoEffect) { + return false; + } + if ((child.subtreeTag & Mutation) !== NoSubtreeTag) { + return false; + } + child = child.sibling; + } + return true; +} + let appendAllChildren; let updateHostContainer; let updateHostComponent; @@ -198,7 +219,7 @@ if (supportsMutation) { } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -442,13 +463,13 @@ if (supportsMutation) { node = node.sibling; } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { const portalOrRoot: { containerInfo: Container, pendingChildren: ChildSet, ... } = workInProgress.stateNode; - const childrenUnchanged = workInProgress.firstEffect === null; + const childrenUnchanged = hadNoMutationsEffects(current, workInProgress); if (childrenUnchanged) { // No changes, just reuse the existing instance. } else { @@ -473,7 +494,7 @@ if (supportsMutation) { const oldProps = current.memoizedProps; // If there are no effects associated with this node, then none of our children had any updates. // This guarantees that we can reuse all of them. - const childrenUnchanged = workInProgress.firstEffect === null; + const childrenUnchanged = hadNoMutationsEffects(current, workInProgress); if (childrenUnchanged && oldProps === newProps) { // No changes, just reuse the existing instance. // Note that this might release a previous clone. @@ -556,7 +577,7 @@ if (supportsMutation) { }; } else { // No host operations - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -700,7 +721,7 @@ function completeWork( workInProgress.effectTag |= Snapshot; } } - updateHostContainer(workInProgress); + updateHostContainer(current, workInProgress); return null; } case HostComponent: { @@ -979,7 +1000,7 @@ function completeWork( } case HostPortal: popHostContainer(workInProgress); - updateHostContainer(workInProgress); + updateHostContainer(current, workInProgress); if (current === null) { preparePortalMount(workInProgress.stateNode.containerInfo); }