From 7effaa25edcadc31acbbc66e95dd6915bbc58b61 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 | 31 +++++++++++ .../src/ReactFiberCommitWork.old.js | 51 +++++++++++++++---- packages/shared/ReactFeatureFlags.js | 4 ++ .../forks/ReactFeatureFlags.native-fb.js | 4 ++ .../forks/ReactFeatureFlags.native-oss.js | 4 ++ .../forks/ReactFeatureFlags.test-renderer.js | 4 ++ .../ReactFeatureFlags.test-renderer.native.js | 4 ++ .../ReactFeatureFlags.test-renderer.www.js | 4 ++ .../shared/forks/ReactFeatureFlags.testing.js | 4 ++ .../forks/ReactFeatureFlags.testing.www.js | 4 ++ .../forks/ReactFeatureFlags.www-dynamic.js | 4 ++ .../shared/forks/ReactFeatureFlags.www.js | 1 + 12 files changed, 109 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 1e19f4086e0c7..72e75a8184a48 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -35,7 +35,11 @@ import { enableSuspenseCallback, enableScopeAPI, enableStrictEffects, +<<<<<<< current enableStrongMemoryCleanup, +======= + enableDetachOldChildList, +>>>>>>> patched } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -2323,6 +2327,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 8d187832e4d84..72e75a8184a48 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -17,10 +17,10 @@ import type { } from './ReactFiberHostConfig'; import type {Fiber} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; -import type {LanePriority, Lanes} from './ReactFiberLane.old'; -import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; -import type {UpdateQueue} from './ReactUpdateQueue.old'; -import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; +import type {LanePriority, Lanes} from './ReactFiberLane.new'; +import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; +import type {UpdateQueue} from './ReactUpdateQueue.new'; +import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; import type {Wakeable} from 'shared/ReactTypes'; import type {OffscreenState} from './ReactFiberOffscreenComponent'; import type {HookFlags} from './ReactHookEffectTags'; @@ -35,7 +35,11 @@ import { enableSuspenseCallback, enableScopeAPI, enableStrictEffects, +<<<<<<< current enableStrongMemoryCleanup, +======= + enableDetachOldChildList, +>>>>>>> patched } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -87,8 +91,8 @@ import { setCurrentFiber as setCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; -import {onCommitUnmount} from './ReactFiberDevToolsHook.old'; -import {resolveDefaultProps} from './ReactFiberLazyComponent.old'; +import {onCommitUnmount} from './ReactFiberDevToolsHook.new'; +import {resolveDefaultProps} from './ReactFiberLazyComponent.new'; import { isCurrentUpdateNested, getCommitTime, @@ -96,9 +100,9 @@ import { startLayoutEffectTimer, recordPassiveEffectDuration, startPassiveEffectTimer, -} from './ReactProfilerTimer.old'; +} from './ReactProfilerTimer.new'; import {ProfileMode} from './ReactTypeOfMode'; -import {commitUpdateQueue} from './ReactUpdateQueue.old'; +import {commitUpdateQueue} from './ReactUpdateQueue.new'; import { getPublicInstance, supportsMutation, @@ -134,14 +138,14 @@ import { resolveRetryWakeable, markCommitTimeOfFallback, enqueuePendingPassiveProfilerEffect, -} from './ReactFiberWorkLoop.old'; +} from './ReactFiberWorkLoop.new'; import { NoFlags as NoHookEffect, HasEffect as HookHasEffect, Layout as HookLayout, Passive as HookPassive, } from './ReactHookEffectTags'; -import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old'; +import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new'; import {doesFiberContain} from './ReactFiberTreeReflection'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; @@ -2323,6 +2327,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 2a3d0a337bf88..a3e5ffec9075a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -112,8 +112,12 @@ export const disableNativeComponentFrames = false; // If there are no still-mounted boundaries, the errors should be rethrown. export const skipUnmountedBoundaries = false; +<<<<<<< current // When a node is unmounted, recurse into the Fiber subtree and clean out references. export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched // -------------------------- // Future APIs to be deprecated diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 443c489c0063f..8b42bc76b2165 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -46,7 +46,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 bac13f5091b78..590a292833408 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 da03038c0211a..5b9b1be3f0f8d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 eda47cde1139b..99da0d255fb88 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 9b02f6408229f..12d5d90e5a8c3 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 7bb7ed424c7bd..235382aab657f 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 8834238b02b14..59d66eebde234 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -45,7 +45,11 @@ export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = true; +<<<<<<< current export const enableStrongMemoryCleanup = false; +======= +export const enableDetachOldChildList = false; +>>>>>>> patched 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 f58e0da1e80b9..60e12b53098c3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -52,7 +52,11 @@ export const disableNativeComponentFrames = false; export const createRootStrictEffectsByDefault = false; export const enableStrictEffects = false; export const enableUseRefAccessWarning = __VARIANT__; +<<<<<<< current export const enableStrongMemoryCleanup = __VARIANT__; +======= +export const enableDetachOldChildList = __VARIANT__; +>>>>>>> patched 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 25b88a019f1da..0f9e27011af06 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.