Skip to content

Commit

Permalink
Use recursion to traverse during mutation phase
Browse files Browse the repository at this point in the history
Most of the commit phase uses iterative loops to traverse the tree.
Originally we thought this would be faster than using recursion, but
a while back @trueadm did some performance testing and found that the
loop was slower because we assign to the `return` pointer before
entering a subtree (which we have to do because the `return` pointer
is not always consistent; it could point to one of two fibers).

The other motivation is so we can take advantage of the JS stack to
track contextual information, like the nearest host parent.

We already use recursion in a few places; this changes the mutation
phase to use it, too.
  • Loading branch information
acdlite committed Apr 8, 2022
1 parent f9e6aef commit 481dece
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 86 deletions.
12 changes: 10 additions & 2 deletions packages/react-reconciler/src/ReactCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,22 @@ export function resetCurrentFiber() {
}
}

export function setCurrentFiber(fiber: Fiber) {
export function setCurrentFiber(fiber: Fiber | null) {
if (__DEV__) {
ReactDebugCurrentFrame.getCurrentStack = getCurrentFiberStackInDev;
ReactDebugCurrentFrame.getCurrentStack =
fiber === null ? null : getCurrentFiberStackInDev;
current = fiber;
isRendering = false;
}
}

export function getCurrentFiber(): Fiber | null {
if (__DEV__) {
return current;
}
return null;
}

export function setIsRendering(rendering: boolean) {
if (__DEV__) {
isRendering = rendering;
Expand Down
91 changes: 49 additions & 42 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFrom
import {
resetCurrentFiber as resetCurrentDebugFiberInDEV,
setCurrentFiber as setCurrentDebugFiberInDEV,
getCurrentFiber as getCurrentDebugFiberInDEV,
} from './ReactCurrentFiber';
import {resolveDefaultProps} from './ReactFiberLazyComponent.new';
import {
Expand Down Expand Up @@ -1901,62 +1902,50 @@ export function isSuspenseBoundaryBeingHidden(

export function commitMutationEffects(
root: FiberRoot,
firstChild: Fiber,
finishedWork: Fiber,
committedLanes: Lanes,
) {
inProgressLanes = committedLanes;
inProgressRoot = root;
nextEffect = firstChild;
nextEffect = finishedWork;

commitMutationEffects_begin(root, committedLanes);
setCurrentDebugFiberInDEV(finishedWork);
commitMutationEffectsOnFiber(finishedWork, root, committedLanes);
setCurrentDebugFiberInDEV(finishedWork);

inProgressLanes = null;
inProgressRoot = null;
}

function commitMutationEffects_begin(root: FiberRoot, lanes: Lanes) {
while (nextEffect !== null) {
const fiber = nextEffect;

// TODO: Should wrap this in flags check, too, as optimization
const deletions = fiber.deletions;
if (deletions !== null) {
for (let i = 0; i < deletions.length; i++) {
const childToDelete = deletions[i];
try {
commitDeletion(root, childToDelete, fiber);
} catch (error) {
captureCommitPhaseError(childToDelete, fiber, error);
}
function recursivelyTraverseMutationEffects(
root: FiberRoot,
parentFiber: Fiber,
lanes: Lanes,
) {
// Deletions effects can be scheduled on any fiber type. They need to happen
// before the children effects hae fired.
const deletions = parentFiber.deletions;
if (deletions !== null) {
for (let i = 0; i < deletions.length; i++) {
const childToDelete = deletions[i];
try {
commitDeletion(root, childToDelete, parentFiber);
} catch (error) {
captureCommitPhaseError(childToDelete, parentFiber, error);
}
}

const child = fiber.child;
if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
child.return = fiber;
nextEffect = child;
} else {
commitMutationEffects_complete(root, lanes);
}
}
}

function commitMutationEffects_complete(root: FiberRoot, lanes: Lanes) {
while (nextEffect !== null) {
const fiber = nextEffect;
setCurrentDebugFiberInDEV(fiber);
commitMutationEffectsOnFiber(fiber, root, lanes);
resetCurrentDebugFiberInDEV();

const sibling = fiber.sibling;
if (sibling !== null) {
sibling.return = fiber.return;
nextEffect = sibling;
return;
const prevDebugFiber = getCurrentDebugFiberInDEV();
if (parentFiber.subtreeFlags & MutationMask) {
let child = parentFiber.child;
while (child !== null) {
setCurrentDebugFiberInDEV(child);
commitMutationEffectsOnFiber(child, root, lanes);
child = child.sibling;
}

nextEffect = fiber.return;
}
setCurrentDebugFiberInDEV(prevDebugFiber);
}

function commitMutationEffectsOnFiber(
Expand All @@ -1975,6 +1964,7 @@ function commitMutationEffectsOnFiber(
case ForwardRef:
case MemoComponent:
case SimpleMemoComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Update) {
Expand Down Expand Up @@ -2027,6 +2017,7 @@ function commitMutationEffectsOnFiber(
return;
}
case ClassComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Ref) {
Expand All @@ -2037,6 +2028,7 @@ function commitMutationEffectsOnFiber(
return;
}
case HostComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Ref) {
Expand All @@ -2045,7 +2037,13 @@ function commitMutationEffectsOnFiber(
}
}
if (supportsMutation) {
if (flags & ContentReset) {
// TODO: ContentReset gets cleared by the children during the commit
// phase. This is a refactor hazard because it means we must read
// flags the flags after `commitReconciliationEffects` has already run;
// the order matters. We should refactor so that ContentReset does not
// rely on mutating the flag during commit. Like by setting a flag
// during the render phase instead.
if (finishedWork.flags & ContentReset) {
const instance: Instance = finishedWork.stateNode;
try {
resetTextContent(instance);
Expand Down Expand Up @@ -2092,6 +2090,7 @@ function commitMutationEffectsOnFiber(
return;
}
case HostText: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Update) {
Expand Down Expand Up @@ -2121,6 +2120,7 @@ function commitMutationEffectsOnFiber(
return;
}
case HostRoot: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Update) {
Expand Down Expand Up @@ -2153,6 +2153,7 @@ function commitMutationEffectsOnFiber(
return;
}
case HostPortal: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Update) {
Expand All @@ -2170,6 +2171,7 @@ function commitMutationEffectsOnFiber(
return;
}
case SuspenseComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Visibility) {
Expand All @@ -2194,6 +2196,7 @@ function commitMutationEffectsOnFiber(
return;
}
case OffscreenComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Visibility) {
Expand Down Expand Up @@ -2231,6 +2234,7 @@ function commitMutationEffectsOnFiber(
return;
}
case SuspenseListComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

if (flags & Update) {
Expand All @@ -2240,6 +2244,7 @@ function commitMutationEffectsOnFiber(
}
case ScopeComponent: {
if (enableScopeAPI) {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

// TODO: This is a temporary solution that allowed us to transition away
Expand All @@ -2258,11 +2263,13 @@ function commitMutationEffectsOnFiber(
return;
}
default: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);

return;
}
}
}

function commitReconciliationEffects(finishedWork: Fiber) {
// Placement effects (insertions, reorders) can be scheduled on any fiber
// type. They needs to happen after the children effects have fired, but
Expand Down
Loading

0 comments on commit 481dece

Please sign in to comment.