From 39b04e8f6233376d4da6b03e08580af548decd87 Mon Sep 17 00:00:00 2001 From: Benoit Girard Date: Fri, 19 Mar 2021 16:59:26 -0700 Subject: [PATCH] [Draft] Add feature flag: enableStrongMemoryCleanup Add a feature flag that will test doing a recursive clean of an unmount node. This will disconnect the fiber graph making leaks less severe. --- packages/react-art/src/ReactARTHostConfig.js | 4 ++++ .../src/client/ReactDOMComponentTree.js | 11 ++++++++++ .../src/client/ReactDOMHostConfig.js | 5 +++++ .../src/ReactFabricHostConfig.js | 4 ++++ .../src/ReactNativeHostConfig.js | 4 ++++ .../src/createReactNoop.js | 3 +++ .../src/ReactFiberCommitWork.new.js | 21 ++++++++++++++++++- .../src/ReactFiberCommitWork.old.js | 21 ++++++++++++++++++- .../src/ReactTestHostConfig.js | 4 ++++ packages/shared/ReactFeatureFlags.js | 3 +++ .../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 + 19 files changed, 87 insertions(+), 2 deletions(-) diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index cadf311d54ffd..6a02366979a07 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -471,3 +471,7 @@ export function afterActiveInstanceBlur() { export function preparePortalMount(portalInstance: any): void { // noop } + +export function unmountNode(node: any): void { + // noop +} diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index fbf0a81e29064..1c09f24931aee 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -43,6 +43,17 @@ const internalEventHandlersKey = '__reactEvents$' + randomKey; const internalEventHandlerListenersKey = '__reactListeners$' + randomKey; const internalEventHandlesSetKey = '__reactHandles$' + randomKey; +export function unmountNode( + node: Instance | TextInstance | SuspenseInstance | ReactScopeInstance, +): void { + delete (node: any)[internalInstanceKey]; + delete (node: any)[internalPropsKey]; + delete (node: any)[internalContainerInstanceKey]; + delete (node: any)[internalEventHandlersKey]; + delete (node: any)[internalEventHandlerListenersKey]; + delete (node: any)[internalEventHandlesSetKey]; +} + export function precacheFiberNode( hostInst: Fiber, node: Instance | TextInstance | SuspenseInstance | ReactScopeInstance, diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index dfd29b375a41b..301bd2e72ef8a 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -24,6 +24,7 @@ import { getFiberFromScopeInstance, getInstanceFromNode as getInstanceFromNodeDOMTree, isContainerMarkedAsRoot, + unmountNode as unmountNodeFromDOMTree, } from './ReactDOMComponentTree'; import {hasRole} from './DOMAccessibilityRoles'; import { @@ -1209,3 +1210,7 @@ export function setupIntersectionObserver( }, }; } + +export function unmountNode(node: any): void { + unmountNodeFromDOMTree(node); +} diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 85eb2a741ffac..121480051a4a5 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -481,3 +481,7 @@ export function afterActiveInstanceBlur() { export function preparePortalMount(portalInstance: Instance): void { // noop } + +export function unmountNode(node: any): void { + // noop +} diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index d626a175d9340..f1ec3143020ed 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -538,3 +538,7 @@ export function afterActiveInstanceBlur() { export function preparePortalMount(portalInstance: Instance): void { // noop } + +export function unmountNode(node: any): void { + // noop +} diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 80c888a88e53d..428ece3318265 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -420,6 +420,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { getInstanceFromScope() { throw new Error('Not yet implemented.'); }, + + unmountNode() { + }, }; const hostConfig = useMutation diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 7985366d3f67c..3ba14ef6569bb 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -35,6 +35,7 @@ import { enableSuspenseCallback, enableScopeAPI, enableStrictEffects, + enableStrongMemoryCleanup, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -60,6 +61,9 @@ import { hasCaughtError, clearCaughtError, } from 'shared/ReactErrorUtils'; +import { + unmountNode, +} from './ReactFiberHostConfig'; import { NoFlags, ContentReset, @@ -1212,9 +1216,24 @@ function detachFiberMutation(fiber: Fiber) { fiber.return = null; } -export function detachFiberAfterEffects(fiber: Fiber): void { +export function detachFiberAfterEffects( + fiber: Fiber, + recurseIntoSibbling: ?boolean, +): void { // Null out fields to improve GC for references that may be lingering (e.g. DevTools). // Note that we already cleared the return pointer in detachFiberMutation(). + if (enableStrongMemoryCleanup) { + if (fiber.child) { + detachFiberAfterEffects(fiber.child, true); + } + if (fiber.sibling && recurseIntoSibbling === true) { + detachFiberAfterEffects(fiber.sibling, true); + } + if (fiber.stateNode) { + unmountNode(fiber.stateNode); + } + fiber.return = null; + } fiber.alternate = null; fiber.child = null; fiber.deletions = null; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index d50095b3bebb1..e405d64abcc8f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -35,6 +35,7 @@ import { enableSuspenseCallback, enableScopeAPI, enableStrictEffects, + enableStrongMemoryCleanup, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -60,6 +61,9 @@ import { hasCaughtError, clearCaughtError, } from 'shared/ReactErrorUtils'; +import { + unmountNode, +} from './ReactFiberHostConfig'; import { NoFlags, ContentReset, @@ -1212,9 +1216,24 @@ function detachFiberMutation(fiber: Fiber) { fiber.return = null; } -export function detachFiberAfterEffects(fiber: Fiber): void { +export function detachFiberAfterEffects( + fiber: Fiber, + recurseIntoSibbling: ?boolean, +): void { // Null out fields to improve GC for references that may be lingering (e.g. DevTools). // Note that we already cleared the return pointer in detachFiberMutation(). + if (enableStrongMemoryCleanup) { + if (fiber.child) { + detachFiberAfterEffects(fiber.child, true); + } + if (fiber.sibling && recurseIntoSibbling === true) { + detachFiberAfterEffects(fiber.sibling, true); + } + if (fiber.stateNode) { + unmountNode(fiber.stateNode); + } + fiber.return = null; + } fiber.alternate = null; fiber.child = null; fiber.deletions = null; diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index a787d31840af3..babb0c2437215 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -360,3 +360,7 @@ export function prepareScopeUpdate(scopeInstance: Object, inst: Object): void { export function getInstanceFromScope(scopeInstance: Object): null | Object { return nodeToInstanceMap.get(scopeInstance) || null; } + +export function unmountNode(node: any): void { + // noop +} diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 75ab4b6842d90..2a3d0a337bf88 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -112,6 +112,9 @@ export const disableNativeComponentFrames = false; // If there are no still-mounted boundaries, the errors should be rethrown. export const skipUnmountedBoundaries = false; +// When a node is unmounted, recurse into the Fiber subtree and clean out references. +export const enableStrongMemoryCleanup = 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 b6a8016ec8b77..443c489c0063f 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -46,6 +46,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 cb8721af25fe9..bac13f5091b78 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 b504491d98de2..da03038c0211a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 ddab49284e437..eda47cde1139b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 7098bfed2aa5b..9b02f6408229f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 a321f5ea12f92..7bb7ed424c7bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrongMemoryCleanup = 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 bb6a952355657..8834238b02b14 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -45,6 +45,7 @@ export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = true; +export const enableStrongMemoryCleanup = 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 df98012634970..f58e0da1e80b9 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -52,6 +52,7 @@ export const disableNativeComponentFrames = false; export const createRootStrictEffectsByDefault = false; export const enableStrictEffects = false; export const enableUseRefAccessWarning = __VARIANT__; +export const enableStrongMemoryCleanup = __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 893c2699b0d07..25b88a019f1da 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -33,6 +33,7 @@ export const { disableSchedulerTimeoutInWorkLoop, enableSyncMicroTasks, enableLazyContextPropagation, + enableStrongMemoryCleanup, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.