From 99cba2b041cd13d7ade48a5c97b473e8a188df35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 6 Sep 2024 21:59:09 -0400 Subject: [PATCH] [DevTools] Build Updater List from the Commit instead of Map (#30897) Stacked on #30896. The problem with the `getUpdatersList` function is that it iterates over Fibers and then looks up each of those Fibers in the fiberToFiberInstanceMap which we ideally could get rid of. However, every time an updater comes into play for a commit it must mean that something below the updater itself updated and so the updater will also be cloned which means we'll pass it on the way down when traversing the tree in the commit. When we do this traversal, we can just look if the Fiber is in the updater set and if so add it to the updater list as we go. --- .../src/backend/fiber/renderer.js | 89 +++++++++++-------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 04cd3fa82a..35bc0ab0f9 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -1293,11 +1293,11 @@ export function attach( 'Expected the root instance to already exist when applying filters', ); } - currentRootID = rootInstance.id; + currentRoot = rootInstance; unmountInstanceRecursively(rootInstance); rootToFiberInstanceMap.delete(root); flushPendingEvents(root); - currentRootID = -1; + currentRoot = (null: any); }); applyComponentFilters(componentFilters); @@ -1323,11 +1323,11 @@ export function attach( mightBeOnTrackedPath = true; } - currentRootID = newRoot.id; - setRootPseudoKey(currentRootID, root.current); + currentRoot = newRoot; + setRootPseudoKey(currentRoot.id, root.current); mountFiberRecursively(root.current, false); flushPendingEvents(root); - currentRootID = -1; + currentRoot = (null: any); }); // Also re-evaluate all error and warning counts given the new filters. @@ -1528,7 +1528,7 @@ export function attach( } // When a mount or update is in progress, this value tracks the root that is being operated on. - let currentRootID: number = -1; + let currentRoot: FiberInstance = (null: any); // Returns a FiberInstance if one has already been generated for the Fiber or null if one has not been generated. // Use this method while e.g. logging to avoid over-retaining Fibers. @@ -1885,7 +1885,12 @@ export function attach( 3 + pendingOperations.length, ); operations[0] = rendererID; - operations[1] = currentRootID; + if (currentRoot === null) { + // TODO: This is not always safe so this field is probably not needed. + operations[1] = -1; + } else { + operations[1] = currentRoot.id; + } operations[2] = 0; // String table size for (let j = 0; j < pendingOperations.length; j++) { operations[3 + j] = pendingOperations[j]; @@ -2038,7 +2043,12 @@ export function attach( // Which in turn enables fiber props, states, and hooks to be inspected. let i = 0; operations[i++] = rendererID; - operations[i++] = currentRootID; + if (currentRoot === null) { + // TODO: This is not always safe so this field is probably not needed. + operations[i++] = -1; + } else { + operations[i++] = currentRoot.id; + } // Now fill in the string table. // [stringTableLength, str1Length, ...str1, str2Length, ...str2, ...] @@ -2881,6 +2891,25 @@ export function attach( } } } + + // If this Fiber was in the set of memoizedUpdaters we need to record + // it to be included in the description of the commit. + const fiberRoot: FiberRoot = currentRoot.data.stateNode; + const updaters = fiberRoot.memoizedUpdaters; + if ( + updaters != null && + (updaters.has(fiber) || + // We check the alternate here because we're matching identity and + // prevFiber might be same as fiber. + (fiber.alternate !== null && updaters.has(fiber.alternate))) + ) { + const metadata = + ((currentCommitProfilingMetadata: any): CommitProfilingData); + if (metadata.updaters === null) { + metadata.updaters = []; + } + metadata.updaters.push(instanceToSerializedElement(fiberInstance)); + } } } @@ -3568,8 +3597,8 @@ export function attach( if (alternate) { fiberToFiberInstanceMap.set(alternate, newRoot); } - currentRootID = newRoot.id; - setRootPseudoKey(currentRootID, root.current); + currentRoot = newRoot; + setRootPseudoKey(currentRoot.id, root.current); // Handle multi-renderer edge-case where only some v16 renderers support profiling. if (isProfiling && rootSupportsProfiling(root)) { @@ -3581,35 +3610,20 @@ export function attach( commitTime: getCurrentTime() - profilingStartTime, maxActualDuration: 0, priorityLevel: null, - updaters: getUpdatersList(root), + updaters: null, effectDuration: null, passiveEffectDuration: null, }; } mountFiberRecursively(root.current, false); + flushPendingEvents(root); - currentRootID = -1; + currentRoot = (null: any); }); } } - function getUpdatersList(root: any): Array | null { - const updaters = root.memoizedUpdaters; - if (updaters == null) { - return null; - } - const result = []; - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const updater of updaters) { - const inst = getFiberInstanceUnsafe(updater); - if (inst !== null) { - result.push(instanceToSerializedElement(inst)); - } - } - return result; - } - function handleCommitFiberUnmount(fiber: any) { // This Hook is no longer used. After having shipped DevTools everywhere it is // safe to stop calling it from Fiber. @@ -3646,11 +3660,10 @@ export function attach( if (alternate) { fiberToFiberInstanceMap.set(alternate, rootInstance); } - currentRootID = rootInstance.id; } else { - currentRootID = rootInstance.id; prevFiber = rootInstance.data; } + currentRoot = rootInstance; // Before the traversals, remember to start tracking // our path in case we have selection to restore. @@ -3675,9 +3688,7 @@ export function attach( maxActualDuration: 0, priorityLevel: priorityLevel == null ? null : formatPriorityLevel(priorityLevel), - - updaters: getUpdatersList(root), - + updaters: null, // Initialize to null; if new enough React version is running, // these values will be read during separate handlePostCommitFiberRoot() call. effectDuration: null, @@ -3699,7 +3710,7 @@ export function attach( current.memoizedState.isDehydrated !== true; if (!wasMounted && isMounted) { // Mount a new root. - setRootPseudoKey(currentRootID, current); + setRootPseudoKey(currentRoot.id, current); mountFiberRecursively(current, false); } else if (wasMounted && isMounted) { // Update an existing root. @@ -3707,12 +3718,12 @@ export function attach( } else if (wasMounted && !isMounted) { // Unmount an existing root. unmountInstanceRecursively(rootInstance); - removeRootPseudoKey(currentRootID); + removeRootPseudoKey(currentRoot.id); rootToFiberInstanceMap.delete(root); } } else { // Mount a new root. - setRootPseudoKey(currentRootID, current); + setRootPseudoKey(currentRoot.id, current); mountFiberRecursively(current, false); } @@ -3720,7 +3731,7 @@ export function attach( if (!shouldBailoutWithPendingOperations()) { const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get( - currentRootID, + currentRoot.id, ); if (commitProfilingMetadata != null) { @@ -3729,7 +3740,7 @@ export function attach( ); } else { ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).set( - currentRootID, + currentRoot.id, [((currentCommitProfilingMetadata: any): CommitProfilingData)], ); } @@ -3743,7 +3754,7 @@ export function attach( hook.emit('traceUpdates', traceUpdatesForNodes); } - currentRootID = -1; + currentRoot = (null: any); } function getResourceInstance(fiber: Fiber): HostInstance | null {