Skip to content

Commit 5f21a9f

Browse files
authored
Clean up host pointers in level 2 of clean-up flag (#21112)
The host tree is a cyclical structure. Leaking a single DOM node can retain a large amount of memory. React-managed DOM nodes also point back to a fiber tree. Perf testing suggests that disconnecting these fields has a big memory impact. That suggests leaks in non-React code but since it's hard to completely eliminate those, it may still be worth the extra work to clear these fields. I'm moving this to level 2 to confirm whether this alone is responsible for the memory savings, or if there are other fields that are retaining large amounts of memory. In our plan for removing the alternate model, DOM nodes would not be connected to fibers, except at the root of the whole tree, which is easy to disconnect on deletion. So in that world, we likely won't have to do any additional work.
1 parent 32d6f39 commit 5f21a9f

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,6 +1251,18 @@ function detachFiberAfterEffects(fiber: Fiber) {
12511251
fiber.deletions = null;
12521252
fiber.sibling = null;
12531253

1254+
// The `stateNode` is cyclical because on host nodes it points to the host
1255+
// tree, which has its own pointers to children, parents, and siblings.
1256+
// The other host nodes also point back to fibers, so we should detach that
1257+
// one, too.
1258+
if (fiber.tag === HostComponent) {
1259+
const hostInstance: Instance = fiber.stateNode;
1260+
if (hostInstance !== null) {
1261+
detachDeletedInstance(hostInstance);
1262+
}
1263+
}
1264+
fiber.stateNode = null;
1265+
12541266
// I'm intentionally not clearing the `return` field in this level. We
12551267
// already disconnect the `return` pointer at the root of the deleted
12561268
// subtree (in `detachFiberMutation`). Besides, `return` by itself is not
@@ -1269,15 +1281,6 @@ function detachFiberAfterEffects(fiber: Fiber) {
12691281
// The purpose of this branch is to be super aggressive so we can measure
12701282
// if there's any difference in memory impact. If there is, that could
12711283
// indicate a React leak we don't know about.
1272-
1273-
// For host components, disconnect host instance -> fiber pointer.
1274-
if (fiber.tag === HostComponent) {
1275-
const hostInstance: Instance = fiber.stateNode;
1276-
if (hostInstance !== null) {
1277-
detachDeletedInstance(hostInstance);
1278-
}
1279-
}
1280-
12811284
fiber.return = null;
12821285
fiber.dependencies = null;
12831286
fiber.memoizedProps = null;

packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,6 +1251,18 @@ function detachFiberAfterEffects(fiber: Fiber) {
12511251
fiber.deletions = null;
12521252
fiber.sibling = null;
12531253

1254+
// The `stateNode` is cyclical because on host nodes it points to the host
1255+
// tree, which has its own pointers to children, parents, and siblings.
1256+
// The other host nodes also point back to fibers, so we should detach that
1257+
// one, too.
1258+
if (fiber.tag === HostComponent) {
1259+
const hostInstance: Instance = fiber.stateNode;
1260+
if (hostInstance !== null) {
1261+
detachDeletedInstance(hostInstance);
1262+
}
1263+
}
1264+
fiber.stateNode = null;
1265+
12541266
// I'm intentionally not clearing the `return` field in this level. We
12551267
// already disconnect the `return` pointer at the root of the deleted
12561268
// subtree (in `detachFiberMutation`). Besides, `return` by itself is not
@@ -1269,15 +1281,6 @@ function detachFiberAfterEffects(fiber: Fiber) {
12691281
// The purpose of this branch is to be super aggressive so we can measure
12701282
// if there's any difference in memory impact. If there is, that could
12711283
// indicate a React leak we don't know about.
1272-
1273-
// For host components, disconnect host instance -> fiber pointer.
1274-
if (fiber.tag === HostComponent) {
1275-
const hostInstance: Instance = fiber.stateNode;
1276-
if (hostInstance !== null) {
1277-
detachDeletedInstance(hostInstance);
1278-
}
1279-
}
1280-
12811284
fiber.return = null;
12821285
fiber.dependencies = null;
12831286
fiber.memoizedProps = null;

0 commit comments

Comments
 (0)