Skip to content

Commit

Permalink
Add static effectTag bit for passive effect cleanup after unmount
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Jul 28, 2020
1 parent c45bc43 commit 5b6bc9d
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 42 deletions.
19 changes: 18 additions & 1 deletion packages/react-reconciler/src/ReactChildFiber.new.js
Expand Up @@ -15,7 +15,16 @@ import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane';

import getComponentName from 'shared/getComponentName';
import {Placement, Deletion} from './ReactSideEffectTags';
import {
Deletion,
NoEffect,
PassiveMask,
Placement,
} from './ReactSideEffectTags';
import {
NoEffect as NoSubtreeTag,
Passive as PassiveSubtreeTag,
} from './ReactSubtreeTags';
import {
getIteratorFn,
REACT_ELEMENT_TYPE,
Expand Down Expand Up @@ -293,6 +302,14 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.deletions = [childToDelete];
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;

// If we are deleting a subtree that contains a passive effect,
// mark the parent so that we're sure to traverse after commit and run any unmount functions.
const primaryEffectTag = childToDelete.effectTag & PassiveMask;
const primarySubtreeTag = childToDelete.subtreeTag & PassiveSubtreeTag;
if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) {
returnFiber.subtreeTag |= PassiveSubtreeTag;
}
} else {
deletions.push(childToDelete);
}
Expand Down
16 changes: 9 additions & 7 deletions packages/react-reconciler/src/ReactFiber.new.js
Expand Up @@ -29,11 +29,8 @@ import {
enableScopeAPI,
enableBlocksAPI,
} from 'shared/ReactFeatureFlags';
import {NoEffect, Placement} from './ReactSideEffectTags';
import {
NoEffect as NoSubtreeEffect,
Static as StaticSubtreeEffects,
} from './ReactSubtreeTags';
import {NoEffect, Placement, StaticMask} from './ReactSideEffectTags';
import {NoEffect as NoSubtreeEffect} from './ReactSubtreeTags';
import {ConcurrentRoot, BlockingRoot} from './ReactRootTags';
import {
IndeterminateComponent,
Expand Down Expand Up @@ -310,7 +307,11 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
}
}

workInProgress.subtreeTag = current.subtreeTag & StaticSubtreeEffects;
// Reset all effects except static ones.
// Static effects are not specific to a render.
workInProgress.effectTag = current.effectTag & StaticMask;

workInProgress.subtreeTag = NoSubtreeEffect;
workInProgress.childLanes = current.childLanes;
workInProgress.lanes = current.lanes;

Expand Down Expand Up @@ -375,7 +376,8 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {

// Reset the effect tag but keep any Placement tags, since that's something
// that child fiber is setting, not the reconciliation.
workInProgress.effectTag &= Placement;
// Also keep static tags since those are meant to last the life of the fiber.
workInProgress.effectTag &= Placement | StaticMask;

// The effect list is no longer valid.
workInProgress.nextEffect = null;
Expand Down
27 changes: 26 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
Expand Up @@ -64,7 +64,13 @@ import {
Ref,
Deletion,
ForceUpdateForLegacySuspense,
PassiveMask,
StaticMask,
} from './ReactSideEffectTags';
import {
NoEffect as NoSubtreeTag,
Passive as PassiveSubtreeTag,
} from './ReactSubtreeTags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
debugRenderPhaseSideEffectsForStrictMode,
Expand Down Expand Up @@ -2064,13 +2070,24 @@ function updateSuspensePrimaryChildren(
if (currentFallbackChildFragment !== null) {
// Delete the fallback child fragment
currentFallbackChildFragment.nextEffect = null;
currentFallbackChildFragment.effectTag = Deletion;
currentFallbackChildFragment.effectTag =
(currentFallbackChildFragment.effectTag & StaticMask) | Deletion;
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
const deletions = workInProgress.deletions;
if (deletions === null) {
workInProgress.deletions = [currentFallbackChildFragment];
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
workInProgress.effectTag |= Deletion;

// If we are deleting a subtree that contains a passive effect,
// mark the parent so that we're sure to traverse after commit and run any unmount functions.
const primaryEffectTag =
currentFallbackChildFragment.effectTag & PassiveMask;
const primarySubtreeTag =
currentFallbackChildFragment.subtreeTag & PassiveSubtreeTag;
if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) {
workInProgress.subtreeTag |= PassiveSubtreeTag;
}
} else {
deletions.push(currentFallbackChildFragment);
}
Expand Down Expand Up @@ -3055,6 +3072,14 @@ function remountFiber(
returnFiber.deletions = [current];
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;

// If we are deleting a subtree that contains a passive effect,
// mark the parent so that we're sure to traverse after commit and run any unmount functions.
const primaryEffectTag = current.effectTag & PassiveMask;
const primarySubtreeTag = current.subtreeTag & PassiveSubtreeTag;
if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) {
returnFiber.subtreeTag |= PassiveSubtreeTag;
}
} else {
deletions.push(current);
}
Expand Down
8 changes: 5 additions & 3 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Expand Up @@ -50,6 +50,7 @@ import {createDeprecatedResponderListener} from './ReactFiberDeprecatedEvents.ne
import {
Update as UpdateEffect,
Passive as PassiveEffect,
PassiveStatic as PassiveStaticEffect,
} from './ReactSideEffectTags';
import {
HasEffect as HookHasEffect,
Expand Down Expand Up @@ -1270,7 +1271,7 @@ function mountEffect(
}
}
return mountEffectImpl(
UpdateEffect | PassiveEffect,
UpdateEffect | PassiveEffect | PassiveStaticEffect,
HookPassive,
create,
deps,
Expand All @@ -1288,7 +1289,7 @@ function updateEffect(
}
}
return updateEffectImpl(
UpdateEffect | PassiveEffect,
UpdateEffect | PassiveEffect | PassiveStaticEffect,
HookPassive,
create,
deps,
Expand Down Expand Up @@ -1631,7 +1632,8 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
const setId = mountState(id)[1];

if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
currentlyRenderingFiber.effectTag |= UpdateEffect | PassiveEffect;
currentlyRenderingFiber.effectTag |=
UpdateEffect | PassiveEffect | PassiveStaticEffect;
pushEffect(
HookHasEffect | HookPassive,
() => {
Expand Down
20 changes: 19 additions & 1 deletion packages/react-reconciler/src/ReactFiberHydrationContext.new.js
Expand Up @@ -24,7 +24,17 @@ import {
HostRoot,
SuspenseComponent,
} from './ReactWorkTags';
import {Deletion, Hydrating, Placement} from './ReactSideEffectTags';
import {
Deletion,
Hydrating,
NoEffect,
PassiveMask,
Placement,
} from './ReactSideEffectTags';
import {
NoEffect as NoSubtreeTag,
Passive as PassiveSubtreeTag,
} from './ReactSubtreeTags';
import invariant from 'shared/invariant';

import {
Expand Down Expand Up @@ -130,6 +140,14 @@ function deleteHydratableInstance(
returnFiber.deletions = [childToDelete];
// TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions)
returnFiber.effectTag |= Deletion;

// If we are deleting a subtree that contains a passive effect,
// mark the parent so that we're sure to traverse after commit and run any unmount functions.
const primaryEffectTag = childToDelete.effectTag & PassiveMask;
const primarySubtreeTag = childToDelete.subtreeTag & PassiveSubtreeTag;
if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) {
returnFiber.subtreeTag |= PassiveSubtreeTag;
}
} else {
deletions.push(childToDelete);
}
Expand Down
60 changes: 36 additions & 24 deletions packages/react-reconciler/src/ReactSideEffectTags.js
Expand Up @@ -10,37 +10,49 @@
export type SideEffectTag = number;

// Don't change these two values. They're used by React Dev Tools.
export const NoEffect = /* */ 0b000000000000000;
export const PerformedWork = /* */ 0b000000000000001;
export const NoEffect = /* */ 0b0000000000000000;
export const PerformedWork = /* */ 0b0000000000000001;

// You can change the rest (and add more).
export const Placement = /* */ 0b000000000000010;
export const Update = /* */ 0b000000000000100;
export const PlacementAndUpdate = /* */ 0b000000000000110;
export const Deletion = /* */ 0b000000000001000;
export const ContentReset = /* */ 0b000000000010000;
export const Callback = /* */ 0b000000000100000;
export const DidCapture = /* */ 0b000000001000000;
export const Ref = /* */ 0b000000010000000;
export const Snapshot = /* */ 0b000000100000000;
export const Passive = /* */ 0b000001000000000;
export const PassiveUnmountPendingDev = /* */ 0b010000000000000;
export const Hydrating = /* */ 0b000010000000000;
export const HydratingAndUpdate = /* */ 0b000010000000100;
export const Placement = /* */ 0b0000000000000010;
export const Update = /* */ 0b0000000000000100;
export const PlacementAndUpdate = /* */ 0b0000000000000110;
export const Deletion = /* */ 0b0000000000001000;
export const ContentReset = /* */ 0b0000000000010000;
export const Callback = /* */ 0b0000000000100000;
export const DidCapture = /* */ 0b0000000001000000;
export const Ref = /* */ 0b0000000010000000;
export const Snapshot = /* */ 0b0000000100000000;
export const Passive = /* */ 0b0000001000000000;
export const PassiveUnmountPendingDev = /* */ 0b0010000000000000;
export const Hydrating = /* */ 0b0000010000000000;
export const HydratingAndUpdate = /* */ 0b0000010000000100;

// Passive & Update & Callback & Ref & Snapshot
export const LifecycleEffectMask = /* */ 0b000001110100100;
export const LifecycleEffectMask = /* */ 0b0000001110100100;

// Union of all host effects
export const HostEffectMask = /* */ 0b000011111111111;
export const HostEffectMask = /* */ 0b0000011111111111;

// These are not really side effects, but we still reuse this field.
export const Incomplete = /* */ 0b000100000000000;
export const ShouldCapture = /* */ 0b001000000000000;
export const ForceUpdateForLegacySuspense = /* */ 0b100000000000000;
export const Incomplete = /* */ 0b0000100000000000;
export const ShouldCapture = /* */ 0b0001000000000000;
export const ForceUpdateForLegacySuspense = /* */ 0b0100000000000000;

// Static tags describe aspects of a fiber that are not specific to a render,
// e.g. a fiber uses a passive effect (even if there are no updates on this particular render).
// This enables us to defer more work in the unmount case,
// since we can defer traversing the tree during layout to look for Passive effects,
// and instead rely on the static flag as a signal that there may be cleanup work.
export const PassiveStatic = /* */ 0b1000000000000000;

// Union of side effect groupings as pertains to subtreeTag
export const BeforeMutationMask = /* */ 0b000001100001010;
export const MutationMask = /* */ 0b000010010011110;
export const LayoutMask = /* */ 0b000000010100100;
export const PassiveMask = /* */ 0b000001000000000;
export const BeforeMutationMask = /* */ 0b0000001100001010;
export const MutationMask = /* */ 0b0000010010011110;
export const LayoutMask = /* */ 0b0000000010100100;
export const PassiveMask = /* */ 0b1000001000000000;

// Union of tags that don't get reset on clones.
// This allows certain concepts to persist without recalculting them,
// e.g. whether a subtree contains passive effects or portals.
export const StaticMask = /* */ 0b1000000000000000;
5 changes: 0 additions & 5 deletions packages/react-reconciler/src/ReactSubtreeTags.js
Expand Up @@ -14,8 +14,3 @@ export const BeforeMutation = /* */ 0b0001;
export const Mutation = /* */ 0b0010;
export const Layout = /* */ 0b0100;
export const Passive = /* */ 0b1000;

// Union of tags that don't get reset on clones.
// This allows certain concepts to persist without recalculting them,
// e.g. whether a subtree contains passive effects or portals.
export const Static = /* */ 0b1000;
Expand Up @@ -502,7 +502,11 @@ describe('SchedulingProfiler', () => {
'--render-start-1024',
'--render-stop',
'--commit-start-1024',
'--layout-effects-start-1024',
'--layout-effects-stop',
'--commit-stop',
'--passive-effects-start-1024',
'--passive-effects-stop',
]);
});

Expand Down

0 comments on commit 5b6bc9d

Please sign in to comment.