@samitouri / QOS-React-2 / commits / 02206099af

Use recursion to traverse during passive unmount phase (#24918)

This converts the layout phase to iterate over its effects recursively instead of iteratively. This makes it easier to track contextual information, like whether a fiber is inside a hidden tree. We already made this change for several other phases, like mutation and layout mount. See 481dece for more context.

Andrew Clark committed Jul 14, 2022 at 11:08 UTC 02206099afe9bf54435fe0bd978a4ccf3df9cace
2 files changed +154 -166
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+77 -83
@@ -3144,85 +3144,72 @@ function commitPassiveMountOnFiber(
3144 }
3145 }
3146
3147 -export function commitPassiveUnmountEffects(firstChild: Fiber): void {
3148 - nextEffect = firstChild;
3149 - commitPassiveUnmountEffects_begin();
3147 +export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
3148 + setCurrentDebugFiberInDEV(finishedWork);
3149 + commitPassiveUnmountOnFiber(finishedWork);
3150 + resetCurrentDebugFiberInDEV();
3151 }
3152
3152 -function commitPassiveUnmountEffects_begin() {
3153 - while (nextEffect !== null) {
3154 - const fiber = nextEffect;
3155 - const child = fiber.child;
3153 +function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
3154 + // Deletions effects can be scheduled on any fiber type. They need to happen
3155 + // before the children effects have fired.
3156 + const deletions = parentFiber.deletions;
3157
3157 - if ((nextEffect.flags & ChildDeletion) !== NoFlags) {
3158 - const deletions = fiber.deletions;
3159 - if (deletions !== null) {
3160 - for (let i = 0; i < deletions.length; i++) {
3161 - const fiberToDelete = deletions[i];
3162 - nextEffect = fiberToDelete;
3158 + if ((parentFiber.flags & ChildDeletion) !== NoFlags) {
3159 + if (deletions !== null) {
3160 + for (let i = 0; i < deletions.length; i++) {
3161 + const childToDelete = deletions[i];
3162 + try {
3163 + // TODO: Convert this to use recursion
3164 + nextEffect = childToDelete;
3165 commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
3164 - fiberToDelete,
3165 - fiber,
3166 + childToDelete,
3167 + parentFiber,
3168 );
3169 + } catch (error) {
3170 + captureCommitPhaseError(childToDelete, parentFiber, error);
3171 }
3168 -
3169 - if (deletedTreeCleanUpLevel >= 1) {
3170 - // A fiber was deleted from this parent fiber, but it's still part of
3171 - // the previous (alternate) parent fiber's list of children. Because
3172 - // children are a linked list, an earlier sibling that's still alive
3173 - // will be connected to the deleted fiber via its `alternate`:
3174 - //
3175 - // live fiber
3176 - // --alternate--> previous live fiber
3177 - // --sibling--> deleted fiber
3178 - //
3179 - // We can't disconnect `alternate` on nodes that haven't been deleted
3180 - // yet, but we can disconnect the `sibling` and `child` pointers.
3181 - const previousFiber = fiber.alternate;
3182 - if (previousFiber !== null) {
3183 - let detachedChild = previousFiber.child;
3184 - if (detachedChild !== null) {
3185 - previousFiber.child = null;
3186 - do {
3187 - const detachedSibling = detachedChild.sibling;
3188 - detachedChild.sibling = null;
3189 - detachedChild = detachedSibling;
3190 - } while (detachedChild !== null);
3191 - }
3192 - }
3193 - }
3194 -
3195 - nextEffect = fiber;
3172 }
3173 }
3174
3199 - if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
3200 - child.return = fiber;
3201 - nextEffect = child;
3202 - } else {
3203 - commitPassiveUnmountEffects_complete();
3175 + if (deletedTreeCleanUpLevel >= 1) {
3176 + // A fiber was deleted from this parent fiber, but it's still part of
3177 + // the previous (alternate) parent fiber's list of children. Because
3178 + // children are a linked list, an earlier sibling that's still alive
3179 + // will be connected to the deleted fiber via its `alternate`:
3180 + //
3181 + // live fiber
3182 + // --alternate--> previous live fiber
3183 + // --sibling--> deleted fiber
3184 + //
3185 + // We can't disconnect `alternate` on nodes that haven't been deleted
3186 + // yet, but we can disconnect the `sibling` and `child` pointers.
3187 + const previousFiber = parentFiber.alternate;
3188 + if (previousFiber !== null) {
3189 + let detachedChild = previousFiber.child;
3190 + if (detachedChild !== null) {
3191 + previousFiber.child = null;
3192 + do {
3193 + const detachedSibling = detachedChild.sibling;
3194 + detachedChild.sibling = null;
3195 + detachedChild = detachedSibling;
3196 + } while (detachedChild !== null);
3197 + }
3198 + }
3199 }
3200 }
3206 -}
3207 -
3208 -function commitPassiveUnmountEffects_complete() {
3209 - while (nextEffect !== null) {
3210 - const fiber = nextEffect;
3211 - if ((fiber.flags & Passive) !== NoFlags) {
3212 - setCurrentDebugFiberInDEV(fiber);
3213 - commitPassiveUnmountOnFiber(fiber);
3214 - resetCurrentDebugFiberInDEV();
3215 - }
3201
3217 - const sibling = fiber.sibling;
3218 - if (sibling !== null) {
3219 - sibling.return = fiber.return;
3220 - nextEffect = sibling;
3221 - return;
3202 + const prevDebugFiber = getCurrentDebugFiberInDEV();
3203 + // TODO: Split PassiveMask into separate masks for mount and unmount?
3204 + if (parentFiber.subtreeFlags & PassiveMask) {
3205 + let child = parentFiber.child;
3206 + while (child !== null) {
3207 + setCurrentDebugFiberInDEV(child);
3208 + commitPassiveUnmountOnFiber(child);
3209 + child = child.sibling;
3210 }
3223 -
3224 - nextEffect = fiber.return;
3211 }
3212 + setCurrentDebugFiberInDEV(prevDebugFiber);
3213 }
3214
3215 function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
@@ -3230,27 +3217,34 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
3217 case FunctionComponent:
3218 case ForwardRef:
3219 case SimpleMemoComponent: {
3233 - if (
3234 - enableProfilerTimer &&
3235 - enableProfilerCommitHooks &&
3236 - finishedWork.mode & ProfileMode
3237 - ) {
3238 - startPassiveEffectTimer();
3239 - commitHookEffectListUnmount(
3240 - HookPassive | HookHasEffect,
3241 - finishedWork,
3242 - finishedWork.return,
3243 - );
3244 - recordPassiveEffectDuration(finishedWork);
3245 - } else {
3246 - commitHookEffectListUnmount(
3247 - HookPassive | HookHasEffect,
3248 - finishedWork,
3249 - finishedWork.return,
3250 - );
3220 + recursivelyTraversePassiveUnmountEffects(finishedWork);
3221 + if (finishedWork.flags & Passive) {
3222 + if (
3223 + enableProfilerTimer &&
3224 + enableProfilerCommitHooks &&
3225 + finishedWork.mode & ProfileMode
3226 + ) {
3227 + startPassiveEffectTimer();
3228 + commitHookEffectListUnmount(
3229 + HookPassive | HookHasEffect,
3230 + finishedWork,
3231 + finishedWork.return,
3232 + );
3233 + recordPassiveEffectDuration(finishedWork);
3234 + } else {
3235 + commitHookEffectListUnmount(
3236 + HookPassive | HookHasEffect,
3237 + finishedWork,
3238 + finishedWork.return,
3239 + );
3240 + }
3241 }
3242 break;
3243 }
3244 + default: {
3245 + recursivelyTraversePassiveUnmountEffects(finishedWork);
3246 + break;
3247 + }
3248 }
3249 }
3250
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+77 -83
@@ -3144,85 +3144,72 @@ function commitPassiveMountOnFiber(
3144 }
3145 }
3146
3147 -export function commitPassiveUnmountEffects(firstChild: Fiber): void {
3148 - nextEffect = firstChild;
3149 - commitPassiveUnmountEffects_begin();
3147 +export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
3148 + setCurrentDebugFiberInDEV(finishedWork);
3149 + commitPassiveUnmountOnFiber(finishedWork);
3150 + resetCurrentDebugFiberInDEV();
3151 }
3152
3152 -function commitPassiveUnmountEffects_begin() {
3153 - while (nextEffect !== null) {
3154 - const fiber = nextEffect;
3155 - const child = fiber.child;
3153 +function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void {
3154 + // Deletions effects can be scheduled on any fiber type. They need to happen
3155 + // before the children effects have fired.
3156 + const deletions = parentFiber.deletions;
3157
3157 - if ((nextEffect.flags & ChildDeletion) !== NoFlags) {
3158 - const deletions = fiber.deletions;
3159 - if (deletions !== null) {
3160 - for (let i = 0; i < deletions.length; i++) {
3161 - const fiberToDelete = deletions[i];
3162 - nextEffect = fiberToDelete;
3158 + if ((parentFiber.flags & ChildDeletion) !== NoFlags) {
3159 + if (deletions !== null) {
3160 + for (let i = 0; i < deletions.length; i++) {
3161 + const childToDelete = deletions[i];
3162 + try {
3163 + // TODO: Convert this to use recursion
3164 + nextEffect = childToDelete;
3165 commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
3164 - fiberToDelete,
3165 - fiber,
3166 + childToDelete,
3167 + parentFiber,
3168 );
3169 + } catch (error) {
3170 + captureCommitPhaseError(childToDelete, parentFiber, error);
3171 }
3168 -
3169 - if (deletedTreeCleanUpLevel >= 1) {
3170 - // A fiber was deleted from this parent fiber, but it's still part of
3171 - // the previous (alternate) parent fiber's list of children. Because
3172 - // children are a linked list, an earlier sibling that's still alive
3173 - // will be connected to the deleted fiber via its `alternate`:
3174 - //
3175 - // live fiber
3176 - // --alternate--> previous live fiber
3177 - // --sibling--> deleted fiber
3178 - //
3179 - // We can't disconnect `alternate` on nodes that haven't been deleted
3180 - // yet, but we can disconnect the `sibling` and `child` pointers.
3181 - const previousFiber = fiber.alternate;
3182 - if (previousFiber !== null) {
3183 - let detachedChild = previousFiber.child;
3184 - if (detachedChild !== null) {
3185 - previousFiber.child = null;
3186 - do {
3187 - const detachedSibling = detachedChild.sibling;
3188 - detachedChild.sibling = null;
3189 - detachedChild = detachedSibling;
3190 - } while (detachedChild !== null);
3191 - }
3192 - }
3193 - }
3194 -
3195 - nextEffect = fiber;
3172 }
3173 }
3174
3199 - if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
3200 - child.return = fiber;
3201 - nextEffect = child;
3202 - } else {
3203 - commitPassiveUnmountEffects_complete();
3175 + if (deletedTreeCleanUpLevel >= 1) {
3176 + // A fiber was deleted from this parent fiber, but it's still part of
3177 + // the previous (alternate) parent fiber's list of children. Because
3178 + // children are a linked list, an earlier sibling that's still alive
3179 + // will be connected to the deleted fiber via its `alternate`:
3180 + //
3181 + // live fiber
3182 + // --alternate--> previous live fiber
3183 + // --sibling--> deleted fiber
3184 + //
3185 + // We can't disconnect `alternate` on nodes that haven't been deleted
3186 + // yet, but we can disconnect the `sibling` and `child` pointers.
3187 + const previousFiber = parentFiber.alternate;
3188 + if (previousFiber !== null) {
3189 + let detachedChild = previousFiber.child;
3190 + if (detachedChild !== null) {
3191 + previousFiber.child = null;
3192 + do {
3193 + const detachedSibling = detachedChild.sibling;
3194 + detachedChild.sibling = null;
3195 + detachedChild = detachedSibling;
3196 + } while (detachedChild !== null);
3197 + }
3198 + }
3199 }
3200 }
3206 -}
3207 -
3208 -function commitPassiveUnmountEffects_complete() {
3209 - while (nextEffect !== null) {
3210 - const fiber = nextEffect;
3211 - if ((fiber.flags & Passive) !== NoFlags) {
3212 - setCurrentDebugFiberInDEV(fiber);
3213 - commitPassiveUnmountOnFiber(fiber);
3214 - resetCurrentDebugFiberInDEV();
3215 - }
3201
3217 - const sibling = fiber.sibling;
3218 - if (sibling !== null) {
3219 - sibling.return = fiber.return;
3220 - nextEffect = sibling;
3221 - return;
3202 + const prevDebugFiber = getCurrentDebugFiberInDEV();
3203 + // TODO: Split PassiveMask into separate masks for mount and unmount?
3204 + if (parentFiber.subtreeFlags & PassiveMask) {
3205 + let child = parentFiber.child;
3206 + while (child !== null) {
3207 + setCurrentDebugFiberInDEV(child);
3208 + commitPassiveUnmountOnFiber(child);
3209 + child = child.sibling;
3210 }
3223 -
3224 - nextEffect = fiber.return;
3211 }
3212 + setCurrentDebugFiberInDEV(prevDebugFiber);
3213 }
3214
3215 function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
@@ -3230,27 +3217,34 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void {
3217 case FunctionComponent:
3218 case ForwardRef:
3219 case SimpleMemoComponent: {
3233 - if (
3234 - enableProfilerTimer &&
3235 - enableProfilerCommitHooks &&
3236 - finishedWork.mode & ProfileMode
3237 - ) {
3238 - startPassiveEffectTimer();
3239 - commitHookEffectListUnmount(
3240 - HookPassive | HookHasEffect,
3241 - finishedWork,
3242 - finishedWork.return,
3243 - );
3244 - recordPassiveEffectDuration(finishedWork);
3245 - } else {
3246 - commitHookEffectListUnmount(
3247 - HookPassive | HookHasEffect,
3248 - finishedWork,
3249 - finishedWork.return,
3250 - );
3220 + recursivelyTraversePassiveUnmountEffects(finishedWork);
3221 + if (finishedWork.flags & Passive) {
3222 + if (
3223 + enableProfilerTimer &&
3224 + enableProfilerCommitHooks &&
3225 + finishedWork.mode & ProfileMode
3226 + ) {
3227 + startPassiveEffectTimer();
3228 + commitHookEffectListUnmount(
3229 + HookPassive | HookHasEffect,
3230 + finishedWork,
3231 + finishedWork.return,
3232 + );
3233 + recordPassiveEffectDuration(finishedWork);
3234 + } else {
3235 + commitHookEffectListUnmount(
3236 + HookPassive | HookHasEffect,
3237 + finishedWork,
3238 + finishedWork.return,
3239 + );
3240 + }
3241 }
3242 break;
3243 }
3244 + default: {
3245 + recursivelyTraversePassiveUnmountEffects(finishedWork);
3246 + break;
3247 + }
3248 }
3249 }
3250