From 9e587423e7312dda09744439f87c3999bc1b0326 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 17 May 2022 20:53:52 +0200 Subject: [PATCH 1/5] Remove commitRootId from FlameGraph --- src/view/components/profiler/flamegraph/FlameNode.tsx | 3 --- .../components/profiler/flamegraph/modes/FlamegraphLayout.tsx | 1 - .../components/profiler/flamegraph/ranked/RankedLayout.tsx | 1 - 3 files changed, 5 deletions(-) diff --git a/src/view/components/profiler/flamegraph/FlameNode.tsx b/src/view/components/profiler/flamegraph/FlameNode.tsx index b30f4fc3..b0530518 100644 --- a/src/view/components/profiler/flamegraph/FlameNode.tsx +++ b/src/view/components/profiler/flamegraph/FlameNode.tsx @@ -8,7 +8,6 @@ export interface Props { selected: boolean; children: any; parentId: number; - commitRootId: number; name: string; onClick: (id: ID) => void; node: NodeTransform; @@ -43,8 +42,6 @@ export function FlameNode(props: Props) { onMouseEnter={onRawMouseEnter} onMouseLeave={onMouseLeave} data-id={node.id} - data-commit-root={props.commitRootId} - data-active-commit-root={node.id === props.commitRootId} data-parent-id={props.parentId} data-visible={visible} data-weight={node.weight} diff --git a/src/view/components/profiler/flamegraph/modes/FlamegraphLayout.tsx b/src/view/components/profiler/flamegraph/modes/FlamegraphLayout.tsx index a433a152..624e690a 100644 --- a/src/view/components/profiler/flamegraph/modes/FlamegraphLayout.tsx +++ b/src/view/components/profiler/flamegraph/modes/FlamegraphLayout.tsx @@ -57,7 +57,6 @@ export function FlamegraphLayout({ key={pos.id} onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave} - commitRootId={commit.commitRootId} node={pos} name={node.name} selected={pos.id === selected.id} diff --git a/src/view/components/profiler/flamegraph/ranked/RankedLayout.tsx b/src/view/components/profiler/flamegraph/ranked/RankedLayout.tsx index 0531dd63..d64ea766 100644 --- a/src/view/components/profiler/flamegraph/ranked/RankedLayout.tsx +++ b/src/view/components/profiler/flamegraph/ranked/RankedLayout.tsx @@ -75,7 +75,6 @@ export function RankedLayout({ return ( Date: Tue, 17 May 2022 20:58:14 +0200 Subject: [PATCH 2/5] Remove commitRootId from RenderReasons --- .../profiler/components/RenderReasons.tsx | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/view/components/profiler/components/RenderReasons.tsx b/src/view/components/profiler/components/RenderReasons.tsx index 200cff18..bbbe6649 100644 --- a/src/view/components/profiler/components/RenderReasons.tsx +++ b/src/view/components/profiler/components/RenderReasons.tsx @@ -42,18 +42,11 @@ export function RenderReasons() { const hasReasons = reason !== null && reason.items && reason.items.length > 0; - let rendered = false; - if (!captureReason && commit) { - const root = commit.nodes.get(commit.commitRootId); - if ( - root && - selected && - selected.startTime >= root.startTime && - selected.endTime <= root.endTime - ) { - rendered = true; - } - } + const rendered = + !captureReason && + commit && + selected && + commit.rendered.includes(selected.id); return ( From f52a5db570c72e001afe1edeb6b92a502cdf950a Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 17 May 2022 20:58:31 +0200 Subject: [PATCH 3/5] Remove commitRootId from RenderedAt --- .../components/RenderedAt/RenderedAt.tsx | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx b/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx index d0a9fb99..0f26ad50 100644 --- a/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx +++ b/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx @@ -9,24 +9,18 @@ export function RenderedAt() { const data = useObserver(() => { const id = store.profiler.selectedNodeId.$; - return store.profiler.commits.$.filter(x => { - if (x.nodes.has(id)) { - const node = x.nodes.get(id)!; - const root = x.nodes.get(x.commitRootId)!; - return node.startTime >= root.startTime && node.endTime <= root.endTime; - } + return store.profiler.commits.$.filter(x => x.rendered.includes(id)).map( + commit => { + const node = commit.nodes.get(id)!; - return false; - }).map(commit => { - const node = commit.nodes.get(id)!; - - const selfDuration = commit.selfDurations.get(id) || 0; - return { - index: store.profiler.commits.$.findIndex(x => x === commit), - startTime: node.startTime, - selfDuration, - }; - }); + const selfDuration = commit.selfDurations.get(id) || 0; + return { + index: store.profiler.commits.$.findIndex(x => x === commit), + startTime: node.startTime, + selfDuration, + }; + }, + ); }); const commitIdx = useObserver(() => store.profiler.activeCommitIdx.$); From a4cd175fd47a14b27f2b3f1b0b754969970ffd21 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 17 May 2022 22:08:52 +0200 Subject: [PATCH 4/5] Remove commitRootId from CommitInfo --- .../profiler/components/CommitInfo/CommitInfo.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/view/components/profiler/components/CommitInfo/CommitInfo.tsx b/src/view/components/profiler/components/CommitInfo/CommitInfo.tsx index 66add015..247d297e 100644 --- a/src/view/components/profiler/components/CommitInfo/CommitInfo.tsx +++ b/src/view/components/profiler/components/CommitInfo/CommitInfo.tsx @@ -9,20 +9,17 @@ export function CommitInfo() { const commit = useObserver(() => store.profiler.activeCommit.$); const isRecording = useObserver(() => store.profiler.isRecording.$); - if (commit === null || isRecording) { + if (commit === null || !commit.rendered.length || isRecording) { return null; } - const root = commit.nodes.get(commit.commitRootId)!; - if (!root) { - return null; - } + const startTime = commit.nodes.get(commit.rendered[0])?.startTime || 0; return (
Start:
-
{formatTime(root.startTime)}
+
{formatTime(startTime)}

Duration:
{formatTime(commit.duration)}
From d5a9c61271968a3d2f4ef23fd52b9d83c1bfb60d Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 17 May 2022 22:12:12 +0200 Subject: [PATCH 5/5] WIP --- src/adapter/protocol/events.ts | 6 +- src/adapter/protocol/operations.test.ts | 1 + src/adapter/protocol/operations.ts | 30 +++- src/adapter/shared/renderer.test.tsx | 14 +- src/adapter/shared/renderer.ts | 10 +- src/adapter/shared/traverse.ts | 133 ++++++++++++------ src/view/components/elements/TreeView.tsx | 8 +- .../components/RenderedAt/DebugNodeNav.tsx | 4 +- src/view/components/profiler/data/commits.ts | 38 +++-- .../profiler/flamegraph/modes/patchTree.ts | 21 ++- .../profiler/flamegraph/placeNodes.ts | 2 +- .../flamegraph/ranked/ranked-utils.ts | 16 +-- .../profiler/flamegraph/testHelpers.test.ts | 2 +- .../profiler/flamegraph/testHelpers.ts | 2 +- src/view/components/tree/windowing.ts | 8 +- src/view/store/index.ts | 4 - 16 files changed, 184 insertions(+), 115 deletions(-) diff --git a/src/adapter/protocol/events.ts b/src/adapter/protocol/events.ts index 3ee3b3ba..d14c4745 100644 --- a/src/adapter/protocol/events.ts +++ b/src/adapter/protocol/events.ts @@ -107,7 +107,7 @@ export function flush(commit: Commit) { * We currently expect all operations to be in order. */ export function applyOperationsV2(store: Store, data: number[]) { - const { rootId: commitRootId, roots, tree, reasons, stats } = ops2Tree( + const { rootId, roots, tree, reasons, stats, rendered } = ops2Tree( store.nodes.$, store.roots.$, data, @@ -127,9 +127,9 @@ export function applyOperationsV2(store: Store, data: number[]) { // If we are profiling, we'll make a frozen copy of the mutable // elements tree because the profiler can step through time if (store.profiler.isRecording.$) { - recordProfilerCommit(store.nodes.$, store.profiler, commitRootId); + recordProfilerCommit(store.nodes.$, store.profiler, rendered, rootId); store.profiler.renderReasons.update(m => { - m.set(commitRootId, reasons); + m.set(rootId, reasons); }); } diff --git a/src/adapter/protocol/operations.test.ts b/src/adapter/protocol/operations.test.ts index 682e12f5..5dcea191 100644 --- a/src/adapter/protocol/operations.test.ts +++ b/src/adapter/protocol/operations.test.ts @@ -21,6 +21,7 @@ describe("ops2Tree", () => { rootId: 1, roots: [1], removals: [], + rendered: [], tree: new Map(), reasons: new Map(), stats: null, diff --git a/src/adapter/protocol/operations.ts b/src/adapter/protocol/operations.ts index b082bf8d..1bf58271 100644 --- a/src/adapter/protocol/operations.ts +++ b/src/adapter/protocol/operations.ts @@ -1,4 +1,4 @@ -import { ID, Tree } from "../../view/store/types"; +import { DevNodeType, ID, Tree } from "../../view/store/types"; import { parseTable } from "./string-table"; import { MsgTypes } from "./events"; import { deepClone } from "../../view/components/profiler/flamegraph/modes/adjustNodesToRight"; @@ -19,16 +19,32 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { const removals: ID[] = []; const reasons: RenderReasonMap = new Map(); let stats: ParsedStats | null = null; + const rendered: ID[] = []; let i = ops[1] + 1; const strings = parseTable(ops.slice(1, i + 1)); for (i += 1; i < ops.length; i++) { switch (ops[i]) { - case MsgTypes.ADD_ROOT: - roots.push(ops[i + 1]); + case MsgTypes.ADD_ROOT: { + const id = ops[i + 1]; + roots.push(id); i += 1; + + pending.set(id, { + children: [], + depth: -1, + id, + hocs: null, + name: "__VIRTUAL__ROOT__", + parent: -1, + type: DevNodeType.Group, + key: "", + startTime: -1, + endTime: -1, + }); break; + } case MsgTypes.ADD_VNODE: { const id = ops[i + 1]; const parentId = ops[i + 3]; @@ -39,6 +55,8 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { clone.children.push(id); } + rendered.push(id); + pending.set(id, { children: [], depth: parent ? parent.depth + 1 : 0, @@ -61,6 +79,8 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { x.startTime = ops[i + 2] / 1000; x.endTime = ops[i + 3] / 1000; + rendered.push(id); + i += 3; break; } @@ -85,7 +105,7 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { } // Check if node was a root - const rootIdx = roots.indexOf(node.id); + const rootIdx = roots.indexOf(node.parent); if (rootIdx > -1) { roots.splice(rootIdx, 1); } @@ -159,5 +179,5 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { } } - return { rootId, roots, tree: pending, removals, reasons, stats }; + return { rootId, roots, tree: pending, removals, reasons, stats, rendered }; } diff --git a/src/adapter/shared/renderer.test.tsx b/src/adapter/shared/renderer.test.tsx index b2924058..7f4338bc 100644 --- a/src/adapter/shared/renderer.test.tsx +++ b/src/adapter/shared/renderer.test.tsx @@ -77,7 +77,6 @@ describe("Renderer 10", () => { expect(toSnapshot(spy.args[1][1])).to.deep.equal([ "rootId: 1", "Update timings 1", - "Update timings 2", ]); }); @@ -111,7 +110,6 @@ describe("Renderer 10", () => { "rootId: 1", "Add 3 to parent 2", "Update timings 1", - "Update timings 2", ]); }); @@ -154,7 +152,6 @@ describe("Renderer 10", () => { expect(toSnapshot(spy.args[1][1])).to.deep.equal([ "rootId: 1", "Update timings 1", - "Update timings 2", ]); }); @@ -177,9 +174,6 @@ describe("Renderer 10", () => { expect(toSnapshot(spy.args[1][1])).to.deep.equal([ "rootId: 1", "Update timings 1", - "Update timings 2", - "Update timings 4", - "Update timings 3", "Reorder 2 [4, 3]", ]); }); @@ -341,9 +335,8 @@ describe("Renderer 10", () => { }); expect(toSnapshot(spy.args[1][1])).to.deep.equal([ - "rootId: 2", + "rootId: 1", "Update timings 2", - "Update timings 3", ]); }); @@ -383,13 +376,12 @@ describe("Renderer 10", () => { expect(toSnapshot(spy.args[2][1])).to.deep.equal([ "rootId: 3", + "Add 3 to parent -1", "Add 4
to parent 3", "Add 5 to parent 4", "Add 6
to parent 5", "Add 7 to parent 4", "Add 8 to parent 4", - "Remove 3", // TODO: Seems wrong - "Remove 3", // TODO: Seems wrong ]); }); @@ -434,6 +426,7 @@ describe("Renderer 10", () => { ]); expect(toSnapshot(spy.args[2][1])).to.deep.equal([ "rootId: 4", + "Add 4 to parent -1", "Add 5
to parent 4", "Add 6 to parent 5", "Add 7
to parent 6", @@ -442,7 +435,6 @@ describe("Renderer 10", () => { "Add 10
to parent 9", "Add 11 to parent 5", "Add 12 to parent 5", - "Update timings 2", ]); renderer.applyFilters({ diff --git a/src/adapter/shared/renderer.ts b/src/adapter/shared/renderer.ts index 2fd838cf..1e6cf8bc 100644 --- a/src/adapter/shared/renderer.ts +++ b/src/adapter/shared/renderer.ts @@ -21,6 +21,7 @@ import { PreactBindings, SharedVNode } from "../shared/bindings"; import { inspectVNode } from "./inspectVNode"; import { logVNode } from "../10/log"; import { VNodeTimings } from "./timings"; +import { printCommit } from "../debug"; export interface RendererConfig { Fragment: FunctionalComponent; @@ -69,7 +70,7 @@ export function createRenderer( bindings: PreactBindings, timings: VNodeTimings, ): Renderer { - const roots = new Set(); + const roots = new Map(); let currentUnmounts: number[] = []; @@ -96,9 +97,10 @@ export function createRenderer( return { clear() { - roots.forEach(vnode => { + roots.forEach((id, vnode) => { onUnmount(vnode); }); + roots.clear(); }, getVNodeById: id => getVNodeById(ids, id), @@ -179,8 +181,7 @@ export function createRenderer( /** Queue events and flush in one go */ const queue: BaseEvent[] = []; - roots.forEach(root => { - const rootId = getVNodeId(ids, root); + roots.forEach((rootId, root) => { traverse(root, vnode => this.onUnmount(vnode), bindings); const commit: Commit = { @@ -258,6 +259,7 @@ export function createRenderer( profiler.pendingHighlightUpdates.clear(); } + printCommit(ev.data); port.send(ev.type as any, ev.data); }, onUnmount, diff --git a/src/adapter/shared/traverse.ts b/src/adapter/shared/traverse.ts index 6981482c..c6e374c1 100644 --- a/src/adapter/shared/traverse.ts +++ b/src/adapter/shared/traverse.ts @@ -129,6 +129,8 @@ export function shouldFilter( return false; } else if (bindings.isElement(vnode) && filters.type.has("dom")) { return true; + } else if (bindings.isRoot(vnode, config) && filters.type.has("root")) { + return true; } else if (filters.type.has("hoc")) { const name = bindings.getDisplayName(vnode, config); @@ -180,7 +182,41 @@ function mount( commit.stats.mounts++; } + // FIXME: Move out of mount() into createCommit() const root = bindings.isRoot(vnode, config); + if (root) { + const virtualRootId = ids.nextId++; + commit.operations.push(MsgTypes.ADD_ROOT, virtualRootId); + commit.rootId = virtualRootId; + + let vnodeToMount = vnode; + if (filters.type.has("root")) { + const children = bindings.getActualChildren(vnode); + if (!children.length || children[0] == null) { + return; + } + + vnodeToMount = children[0]; + } + + mount( + ids, + commit, + vnodeToMount, + virtualRootId, + filters, + domCache, + config, + profiler, + hocs, + bindings, + timings, + timingsByVNode, + renderReasonPre, + ); + + return; + } const skip = shouldFilter(vnode, filters, config, bindings); let name = bindings.getDisplayName(vnode, config); @@ -196,11 +232,8 @@ function mount( } } - if (root || !skip) { + if (!skip) { const id = getOrCreateVNodeId(ids, vnode); - if (bindings.isRoot(vnode, config)) { - commit.operations.push(MsgTypes.ADD_ROOT, id); - } if (!timings.start.has(id)) { timings.start.set(id, timingsByVNode.start.get(vnode) || 0); @@ -389,42 +422,51 @@ function update( } const id = getVNodeId(ids, vnode); - commit.operations.push( - MsgTypes.UPDATE_VNODE_TIMINGS, - id, - (timings.start.get(id) || 0) * 1000, - (timings.end.get(id) || 0) * 1000, - ); - - const name = bindings.getDisplayName(vnode, config); - const hoc = getHocName(name); - if (hoc) { - hocs = [...hocs, hoc]; - } else { - addHocs(commit, id, hocs); - hocs = []; - } - const oldVNode = getVNodeById(ids, id); updateVNodeId(ids, id, vnode); - if (profiler.isProfiling && profiler.captureRenderReasons) { - const reason = - renderReasonPre !== null - ? renderReasonPre.get(vnode) || null - : bindings.getRenderReasonPost(ids, bindings, timings, oldVNode, vnode); - if (reason !== null) { - const count = reason.items ? reason.items.length : 0; - commit.operations.push(MsgTypes.RENDER_REASON, id, reason.type, count); - if (reason.items && count > 0) { - commit.operations.push( - ...reason.items.map(str => getStringId(commit.strings, str)), - ); + const didRender = timingsByVNode.end.has(vnode); + if (didRender) { + commit.operations.push( + MsgTypes.UPDATE_VNODE_TIMINGS, + id, + (timings.start.get(id) || 0) * 1000, + (timings.end.get(id) || 0) * 1000, + ); + + const name = bindings.getDisplayName(vnode, config); + const hoc = getHocName(name); + if (hoc) { + hocs = [...hocs, hoc]; + } else { + addHocs(commit, id, hocs); + hocs = []; + } + + if (profiler.isProfiling && profiler.captureRenderReasons) { + const reason = + renderReasonPre !== null + ? renderReasonPre.get(vnode) || null + : bindings.getRenderReasonPost( + ids, + bindings, + timings, + oldVNode, + vnode, + ); + if (reason !== null) { + const count = reason.items ? reason.items.length : 0; + commit.operations.push(MsgTypes.RENDER_REASON, id, reason.type, count); + if (reason.items && count > 0) { + commit.operations.push( + ...reason.items.map(str => getStringId(commit.strings, str)), + ); + } } } - } - updateHighlight(profiler, vnode, bindings); + updateHighlight(profiler, vnode, bindings); + } const oldChildren = oldVNode ? bindings @@ -528,7 +570,7 @@ function findClosestNonFilteredParent( export function createCommit( ids: IdMappingState, - roots: Set, + roots: Map, vnode: T, filters: FilterState, domCache: WeakMap, @@ -539,7 +581,7 @@ export function createCommit( timingsByVNode: VNodeTimings, renderReasonPre: Map | null, ): Commit { - const commit = { + const commit: Commit & { renderReasons: Map } = { operations: [], rootId: -1, strings: new Map(), @@ -559,8 +601,12 @@ export function createCommit( commit.stats.roots.children.push(children.length); } + const virtualRootId = roots.get(vnode) || ids.nextId++; + commit.operations.push(MsgTypes.ADD_ROOT, virtualRootId); + commit.rootId = virtualRootId; + parentId = -1; - roots.add(vnode); + roots.set(vnode, virtualRootId); } else { parentId = findClosestNonFilteredParent(ids, helpers, vnode); } @@ -599,11 +645,16 @@ export function createCommit( ); } - let rootId = getVNodeId(ids, vnode); - if (rootId === -1) { - rootId = findClosestNonFilteredParent(ids, helpers, vnode); + // Find actual root node + if (commit.rootId === -1) { + let rootVNode: T | null = vnode; + while ((rootVNode = helpers.getVNodeParent(rootVNode)) != null) { + if (helpers.isRoot(rootVNode, config)) { + commit.rootId = roots.get(rootVNode)!; + break; + } + } } - commit.rootId = rootId; return commit; } diff --git a/src/view/components/elements/TreeView.tsx b/src/view/components/elements/TreeView.tsx index df0d477b..f314bb57 100644 --- a/src/view/components/elements/TreeView.tsx +++ b/src/view/components/elements/TreeView.tsx @@ -190,7 +190,6 @@ export function TreeItem(props: { key: any; id: ID; top: number }) { const as = useSelection(); const { collapsed, toggle } = useCollapser(); const node = useObserver(() => store.nodes.$.get(id) || null); - const filterRoot = useObserver(() => store.filter.filterRoot.$); const filterHoc = useObserver(() => store.filter.filterHoc.$); const roots = useObserver(() => store.roots.$); const onToggle = () => toggle(id); @@ -198,7 +197,8 @@ export function TreeItem(props: { key: any; id: ID; top: number }) { if (!node) return null; - const isRoot = node.parent === -1 && roots.includes(node.id); + // FIXME: This seems wrong + const isRoot = roots.includes(node.id); return (
{node.children.length > 0 && (