From a16ac10c514f830e461f7d8e33f10bc3e139555f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 22 Mar 2021 20:21:32 -0500 Subject: [PATCH] Detach sibling pointers in old child list When a fiber is deleted, it's still part of the previous (alternate) parent fiber's list of children. Because children are a linked list, an earlier sibling that's still alive will be connected to the deleted fiber via its alternate: live fiber --alternate--> previous live fiber --sibling--> deleted fiber We can't disconnect `alternate` on nodes that haven't been deleted yet, but we can disconnect the `sibling` and `child` pointers. Will use this feature flag to test the memory impact. --- .../src/ReactFiberCommitWork.new.js | 28 +++++++++++++++++++ .../src/ReactFiberCommitWork.old.js | 28 +++++++++++++++++++ packages/shared/ReactFeatureFlags.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 12 files changed, 66 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 1e19f4086e0c..36472020c602 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -36,6 +36,7 @@ import { enableScopeAPI, enableStrictEffects, enableStrongMemoryCleanup, + enableDetachOldChildList, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -2323,6 +2324,33 @@ function commitPassiveUnmountEffects_begin() { detachFiberAfterEffects(alternate); } } + + if (enableDetachOldChildList) { + // A fiber was deleted from this parent fiber, but it's still part of + // the previous (alternate) parent fiber's list of children. Because + // children are a linked list, an earlier sibling that's still alive + // will be connected to the deleted fiber via its `alternate`: + // + // live fiber + // --alternate--> previous live fiber + // --sibling--> deleted fiber + // + // We can't disconnect `alternate` on nodes that haven't been deleted + // yet, but we can disconnect the `sibling` and `child` pointers. + const previousFiber = fiber.alternate; + if (previousFiber !== null) { + let detachedChild = previousFiber.child; + if (detachedChild !== null) { + previousFiber.child = null; + do { + const detachedSibling = detachedChild.sibling; + detachedChild.sibling = null; + detachedChild = detachedSibling; + } while (detachedChild !== null); + } + } + } + nextEffect = fiber; } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 8d187832e4d8..b21ded003575 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -36,6 +36,7 @@ import { enableScopeAPI, enableStrictEffects, enableStrongMemoryCleanup, + enableDetachOldChildList, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -2323,6 +2324,33 @@ function commitPassiveUnmountEffects_begin() { detachFiberAfterEffects(alternate); } } + + if (enableDetachOldChildList) { + // A fiber was deleted from this parent fiber, but it's still part of + // the previous (alternate) parent fiber's list of children. Because + // children are a linked list, an earlier sibling that's still alive + // will be connected to the deleted fiber via its `alternate`: + // + // live fiber + // --alternate--> previous live fiber + // --sibling--> deleted fiber + // + // We can't disconnect `alternate` on nodes that haven't been deleted + // yet, but we can disconnect the `sibling` and `child` pointers. + const previousFiber = fiber.alternate; + if (previousFiber !== null) { + let detachedChild = previousFiber.child; + if (detachedChild !== null) { + previousFiber.child = null; + do { + const detachedSibling = detachedChild.sibling; + detachedChild.sibling = null; + detachedChild = detachedSibling; + } while (detachedChild !== null); + } + } + } + nextEffect = fiber; } } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 2a3d0a337bf8..591a24c3cd31 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -114,6 +114,7 @@ export const skipUnmountedBoundaries = false; // When a node is unmounted, recurse into the Fiber subtree and clean out references. export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; // -------------------------- // Future APIs to be deprecated diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 443c489c0063..913063d587be 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -47,6 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index bac13f5091b7..8714352a1768 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index da03038c0211..0ea1b5fda395 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index eda47cde1139..db62ddc1dc10 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 9b02f6408229..46fd65d18765 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 7bb7ed424c7b..144ce7f4dca8 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 8834238b02b1..188c6bdcffdf 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -46,6 +46,7 @@ export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = true; export const enableStrongMemoryCleanup = false; +export const enableDetachOldChildList = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index f58e0da1e80b..bd666333c5f7 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -53,6 +53,7 @@ export const createRootStrictEffectsByDefault = false; export const enableStrictEffects = false; export const enableUseRefAccessWarning = __VARIANT__; export const enableStrongMemoryCleanup = __VARIANT__; +export const enableDetachOldChildList = __VARIANT__; export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 25b88a019f1d..0f9e27011af0 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -34,6 +34,7 @@ export const { enableSyncMicroTasks, enableLazyContextPropagation, enableStrongMemoryCleanup, + enableDetachOldChildList, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.