Skip to content

Commit 7effaa2

Browse files
committed
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.
1 parent 3bec5ab commit 7effaa2

12 files changed

+109
-10
lines changed

Diff for: packages/react-reconciler/src/ReactFiberCommitWork.new.js

+31
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ import {
3535
enableSuspenseCallback,
3636
enableScopeAPI,
3737
enableStrictEffects,
38+
<<<<<<< current
3839
enableStrongMemoryCleanup,
40+
=======
41+
enableDetachOldChildList,
42+
>>>>>>> patched
3943
} from 'shared/ReactFeatureFlags';
4044
import {
4145
FunctionComponent,
@@ -2323,6 +2327,33 @@ function commitPassiveUnmountEffects_begin() {
23232327
detachFiberAfterEffects(alternate);
23242328
}
23252329
}
2330+
2331+
if (enableDetachOldChildList) {
2332+
// A fiber was deleted from this parent fiber, but it's still part of
2333+
// the previous (alternate) parent fiber's list of children. Because
2334+
// children are a linked list, an earlier sibling that's still alive
2335+
// will be connected to the deleted fiber via its `alternate`:
2336+
//
2337+
// live fiber
2338+
// --alternate--> previous live fiber
2339+
// --sibling--> deleted fiber
2340+
//
2341+
// We can't disconnect `alternate` on nodes that haven't been deleted
2342+
// yet, but we can disconnect the `sibling` and `child` pointers.
2343+
const previousFiber = fiber.alternate;
2344+
if (previousFiber !== null) {
2345+
let detachedChild = previousFiber.child;
2346+
if (detachedChild !== null) {
2347+
previousFiber.child = null;
2348+
do {
2349+
const detachedSibling = detachedChild.sibling;
2350+
detachedChild.sibling = null;
2351+
detachedChild = detachedSibling;
2352+
} while (detachedChild !== null);
2353+
}
2354+
}
2355+
}
2356+
23262357
nextEffect = fiber;
23272358
}
23282359
}

Diff for: packages/react-reconciler/src/ReactFiberCommitWork.old.js

+41-10
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ import type {
1717
} from './ReactFiberHostConfig';
1818
import type {Fiber} from './ReactInternalTypes';
1919
import type {FiberRoot} from './ReactInternalTypes';
20-
import type {LanePriority, Lanes} from './ReactFiberLane.old';
21-
import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
22-
import type {UpdateQueue} from './ReactUpdateQueue.old';
23-
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old';
20+
import type {LanePriority, Lanes} from './ReactFiberLane.new';
21+
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
22+
import type {UpdateQueue} from './ReactUpdateQueue.new';
23+
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
2424
import type {Wakeable} from 'shared/ReactTypes';
2525
import type {OffscreenState} from './ReactFiberOffscreenComponent';
2626
import type {HookFlags} from './ReactHookEffectTags';
@@ -35,7 +35,11 @@ import {
3535
enableSuspenseCallback,
3636
enableScopeAPI,
3737
enableStrictEffects,
38+
<<<<<<< current
3839
enableStrongMemoryCleanup,
40+
=======
41+
enableDetachOldChildList,
42+
>>>>>>> patched
3943
} from 'shared/ReactFeatureFlags';
4044
import {
4145
FunctionComponent,
@@ -87,18 +91,18 @@ import {
8791
setCurrentFiber as setCurrentDebugFiberInDEV,
8892
} from './ReactCurrentFiber';
8993

90-
import {onCommitUnmount} from './ReactFiberDevToolsHook.old';
91-
import {resolveDefaultProps} from './ReactFiberLazyComponent.old';
94+
import {onCommitUnmount} from './ReactFiberDevToolsHook.new';
95+
import {resolveDefaultProps} from './ReactFiberLazyComponent.new';
9296
import {
9397
isCurrentUpdateNested,
9498
getCommitTime,
9599
recordLayoutEffectDuration,
96100
startLayoutEffectTimer,
97101
recordPassiveEffectDuration,
98102
startPassiveEffectTimer,
99-
} from './ReactProfilerTimer.old';
103+
} from './ReactProfilerTimer.new';
100104
import {ProfileMode} from './ReactTypeOfMode';
101-
import {commitUpdateQueue} from './ReactUpdateQueue.old';
105+
import {commitUpdateQueue} from './ReactUpdateQueue.new';
102106
import {
103107
getPublicInstance,
104108
supportsMutation,
@@ -134,14 +138,14 @@ import {
134138
resolveRetryWakeable,
135139
markCommitTimeOfFallback,
136140
enqueuePendingPassiveProfilerEffect,
137-
} from './ReactFiberWorkLoop.old';
141+
} from './ReactFiberWorkLoop.new';
138142
import {
139143
NoFlags as NoHookEffect,
140144
HasEffect as HookHasEffect,
141145
Layout as HookLayout,
142146
Passive as HookPassive,
143147
} from './ReactHookEffectTags';
144-
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old';
148+
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
145149
import {doesFiberContain} from './ReactFiberTreeReflection';
146150

147151
let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
@@ -2323,6 +2327,33 @@ function commitPassiveUnmountEffects_begin() {
23232327
detachFiberAfterEffects(alternate);
23242328
}
23252329
}
2330+
2331+
if (enableDetachOldChildList) {
2332+
// A fiber was deleted from this parent fiber, but it's still part of
2333+
// the previous (alternate) parent fiber's list of children. Because
2334+
// children are a linked list, an earlier sibling that's still alive
2335+
// will be connected to the deleted fiber via its `alternate`:
2336+
//
2337+
// live fiber
2338+
// --alternate--> previous live fiber
2339+
// --sibling--> deleted fiber
2340+
//
2341+
// We can't disconnect `alternate` on nodes that haven't been deleted
2342+
// yet, but we can disconnect the `sibling` and `child` pointers.
2343+
const previousFiber = fiber.alternate;
2344+
if (previousFiber !== null) {
2345+
let detachedChild = previousFiber.child;
2346+
if (detachedChild !== null) {
2347+
previousFiber.child = null;
2348+
do {
2349+
const detachedSibling = detachedChild.sibling;
2350+
detachedChild.sibling = null;
2351+
detachedChild = detachedSibling;
2352+
} while (detachedChild !== null);
2353+
}
2354+
}
2355+
}
2356+
23262357
nextEffect = fiber;
23272358
}
23282359
}

Diff for: packages/shared/ReactFeatureFlags.js

+4
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,12 @@ export const disableNativeComponentFrames = false;
112112
// If there are no still-mounted boundaries, the errors should be rethrown.
113113
export const skipUnmountedBoundaries = false;
114114

115+
<<<<<<< current
115116
// When a node is unmounted, recurse into the Fiber subtree and clean out references.
116117
export const enableStrongMemoryCleanup = false;
118+
=======
119+
export const enableDetachOldChildList = false;
120+
>>>>>>> patched
117121

118122
// --------------------------
119123
// Future APIs to be deprecated

Diff for: packages/shared/forks/ReactFeatureFlags.native-fb.js

+4
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ export const enableLegacyFBSupport = false;
4646
export const enableFilterEmptyStringAttributesDOM = false;
4747
export const disableNativeComponentFrames = false;
4848
export const skipUnmountedBoundaries = false;
49+
<<<<<<< current
4950
export const enableStrongMemoryCleanup = false;
51+
=======
52+
export const enableDetachOldChildList = false;
53+
>>>>>>> patched
5054

5155
export const enableNewReconciler = false;
5256
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.native-oss.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = false;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.test-renderer.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = false;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.test-renderer.native.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = false;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = false;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.testing.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = false;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = false;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.testing.www.js

+4
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ export const enableLegacyFBSupport = !__EXPERIMENTAL__;
4545
export const enableFilterEmptyStringAttributesDOM = false;
4646
export const disableNativeComponentFrames = false;
4747
export const skipUnmountedBoundaries = true;
48+
<<<<<<< current
4849
export const enableStrongMemoryCleanup = false;
50+
=======
51+
export const enableDetachOldChildList = false;
52+
>>>>>>> patched
4953

5054
export const enableNewReconciler = false;
5155
export const deferRenderPhaseUpdateToNextBatch = true;

Diff for: packages/shared/forks/ReactFeatureFlags.www-dynamic.js

+4
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ export const disableNativeComponentFrames = false;
5252
export const createRootStrictEffectsByDefault = false;
5353
export const enableStrictEffects = false;
5454
export const enableUseRefAccessWarning = __VARIANT__;
55+
<<<<<<< current
5556
export const enableStrongMemoryCleanup = __VARIANT__;
57+
=======
58+
export const enableDetachOldChildList = __VARIANT__;
59+
>>>>>>> patched
5660

5761
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
5862
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;

Diff for: packages/shared/forks/ReactFeatureFlags.www.js

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const {
3434
enableSyncMicroTasks,
3535
enableLazyContextPropagation,
3636
enableStrongMemoryCleanup,
37+
enableDetachOldChildList,
3738
} = dynamicFeatureFlags;
3839

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

0 commit comments

Comments
 (0)