From db4d63b88a37c4e0ddddb6c309f19fa1fe0888fd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 26 Aug 2024 16:33:34 -0400 Subject: [PATCH 1/3] Track the treeBaseDuration of Virtual Instances These don't have their own time since they don't take up any time to render but they show up in the tree for context. However they never render themselves. Their base tree time is the base time of their children. --- .../src/backend/fiber/renderer.js | 52 ++++++++++++++++--- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 57eafa176f550..88063de357227 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2194,7 +2194,7 @@ export function attach( if (isProfilingSupported) { idToRootMap.set(id, currentRootID); - recordProfilingDurations(fiber); + recordProfilingDurations(fiberInstance); } return fiberInstance; } @@ -2248,8 +2248,6 @@ export function attach( if (isProfilingSupported) { idToRootMap.set(id, currentRootID); - // TODO: Include tree base duration of children somehow. - // recordProfilingDurations(...); } } @@ -2401,6 +2399,8 @@ export function attach( traceNearestHostComponentUpdate, virtualLevel + 1, ); + // Must be called after all children have been appended. + recordVirtualProfilingDurations(virtualInstance); } finally { reconcilingParent = stashedParent; previouslyReconciledSibling = stashedPrevious; @@ -2655,8 +2655,9 @@ export function attach( removeChild(instance); } - function recordProfilingDurations(fiber: Fiber) { - const id = getFiberIDThrows(fiber); + function recordProfilingDurations(fiberInstance: FiberInstance) { + const id = fiberInstance.id; + const fiber = fiberInstance.data; const {actualDuration, treeBaseDuration} = fiber; idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); @@ -2722,6 +2723,41 @@ export function attach( } } + function recordVirtualProfilingDurations(virtualInstance: VirtualInstance) { + const id = virtualInstance.id; + + let treeBaseDuration = 0; + // Add up the base duration of the child instances. The virtual base duration + // will be the same as children's duration since we don't take up any render + // time in the virtual instance. + for ( + let child = virtualInstance.firstChild; + child !== null; + child = child.nextSibling + ) { + const childDuration = idToTreeBaseDurationMap.get(child.id); + if (childDuration !== undefined) { + treeBaseDuration += childDuration; + } + } + + if (isProfiling) { + const previousTreeBaseDuration = idToTreeBaseDurationMap.get(id); + if (treeBaseDuration !== previousTreeBaseDuration) { + // Tree base duration updates are included in the operations typed array. + // So we have to convert them from milliseconds to microseconds so we can send them as ints. + const convertedTreeBaseDuration = Math.floor( + (treeBaseDuration || 0) * 1000, + ); + pushOperation(TREE_OPERATION_UPDATE_TREE_BASE_DURATION); + pushOperation(id); + pushOperation(convertedTreeBaseDuration); + } + } + + idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); + } + function recordResetChildren(parentInstance: DevToolsInstance) { if (__DEBUG__) { if ( @@ -2789,6 +2825,8 @@ export function attach( ) { recordResetChildren(virtualInstance); } + // Must be called after all children have been appended. + recordVirtualProfilingDurations(virtualInstance); } finally { unmountRemainingChildren(); reconcilingParent = stashedParent; @@ -3236,11 +3274,11 @@ export function attach( } } - if (shouldIncludeInTree) { + if (fiberInstance !== null) { const isProfilingSupported = nextFiber.hasOwnProperty('treeBaseDuration'); if (isProfilingSupported) { - recordProfilingDurations(nextFiber); + recordProfilingDurations(fiberInstance); } } if (shouldResetChildren) { From b65c514e51fd2ff311e8c65b9bfcd3e9150d5554 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 26 Aug 2024 16:35:35 -0400 Subject: [PATCH 2/3] Mention that these didn't client render to clarify We don't track the difference between a virtual instance and a Fiber that didn't render this update. This might be a bit confusing as to why it didn't render. I add the word "client" to make it a bit clearer. But really we should probably have different verbiage here based on it is a Server Component or something else. --- .../src/devtools/views/Profiler/HoveredFiberInfo.js | 2 +- .../src/devtools/views/Profiler/SidebarSelectedFiberInfo.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js index 51f82038d3489..f1156a05aee14 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js @@ -93,7 +93,7 @@ export default function HoveredFiberInfo({fiberData}: Props): React.Node { )}
- {renderDurationInfo ||
Did not render.
} + {renderDurationInfo ||
Did not client render.
}
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js index 2eca7b0fdad3d..d785cfebdacde 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js @@ -142,7 +142,7 @@ export default function SidebarSelectedFiberInfo(): React.Node { )} {listItems.length === 0 && ( -
Did not render during this profiling session.
+
Did not render on the client during this profiling session.
)} From 3e2119c1373a16127a216c69d4c5c5fad689653a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 26 Aug 2024 16:59:20 -0400 Subject: [PATCH 3/3] Remove idToTreeBaseDurationMap and idToRootMap maps Cloning the Map isn't really all that super fast anyway and it means we have to maintain the map continuously as we render. Instead, we can track it on the instances and then walk the instances to create a snapshot when starting to profile. This isn't as fast but really fast too and requires less bookkeeping while rendering. --- .../src/backend/fiber/renderer.js | 91 ++++++++----------- 1 file changed, 36 insertions(+), 55 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 88063de357227..ec59a626ae6b2 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -155,6 +155,7 @@ type FiberInstance = { componentStack: null | string, errors: null | Map, // error messages and count warnings: null | Map, // warning messages and count + treeBaseDuration: number, // the profiled time of the last render of this subtree data: Fiber, // one of a Fiber pair }; @@ -170,6 +171,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance { componentStack: null, errors: null, warnings: null, + treeBaseDuration: 0, data: fiber, }; } @@ -193,6 +195,7 @@ type VirtualInstance = { // that old errors/warnings don't disappear when the instance is refreshed. errors: null | Map, // error messages and count warnings: null | Map, // warning messages and count + treeBaseDuration: number, // the profiled time of the last render of this subtree // The latest info for this instance. This can be updated over time and the // same info can appear in more than once ServerComponentInstance. data: ReactComponentInfo, @@ -212,6 +215,7 @@ function createVirtualInstance( componentStack: null, errors: null, warnings: null, + treeBaseDuration: 0, data: debugEntry, }; } @@ -1346,16 +1350,6 @@ export function attach( } } - // When profiling is supported, we store the latest tree base durations for each Fiber. - // This is so that we can quickly capture a snapshot of those values if profiling starts. - // If we didn't store these values, we'd have to crawl the tree when profiling started, - // and use a slow path to find each of the current Fibers. - const idToTreeBaseDurationMap: Map = new Map(); - - // When profiling is supported, we store the latest tree base durations for each Fiber. - // This map enables us to filter these times by root when sending them to the frontend. - const idToRootMap: Map = new Map(); - // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; @@ -2192,8 +2186,6 @@ export function attach( } if (isProfilingSupported) { - idToRootMap.set(id, currentRootID); - recordProfilingDurations(fiberInstance); } return fiberInstance; @@ -2207,8 +2199,6 @@ export function attach( idToDevToolsInstanceMap.set(id, instance); - const isProfilingSupported = false; // TODO: Support Tree Base Duration Based on Children. - const componentInfo = instance.data; const key = @@ -2245,10 +2235,6 @@ export function attach( pushOperation(ownerID); pushOperation(displayNameStringID); pushOperation(keyStringID); - - if (isProfilingSupported) { - idToRootMap.set(id, currentRootID); - } } function recordUnmount(fiberInstance: FiberInstance): void { @@ -2283,12 +2269,6 @@ export function attach( } untrackFiber(fiberInstance); - - const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); - if (isProfilingSupported) { - idToRootMap.delete(id); - idToTreeBaseDurationMap.delete(id); - } } // Running state of the remaining children from the previous version of this parent that @@ -2416,12 +2396,6 @@ export function attach( const id = instance.id; pendingRealUnmountedIDs.push(id); - - const isProfilingSupported = false; // TODO: Profiling support. - if (isProfilingSupported) { - idToRootMap.delete(id); - idToTreeBaseDurationMap.delete(id); - } } function mountVirtualChildrenRecursively( @@ -2660,7 +2634,7 @@ export function attach( const fiber = fiberInstance.data; const {actualDuration, treeBaseDuration} = fiber; - idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); + fiberInstance.treeBaseDuration = treeBaseDuration || 0; if (isProfiling) { const {alternate} = fiber; @@ -2735,14 +2709,11 @@ export function attach( child !== null; child = child.nextSibling ) { - const childDuration = idToTreeBaseDurationMap.get(child.id); - if (childDuration !== undefined) { - treeBaseDuration += childDuration; - } + treeBaseDuration += child.treeBaseDuration; } if (isProfiling) { - const previousTreeBaseDuration = idToTreeBaseDurationMap.get(id); + const previousTreeBaseDuration = virtualInstance.treeBaseDuration; if (treeBaseDuration !== previousTreeBaseDuration) { // Tree base duration updates are included in the operations typed array. // So we have to convert them from milliseconds to microseconds so we can send them as ints. @@ -2755,7 +2726,7 @@ export function attach( } } - idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); + virtualInstance.treeBaseDuration = treeBaseDuration; } function recordResetChildren(parentInstance: DevToolsInstance) { @@ -5132,8 +5103,8 @@ export function attach( let currentCommitProfilingMetadata: CommitProfilingData | null = null; let displayNamesByRootID: DisplayNamesByRootID | null = null; let idToContextsMap: Map | null = null; - let initialTreeBaseDurationsMap: Map | null = null; - let initialIDToRootMap: Map | null = null; + let initialTreeBaseDurationsMap: Map> | null = + null; let isProfiling: boolean = false; let profilingStartTime: number = 0; let recordChangeDescriptions: boolean = false; @@ -5152,24 +5123,15 @@ export function attach( rootToCommitProfilingMetadataMap.forEach( (commitProfilingMetadata, rootID) => { const commitData: Array = []; - const initialTreeBaseDurations: Array<[number, number]> = []; const displayName = (displayNamesByRootID !== null && displayNamesByRootID.get(rootID)) || 'Unknown'; - if (initialTreeBaseDurationsMap != null) { - initialTreeBaseDurationsMap.forEach((treeBaseDuration, id) => { - if ( - initialIDToRootMap != null && - initialIDToRootMap.get(id) === rootID - ) { - // We don't need to convert milliseconds to microseconds in this case, - // because the profiling summary is JSON serialized. - initialTreeBaseDurations.push([id, treeBaseDuration]); - } - }); - } + const initialTreeBaseDurations: Array<[number, number]> = + (initialTreeBaseDurationsMap !== null && + initialTreeBaseDurationsMap.get(rootID)) || + []; commitProfilingMetadata.forEach((commitProfilingData, commitIndex) => { const { @@ -5256,6 +5218,22 @@ export function attach( }; } + function snapshotTreeBaseDurations( + instance: DevToolsInstance, + target: Array<[number, number]>, + ) { + // We don't need to convert milliseconds to microseconds in this case, + // because the profiling summary is JSON serialized. + target.push([instance.id, instance.treeBaseDuration]); + for ( + let child = instance.firstChild; + child !== null; + child = child.nextSibling + ) { + snapshotTreeBaseDurations(child, target); + } + } + function startProfiling(shouldRecordChangeDescriptions: boolean) { if (isProfiling) { return; @@ -5268,16 +5246,19 @@ export function attach( // since either of these may change during the profiling session // (e.g. when a fiber is re-rendered or when a fiber gets removed). displayNamesByRootID = new Map(); - initialTreeBaseDurationsMap = new Map(idToTreeBaseDurationMap); - initialIDToRootMap = new Map(idToRootMap); + initialTreeBaseDurationsMap = new Map(); idToContextsMap = new Map(); hook.getFiberRoots(rendererID).forEach(root => { - const rootID = getFiberIDThrows(root.current); + const rootInstance = getFiberInstanceThrows(root.current); + const rootID = rootInstance.id; ((displayNamesByRootID: any): DisplayNamesByRootID).set( rootID, getDisplayNameForRoot(root.current), ); + const initialTreeBaseDurations: Array<[number, number]> = []; + snapshotTreeBaseDurations(rootInstance, initialTreeBaseDurations); + (initialTreeBaseDurationsMap: any).set(rootID, initialTreeBaseDurations); if (shouldRecordChangeDescriptions) { // Record all contexts at the time profiling is started.