Skip to content

Commit

Permalink
[DevTools] Build Updater List from the Commit instead of Map (#30897)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sebmarkbage
sebmarkbage authored Sep 7, 2024
1 parent 6292398 commit 99cba2b
Showing 1 changed file with 50 additions and 39 deletions.
89 changes: 50 additions & 39 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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, ...]
Expand Down Expand Up @@ -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));
}
}
}

Expand Down Expand Up @@ -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)) {
Expand All @@ -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<SerializedElement> | 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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -3699,28 +3710,28 @@ 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.
updateFiberRecursively(rootInstance, current, prevFiber, false);
} 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);
}

if (isProfiling && isProfilingSupported) {
if (!shouldBailoutWithPendingOperations()) {
const commitProfilingMetadata =
((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
currentRootID,
currentRoot.id,
);

if (commitProfilingMetadata != null) {
Expand All @@ -3729,7 +3740,7 @@ export function attach(
);
} else {
((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).set(
currentRootID,
currentRoot.id,
[((currentCommitProfilingMetadata: any): CommitProfilingData)],
);
}
Expand All @@ -3743,7 +3754,7 @@ export function attach(
hook.emit('traceUpdates', traceUpdatesForNodes);
}

currentRootID = -1;
currentRoot = (null: any);
}

function getResourceInstance(fiber: Fiber): HostInstance | null {
Expand Down

0 comments on commit 99cba2b

Please sign in to comment.