Skip to content

Commit de3c069

Browse files
committed
Move flag check into each switch case
The fiber tag is more specific than the effect flag, so we should always refine the type of work first, to minimize redundant checks.
1 parent f5916d1 commit de3c069

File tree

2 files changed

+346
-320
lines changed

2 files changed

+346
-320
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 173 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -2884,20 +2884,18 @@ function commitPassiveMountEffects_complete(
28842884
while (nextEffect !== null) {
28852885
const fiber = nextEffect;
28862886

2887-
if ((fiber.flags & Passive) !== NoFlags) {
2888-
setCurrentDebugFiberInDEV(fiber);
2889-
try {
2890-
commitPassiveMountOnFiber(
2891-
root,
2892-
fiber,
2893-
committedLanes,
2894-
committedTransitions,
2895-
);
2896-
} catch (error) {
2897-
captureCommitPhaseError(fiber, fiber.return, error);
2898-
}
2899-
resetCurrentDebugFiberInDEV();
2887+
setCurrentDebugFiberInDEV(fiber);
2888+
try {
2889+
commitPassiveMountOnFiber(
2890+
root,
2891+
fiber,
2892+
committedLanes,
2893+
committedTransitions,
2894+
);
2895+
} catch (error) {
2896+
captureCommitPhaseError(fiber, fiber.return, error);
29002897
}
2898+
resetCurrentDebugFiberInDEV();
29012899

29022900
if (fiber === subtreeRoot) {
29032901
nextEffect = null;
@@ -2921,197 +2919,212 @@ function commitPassiveMountOnFiber(
29212919
committedLanes: Lanes,
29222920
committedTransitions: Array<Transition> | null,
29232921
): void {
2922+
const flags = finishedWork.flags;
29242923
switch (finishedWork.tag) {
29252924
case FunctionComponent:
29262925
case ForwardRef:
29272926
case SimpleMemoComponent: {
2928-
if (
2929-
enableProfilerTimer &&
2930-
enableProfilerCommitHooks &&
2931-
finishedWork.mode & ProfileMode
2932-
) {
2933-
startPassiveEffectTimer();
2934-
try {
2927+
if (flags & Passive) {
2928+
if (
2929+
enableProfilerTimer &&
2930+
enableProfilerCommitHooks &&
2931+
finishedWork.mode & ProfileMode
2932+
) {
2933+
startPassiveEffectTimer();
2934+
try {
2935+
commitHookEffectListMount(
2936+
HookPassive | HookHasEffect,
2937+
finishedWork,
2938+
);
2939+
} finally {
2940+
recordPassiveEffectDuration(finishedWork);
2941+
}
2942+
} else {
29352943
commitHookEffectListMount(HookPassive | HookHasEffect, finishedWork);
2936-
} finally {
2937-
recordPassiveEffectDuration(finishedWork);
29382944
}
2939-
} else {
2940-
commitHookEffectListMount(HookPassive | HookHasEffect, finishedWork);
29412945
}
29422946
break;
29432947
}
29442948
case HostRoot: {
2945-
if (enableCache) {
2946-
let previousCache: Cache | null = null;
2947-
if (finishedWork.alternate !== null) {
2948-
previousCache = finishedWork.alternate.memoizedState.cache;
2949-
}
2950-
const nextCache = finishedWork.memoizedState.cache;
2951-
// Retain/release the root cache.
2952-
// Note that on initial mount, previousCache and nextCache will be the same
2953-
// and this retain won't occur. To counter this, we instead retain the HostRoot's
2954-
// initial cache when creating the root itself (see createFiberRoot() in
2955-
// ReactFiberRoot.js). Subsequent updates that change the cache are reflected
2956-
// here, such that previous/next caches are retained correctly.
2957-
if (nextCache !== previousCache) {
2958-
retainCache(nextCache);
2959-
if (previousCache != null) {
2960-
releaseCache(previousCache);
2949+
if (flags & Passive) {
2950+
if (enableCache) {
2951+
let previousCache: Cache | null = null;
2952+
if (finishedWork.alternate !== null) {
2953+
previousCache = finishedWork.alternate.memoizedState.cache;
2954+
}
2955+
const nextCache = finishedWork.memoizedState.cache;
2956+
// Retain/release the root cache.
2957+
// Note that on initial mount, previousCache and nextCache will be the same
2958+
// and this retain won't occur. To counter this, we instead retain the HostRoot's
2959+
// initial cache when creating the root itself (see createFiberRoot() in
2960+
// ReactFiberRoot.js). Subsequent updates that change the cache are reflected
2961+
// here, such that previous/next caches are retained correctly.
2962+
if (nextCache !== previousCache) {
2963+
retainCache(nextCache);
2964+
if (previousCache != null) {
2965+
releaseCache(previousCache);
2966+
}
29612967
}
29622968
}
2963-
}
29642969

2965-
if (enableTransitionTracing) {
2966-
// Get the transitions that were initiatized during the render
2967-
// and add a start transition callback for each of them
2968-
const root = finishedWork.stateNode;
2969-
const incompleteTransitions = root.incompleteTransitions;
2970-
// Initial render
2971-
if (committedTransitions !== null) {
2972-
committedTransitions.forEach(transition => {
2973-
addTransitionStartCallbackToPendingTransition(transition);
2970+
if (enableTransitionTracing) {
2971+
// Get the transitions that were initiatized during the render
2972+
// and add a start transition callback for each of them
2973+
const root = finishedWork.stateNode;
2974+
const incompleteTransitions = root.incompleteTransitions;
2975+
// Initial render
2976+
if (committedTransitions !== null) {
2977+
committedTransitions.forEach(transition => {
2978+
addTransitionStartCallbackToPendingTransition(transition);
2979+
});
2980+
2981+
clearTransitionsForLanes(finishedRoot, committedLanes);
2982+
}
2983+
2984+
incompleteTransitions.forEach((markerInstance, transition) => {
2985+
const pendingBoundaries = markerInstance.pendingBoundaries;
2986+
if (pendingBoundaries === null || pendingBoundaries.size === 0) {
2987+
addTransitionCompleteCallbackToPendingTransition(transition);
2988+
incompleteTransitions.delete(transition);
2989+
}
29742990
});
29752991

29762992
clearTransitionsForLanes(finishedRoot, committedLanes);
29772993
}
2978-
2979-
incompleteTransitions.forEach((markerInstance, transition) => {
2980-
const pendingBoundaries = markerInstance.pendingBoundaries;
2981-
if (pendingBoundaries === null || pendingBoundaries.size === 0) {
2982-
addTransitionCompleteCallbackToPendingTransition(transition);
2983-
incompleteTransitions.delete(transition);
2984-
}
2985-
});
2986-
2987-
clearTransitionsForLanes(finishedRoot, committedLanes);
29882994
}
29892995
break;
29902996
}
29912997
case LegacyHiddenComponent:
29922998
case OffscreenComponent: {
2993-
if (enableCache) {
2994-
let previousCache: Cache | null = null;
2995-
if (
2996-
finishedWork.alternate !== null &&
2997-
finishedWork.alternate.memoizedState !== null &&
2998-
finishedWork.alternate.memoizedState.cachePool !== null
2999-
) {
3000-
previousCache = finishedWork.alternate.memoizedState.cachePool.pool;
3001-
}
3002-
let nextCache: Cache | null = null;
3003-
if (
3004-
finishedWork.memoizedState !== null &&
3005-
finishedWork.memoizedState.cachePool !== null
3006-
) {
3007-
nextCache = finishedWork.memoizedState.cachePool.pool;
3008-
}
3009-
// Retain/release the cache used for pending (suspended) nodes.
3010-
// Note that this is only reached in the non-suspended/visible case:
3011-
// when the content is suspended/hidden, the retain/release occurs
3012-
// via the parent Suspense component (see case above).
3013-
if (nextCache !== previousCache) {
3014-
if (nextCache != null) {
3015-
retainCache(nextCache);
2999+
if (flags & Passive) {
3000+
if (enableCache) {
3001+
let previousCache: Cache | null = null;
3002+
if (
3003+
finishedWork.alternate !== null &&
3004+
finishedWork.alternate.memoizedState !== null &&
3005+
finishedWork.alternate.memoizedState.cachePool !== null
3006+
) {
3007+
previousCache = finishedWork.alternate.memoizedState.cachePool.pool;
30163008
}
3017-
if (previousCache != null) {
3018-
releaseCache(previousCache);
3009+
let nextCache: Cache | null = null;
3010+
if (
3011+
finishedWork.memoizedState !== null &&
3012+
finishedWork.memoizedState.cachePool !== null
3013+
) {
3014+
nextCache = finishedWork.memoizedState.cachePool.pool;
3015+
}
3016+
// Retain/release the cache used for pending (suspended) nodes.
3017+
// Note that this is only reached in the non-suspended/visible case:
3018+
// when the content is suspended/hidden, the retain/release occurs
3019+
// via the parent Suspense component (see case above).
3020+
if (nextCache !== previousCache) {
3021+
if (nextCache != null) {
3022+
retainCache(nextCache);
3023+
}
3024+
if (previousCache != null) {
3025+
releaseCache(previousCache);
3026+
}
30193027
}
30203028
}
3021-
}
30223029

3023-
if (enableTransitionTracing) {
3024-
const isFallback = finishedWork.memoizedState;
3025-
const queue: OffscreenQueue | null = (finishedWork.updateQueue: any);
3026-
const instance: OffscreenInstance = finishedWork.stateNode;
3030+
if (enableTransitionTracing) {
3031+
const isFallback = finishedWork.memoizedState;
3032+
const queue: OffscreenQueue | null = (finishedWork.updateQueue: any);
3033+
const instance: OffscreenInstance = finishedWork.stateNode;
30273034

3028-
if (queue !== null) {
3029-
if (isFallback) {
3030-
const transitions = queue.transitions;
3031-
if (transitions !== null) {
3032-
transitions.forEach(transition => {
3033-
// Add all the transitions saved in the update queue during
3034-
// the render phase (ie the transitions associated with this boundary)
3035-
// into the transitions set.
3036-
if (instance.transitions === null) {
3037-
instance.transitions = new Set();
3038-
}
3039-
instance.transitions.add(transition);
3040-
});
3041-
}
3035+
if (queue !== null) {
3036+
if (isFallback) {
3037+
const transitions = queue.transitions;
3038+
if (transitions !== null) {
3039+
transitions.forEach(transition => {
3040+
// Add all the transitions saved in the update queue during
3041+
// the render phase (ie the transitions associated with this boundary)
3042+
// into the transitions set.
3043+
if (instance.transitions === null) {
3044+
instance.transitions = new Set();
3045+
}
3046+
instance.transitions.add(transition);
3047+
});
3048+
}
30423049

3043-
const markerInstances = queue.markerInstances;
3044-
if (markerInstances !== null) {
3045-
markerInstances.forEach(markerInstance => {
3046-
const markerTransitions = markerInstance.transitions;
3047-
// There should only be a few tracing marker transitions because
3048-
// they should be only associated with the transition that
3049-
// caused them
3050-
if (markerTransitions !== null) {
3051-
markerTransitions.forEach(transition => {
3052-
if (instance.transitions === null) {
3053-
instance.transitions = new Set();
3054-
} else if (instance.transitions.has(transition)) {
3055-
if (markerInstance.pendingBoundaries === null) {
3056-
markerInstance.pendingBoundaries = new Map();
3050+
const markerInstances = queue.markerInstances;
3051+
if (markerInstances !== null) {
3052+
markerInstances.forEach(markerInstance => {
3053+
const markerTransitions = markerInstance.transitions;
3054+
// There should only be a few tracing marker transitions because
3055+
// they should be only associated with the transition that
3056+
// caused them
3057+
if (markerTransitions !== null) {
3058+
markerTransitions.forEach(transition => {
3059+
if (instance.transitions === null) {
3060+
instance.transitions = new Set();
3061+
} else if (instance.transitions.has(transition)) {
3062+
if (markerInstance.pendingBoundaries === null) {
3063+
markerInstance.pendingBoundaries = new Map();
3064+
}
3065+
if (instance.pendingMarkers === null) {
3066+
instance.pendingMarkers = new Set();
3067+
}
3068+
3069+
instance.pendingMarkers.add(markerInstance);
30573070
}
3058-
if (instance.pendingMarkers === null) {
3059-
instance.pendingMarkers = new Set();
3060-
}
3061-
3062-
instance.pendingMarkers.add(markerInstance);
3063-
}
3064-
});
3065-
}
3066-
});
3071+
});
3072+
}
3073+
});
3074+
}
30673075
}
3076+
3077+
finishedWork.updateQueue = null;
30683078
}
30693079

3070-
finishedWork.updateQueue = null;
3080+
commitTransitionProgress(finishedWork);
30713081
}
3072-
3073-
commitTransitionProgress(finishedWork);
30743082
}
3075-
30763083
break;
30773084
}
30783085
case CacheComponent: {
3079-
if (enableCache) {
3080-
let previousCache: Cache | null = null;
3081-
if (finishedWork.alternate !== null) {
3082-
previousCache = finishedWork.alternate.memoizedState.cache;
3083-
}
3084-
const nextCache = finishedWork.memoizedState.cache;
3085-
// Retain/release the cache. In theory the cache component
3086-
// could be "borrowing" a cache instance owned by some parent,
3087-
// in which case we could avoid retaining/releasing. But it
3088-
// is non-trivial to determine when that is the case, so we
3089-
// always retain/release.
3090-
if (nextCache !== previousCache) {
3091-
retainCache(nextCache);
3092-
if (previousCache != null) {
3093-
releaseCache(previousCache);
3086+
if (flags & Passive) {
3087+
if (enableCache) {
3088+
let previousCache: Cache | null = null;
3089+
if (finishedWork.alternate !== null) {
3090+
previousCache = finishedWork.alternate.memoizedState.cache;
3091+
}
3092+
const nextCache = finishedWork.memoizedState.cache;
3093+
// Retain/release the cache. In theory the cache component
3094+
// could be "borrowing" a cache instance owned by some parent,
3095+
// in which case we could avoid retaining/releasing. But it
3096+
// is non-trivial to determine when that is the case, so we
3097+
// always retain/release.
3098+
if (nextCache !== previousCache) {
3099+
retainCache(nextCache);
3100+
if (previousCache != null) {
3101+
releaseCache(previousCache);
3102+
}
30943103
}
30953104
}
30963105
}
30973106
break;
30983107
}
30993108
case TracingMarkerComponent: {
31003109
if (enableTransitionTracing) {
3101-
// Get the transitions that were initiatized during the render
3102-
// and add a start transition callback for each of them
3103-
const instance = finishedWork.stateNode;
3104-
if (
3105-
instance.transitions !== null &&
3106-
(instance.pendingBoundaries === null ||
3107-
instance.pendingBoundaries.size === 0)
3108-
) {
3109-
addMarkerCompleteCallbackToPendingTransition(
3110-
finishedWork.memoizedProps.name,
3111-
instance.transitions,
3112-
);
3113-
instance.transitions = null;
3114-
instance.pendingBoundaries = null;
3110+
if (flags & Passive) {
3111+
// Get the transitions that were initiatized during the render
3112+
// and add a start transition callback for each of them
3113+
const instance = finishedWork.stateNode;
3114+
if (
3115+
instance.transitions !== null &&
3116+
(instance.pendingBoundaries === null ||
3117+
instance.pendingBoundaries.size === 0)
3118+
) {
3119+
instance.transitions.forEach(transition => {
3120+
addMarkerCompleteCallbackToPendingTransition(
3121+
finishedWork.memoizedProps.name,
3122+
instance.transitions,
3123+
);
3124+
});
3125+
instance.transitions = null;
3126+
instance.pendingBoundaries = null;
3127+
}
31153128
}
31163129
}
31173130
break;

0 commit comments

Comments
 (0)