From 27d3f65af7abb03bd801c6f660f68d27e08e53f0 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 17 May 2022 21:24:24 +0200 Subject: [PATCH] WIP --- src/adapter/protocol/events.test.ts | 4 +- src/adapter/protocol/events.ts | 6 +- src/adapter/protocol/legacy/operationsV1.ts | 2 +- src/adapter/protocol/operations.test.ts | 27 ++-- src/adapter/protocol/operations.ts | 38 +++-- 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 | 11 +- .../components/CommitInfo/CommitInfo.tsx | 33 +++-- .../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 | 22 ++- src/view/store/types.ts | 6 +- 20 files changed, 236 insertions(+), 163 deletions(-) diff --git a/src/adapter/protocol/events.test.ts b/src/adapter/protocol/events.test.ts index b2694c4b..11ba4c40 100644 --- a/src/adapter/protocol/events.test.ts +++ b/src/adapter/protocol/events.test.ts @@ -9,7 +9,7 @@ describe("applyEvent", () => { const store = createStore(); const data = fromSnapshot(["rootId: 1"]); applyEvent(store, "operation_v2", data); - expect(store.roots.$.length).to.equal(1); + expect(store.roots.$.size).to.equal(1); }); it("should update roots correctly", () => { @@ -20,7 +20,7 @@ describe("applyEvent", () => { "Add 2
to parent 1", ]); applyEvent(store, "operation_v2", data); - expect(store.roots.$.length).to.equal(1); + expect(store.roots.$.size).to.equal(1); }); it("should mount nodes", () => { 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/legacy/operationsV1.ts b/src/adapter/protocol/legacy/operationsV1.ts index 969697f8..f9096e29 100644 --- a/src/adapter/protocol/legacy/operationsV1.ts +++ b/src/adapter/protocol/legacy/operationsV1.ts @@ -15,7 +15,7 @@ export function applyOperationsV1(store: Store, data: number[]) { switch (data[i]) { case MsgTypes.ADD_ROOT: { const id = data[i + 1]; - store.roots.$.push(id); + store.roots.$.add(id); i += 1; break; } diff --git a/src/adapter/protocol/operations.test.ts b/src/adapter/protocol/operations.test.ts index 682e12f5..aaff47d2 100644 --- a/src/adapter/protocol/operations.test.ts +++ b/src/adapter/protocol/operations.test.ts @@ -9,7 +9,7 @@ describe("ops2Tree", () => { Fragment *** `; - const res = ops2Tree(state.idMap, [], []); + const res = ops2Tree(state.idMap, new Set(), []); expect(res.tree).not.to.equal(state.idMap); expect(res.tree.size).to.equal(state.idMap.size); }); @@ -17,10 +17,11 @@ describe("ops2Tree", () => { describe("ADD_ROOT", () => { it("should add new roots", () => { const ops = fromSnapshot(["rootId: 1"]); - expect(ops2Tree(new Map(), [], ops)).to.deep.equal({ + expect(ops2Tree(new Map(), new Set(), ops)).to.deep.equal({ rootId: 1, - roots: [1], + roots: new Map([[1, 1]]), removals: [], + rendered: [], tree: new Map(), reasons: new Map(), stats: null, @@ -32,7 +33,7 @@ describe("ops2Tree", () => { it("should add a vnode", () => { const ops = fromSnapshot(["rootId: 1", "Add 1 to parent -1"]); expect( - Array.from(ops2Tree(new Map(), [], ops).tree.values()), + Array.from(ops2Tree(new Map(), new Set(), ops).tree.values()), ).to.deep.equal([ { children: [], @@ -56,7 +57,7 @@ describe("ops2Tree", () => { "Add 2 to parent 1", ]); - const { tree } = ops2Tree(new Map(), [], ops); + const { tree } = ops2Tree(new Map(), new Set(), ops); expect(Array.from(tree.values())).to.deep.equal([ { children: [2], @@ -93,7 +94,7 @@ describe("ops2Tree", () => { `; const ops = fromSnapshot(["rootId: 1", "Update timings 1 time 20:40"]); - const next = ops2Tree(state.idMap, [], ops).tree.get(1)!; + const next = ops2Tree(state.idMap, new Set(), ops).tree.get(1)!; expect(next.startTime).to.equal(20); expect(next.endTime).to.equal(40); @@ -120,7 +121,7 @@ describe("ops2Tree", () => { "Remove 4", ]); - const next = ops2Tree(state.idMap, [], ops).tree; + const next = ops2Tree(state.idMap, new Set(), ops).tree; const root = next.get(1)!; const span = next.get(2)!; @@ -142,7 +143,7 @@ describe("ops2Tree", () => { "Remove 4", ]); - const next = ops2Tree(state.idMap, [], ops); + const next = ops2Tree(state.idMap, new Set(), ops); expect(next.removals).to.deep.equal([3, 4]); }); @@ -158,7 +159,7 @@ describe("ops2Tree", () => { "Remove 2", ]); - const next = ops2Tree(state.idMap, [], ops); + const next = ops2Tree(state.idMap, new Set(), ops); expect(Array.from(next.tree.keys())).to.deep.equal([1, 4]); }); }); @@ -178,7 +179,7 @@ describe("ops2Tree", () => { "Reorder 1 [3,2]", ]); - const next = ops2Tree(state.idMap, [], ops).tree; + const next = ops2Tree(state.idMap, new Set(), ops).tree; expect(next.get(1)!.children).to.deep.equal([3, 2]); }); @@ -192,8 +193,10 @@ describe("ops2Tree", () => { "Update timings 1 time 20:40", ]); - expect(() => ops2Tree(new Map(), [], ops)).to.not.throw(); - expect(ops2Tree(new Map(), [], ops).tree.get(1)!.startTime).to.equal(20); + expect(() => ops2Tree(new Map(), new Set(), ops)).to.not.throw(); + expect( + ops2Tree(new Map(), new Set(), ops).tree.get(1)!.startTime, + ).to.equal(20); }); }); }); diff --git a/src/adapter/protocol/operations.ts b/src/adapter/protocol/operations.ts index b082bf8d..0ed86a29 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"; @@ -12,23 +12,39 @@ import { ParsedStats, parseStats } from "../shared/stats"; * * We currently expect all operations to be in order. */ -export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { +export function ops2Tree(oldTree: Tree, existingRoots: Set, ops: number[]) { const pending: Tree = new Map(oldTree); const rootId = ops[0]; - const roots: ID[] = [...existingRoots]; + const roots = new Set(existingRoots); 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.add(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; } @@ -84,12 +104,6 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) { } } - // Check if node was a root - const rootIdx = roots.indexOf(node.id); - if (rootIdx > -1) { - roots.splice(rootIdx, 1); - } - // Delete children recursively const stack = [node.id]; let item; @@ -159,5 +173,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..72c23396 100644 --- a/src/view/components/elements/TreeView.tsx +++ b/src/view/components/elements/TreeView.tsx @@ -91,9 +91,9 @@ export function TreeView() { useAutoIndent(paneRef, [listItems]); // When the devtools is connected, but nothing has been sent to the panel yet - const isOnlyConnected = nodeList.length === 0 && roots.length === 0; + const isOnlyConnected = nodeList.length === 0 && roots.size === 0; // When client sent messages, but no nodes were sent due to filters. - const hasNoResults = nodeList.length === 0 && roots.length > 0; + const hasNoResults = nodeList.length === 0 && roots.size > 0; return (
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,7 @@ export function TreeItem(props: { key: any; id: ID; top: number }) { if (!node) return null; - const isRoot = node.parent === -1 && roots.includes(node.id); + const isRoot = roots.has(node.id); return (
{node.children.length > 0 && (