Skip to content

Commit

Permalink
Detach sibling pointers in old child list
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Mar 23, 2021
1 parent 3bec5ab commit 7effaa2
Show file tree
Hide file tree
Showing 12 changed files with 109 additions and 10 deletions.
31 changes: 31 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import {
enableSuspenseCallback,
enableScopeAPI,
enableStrictEffects,
<<<<<<< current
enableStrongMemoryCleanup,
=======
enableDetachOldChildList,
>>>>>>> patched
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
Expand Down Expand Up @@ -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;
}
}
Expand Down
51 changes: 41 additions & 10 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -35,7 +35,11 @@ import {
enableSuspenseCallback,
enableScopeAPI,
enableStrictEffects,
<<<<<<< current
enableStrongMemoryCleanup,
=======
enableDetachOldChildList,
>>>>>>> patched
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
Expand Down Expand Up @@ -87,18 +91,18 @@ 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,
recordLayoutEffectDuration,
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,
Expand Down Expand Up @@ -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<mixed> | null = null;
Expand Down Expand Up @@ -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;
}
}
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const {
enableSyncMicroTasks,
enableLazyContextPropagation,
enableStrongMemoryCleanup,
enableDetachOldChildList,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down

0 comments on commit 7effaa2

Please sign in to comment.