Skip to content

Commit 1027be4

Browse files
committed
docs: make the perf record reproducible and its comparison honest
Three corrections from review: - The scripts lived under temp/, which is gitignored, so the reproduction section could not be run by anyone including a later me. They are in scripts/perf/ now, with the commands updated to match. - The litegraph column is that renderer before this work. The minimap fix is renderer-independent and sped it up too, so main's canvas renderer is now faster than every number in it and the ratios read better than they should. Says so explicitly, with the post-change numbers. - p95 is collected and was not reported, while the repo's own render gate is set on p95 rather than median - which is exactly where fill-in and allocation costs show up.
1 parent 7db5295 commit 1027be4

5 files changed

Lines changed: 371 additions & 7 deletions

File tree

docs/architecture/vue-nodes-large-graph-performance.md

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,13 @@ before any of this work, and is the bar Vue nodes needed to clear.
2121
| 3000 | min | 41.6 ms | 280.5 ms | 34.3 ms | 0.82x | 8.2x |
2222
| 3000 | fit | 126.5 ms | 380.7 ms | 104.0 ms | 0.82x | 3.7x |
2323

24-
Vue nodes now match or beat the original canvas-renderer baseline in five of
25-
six configurations. DOM element counts on the 3000-node graph fell from 227,787
24+
The `litegraph` column is that renderer _before this work_, which is the bar
25+
Vue nodes had to clear - but it is not the bar after merge. The minimap fix is
26+
renderer-independent and speeds the canvas renderer up too, so main's canvas
27+
renderer is now faster than every number in that column. Measured after the
28+
change, the two renderers at 3000 nodes zoomed out are 104ms and 113ms, i.e.
29+
about 0.92x rather than the 0.82x the table implies. Read the ratios as "versus
30+
the problem we started from", not "versus main today". DOM element counts on the 3000-node graph fell from 227,787
2631
to 5,355 at working zoom and to 418 (application chrome only) when zoomed out.
2732

2833
`min` is the lowest zoom reachable by scrolling (0.1). `fit` is the zoom a
@@ -112,6 +117,11 @@ below the threshold that measurement never arrived and links stayed hidden.
112117
Crossing into the simplified mode now releases that wait, and links fall back
113118
to positions derived from node bounds.
114119

120+
p95 is reported alongside the median throughout, because the repo's own render
121+
gate (`docs/architecture/ecs-migration-plan.md`, referenced by ADR 0008) is set
122+
on p95 rather than median - and because per-frame allocation and fill-in cost
123+
show up in the tail while leaving the median flat.
124+
115125
## Method
116126

117127
Graphs are built by stamping the default workflow across empty space in a grid,
@@ -121,13 +131,22 @@ driving a continuous pan, reporting the median and p95 of the stable portion.
121131
Both renderers are measured in the same build, on the same workflow, with a
122132
fresh page each.
123133

124-
Scripts live under `temp/scripts/` (untracked):
134+
Scripts live under `scripts/perf/`. They were untracked under `temp/` in an
135+
earlier revision, which made this section unreproducible for anyone including
136+
a later me, since `/temp/` is gitignored:
125137

126138
```bash
127-
node temp/scripts/gen-big-graph.mjs 3000 # build a workflow
128-
LABEL=my-run node temp/scripts/sweep.mjs # measure both renderers
129-
node temp/scripts/build-report.mjs # render temp/perf-report.html
130-
SCALE=0.0255 node temp/scripts/profile-pan.mjs temp/big-graph-3000.json
139+
# Build a workflow of a given size
140+
node scripts/perf/gen-sized-workflow.mjs 3000 /tmp/wf-3000.json
141+
142+
# Pan cost, both renderers, with the culling threshold swept
143+
node scripts/perf/sweep-threshold.mjs
144+
145+
# Zoom cost, both renderers
146+
WORKFLOW=/tmp/wf-3000.json node scripts/perf/measure-zoom.mjs
147+
148+
# Mounted-element counts while panning
149+
node scripts/perf/measure-culling.mjs
131150
```
132151

133152
Caveat: one sample per configuration, with run-to-run variance measured at
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/** Builds a workflow of exactly N nodes by tiling the default fixture. */
2+
import { readFileSync, writeFileSync } from 'node:fs'
3+
const SRC = JSON.parse(
4+
readFileSync('browser_tests/assets/large-graph-workflow.json', 'utf8')
5+
)
6+
const N = Number(process.argv[2])
7+
const out = { ...SRC, nodes: [], links: [], groups: [] }
8+
const COLS = 6,
9+
TILE_W = 2600,
10+
TILE_H = 2200
11+
let copy = 0
12+
while (out.nodes.length < N) {
13+
const nodeOffset = copy * (SRC.last_node_id + 1)
14+
const linkOffset = copy * (SRC.last_link_id + 1)
15+
const dx = (copy % COLS) * TILE_W,
16+
dy = Math.floor(copy / COLS) * TILE_H
17+
const keep = new Set()
18+
for (const node of SRC.nodes) {
19+
if (out.nodes.length >= N) break
20+
const c = structuredClone(node)
21+
c.id = node.id + nodeOffset
22+
c.pos = [node.pos[0] + dx, node.pos[1] + dy]
23+
for (const i of c.inputs ?? []) if (i.link != null) i.link += linkOffset
24+
for (const o of c.outputs ?? [])
25+
if (o.links) o.links = o.links.map((l) => l + linkOffset)
26+
out.nodes.push(c)
27+
keep.add(c.id)
28+
}
29+
for (const l of SRC.links ?? []) {
30+
const [id, oid, os, tid, ts, ty] = l
31+
if (keep.has(oid + nodeOffset) && keep.has(tid + nodeOffset))
32+
out.links.push([
33+
id + linkOffset,
34+
oid + nodeOffset,
35+
os,
36+
tid + nodeOffset,
37+
ts,
38+
ty
39+
])
40+
}
41+
copy++
42+
}
43+
// Drop dangling input links so the graph stays consistent.
44+
const linkIds = new Set(out.links.map((l) => l[0]))
45+
for (const n of out.nodes) {
46+
for (const i of n.inputs ?? [])
47+
if (i.link != null && !linkIds.has(i.link)) i.link = null
48+
for (const o of n.outputs ?? [])
49+
if (o.links) o.links = o.links.filter((l) => linkIds.has(l))
50+
}
51+
out.last_node_id = copy * (SRC.last_node_id + 1)
52+
out.last_link_id = copy * (SRC.last_link_id + 1)
53+
writeFileSync(process.argv[3], JSON.stringify(out))
54+
console.log(
55+
`${process.argv[3]}: ${out.nodes.length} nodes, ${out.links.length} links`
56+
)

scripts/perf/measure-culling.mjs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* Measures how many Vue node elements are actually in the DOM for a large
3+
* workflow, at default zoom and while panning.
4+
*
5+
* Usage: node temp/scripts/measure-culling.mjs
6+
*/
7+
import { readFileSync } from 'node:fs'
8+
import { chromium } from '@playwright/test'
9+
10+
const APP_URL = process.env.APP_URL ?? 'http://localhost:5173'
11+
const WORKFLOW = JSON.parse(
12+
readFileSync('browser_tests/assets/large-graph-workflow.json', 'utf8')
13+
)
14+
15+
const countNodes = () => document.querySelectorAll('[data-node-id]').length
16+
17+
const browser = await chromium.launch()
18+
const page = await browser.newPage({ viewport: { width: 1280, height: 900 } })
19+
20+
page.on('pageerror', (e) => console.log('PAGE ERROR:', e.message))
21+
22+
await page.goto(APP_URL, { waitUntil: 'domcontentloaded' })
23+
await page.waitForFunction(() => window.app?.canvas != null, {
24+
timeout: 60_000
25+
})
26+
27+
await page.evaluate(async (workflow) => {
28+
await window.app.loadGraphData(workflow)
29+
}, WORKFLOW)
30+
31+
await page.waitForFunction(
32+
() => document.querySelectorAll('[data-node-id]').length > 0,
33+
{ timeout: 60_000 }
34+
)
35+
await page.waitForTimeout(2500)
36+
37+
const totalNodes = await page.evaluate(() => window.app.graph._nodes.length)
38+
const atDefaultZoom = await page.evaluate(countNodes)
39+
40+
// Pan continuously across the graph, sampling the mounted count as we go.
41+
const readCamera = () => {
42+
const ds = window.app.canvas.ds
43+
return {
44+
x: Math.round(ds.offset[0]),
45+
y: Math.round(ds.offset[1]),
46+
z: ds.scale
47+
}
48+
}
49+
50+
const cameraBefore = await page.evaluate(readCamera)
51+
52+
// Pan by driving the canvas transform directly, so the measurement does not
53+
// depend on which mouse button happens to be bound to panning.
54+
const samples = []
55+
for (let step = 1; step <= 6; step++) {
56+
await page.evaluate((dx) => {
57+
window.app.canvas.ds.offset[0] -= dx
58+
window.app.canvas.setDirty(true, true)
59+
}, 900)
60+
await page.waitForTimeout(160)
61+
samples.push(await page.evaluate(countNodes))
62+
}
63+
64+
const cameraAfter = await page.evaluate(readCamera)
65+
await page.waitForTimeout(600)
66+
const afterPan = await page.evaluate(countNodes)
67+
68+
// Continuously pan inside a rAF loop and record frame deltas.
69+
const frames = await page.evaluate(async () => {
70+
const ds = window.app.canvas.ds
71+
ds.offset[0] = 0
72+
const deltas = []
73+
74+
await new Promise((resolve) => {
75+
let last = performance.now()
76+
let i = 0
77+
const tick = () => {
78+
const now = performance.now()
79+
deltas.push(now - last)
80+
last = now
81+
ds.offset[0] -= 40
82+
window.app.canvas.setDirty(true, true)
83+
if (++i >= 150) return resolve()
84+
requestAnimationFrame(tick)
85+
}
86+
requestAnimationFrame(tick)
87+
})
88+
89+
// Drop the first few frames while the pan spins up.
90+
const stable = deltas.slice(10).sort((a, b) => a - b)
91+
const at = (q) => stable[Math.floor(stable.length * q)]
92+
return {
93+
medianFrameMs: +at(0.5).toFixed(1),
94+
p95FrameMs: +at(0.95).toFixed(1),
95+
worstFrameMs: +stable[stable.length - 1].toFixed(1)
96+
}
97+
})
98+
99+
console.log(
100+
JSON.stringify(
101+
{
102+
totalNodes,
103+
atDefaultZoom,
104+
cameraBefore,
105+
cameraAfter,
106+
duringPan: samples,
107+
peakDuringPan: Math.max(...samples),
108+
afterPanSettled: afterPan,
109+
frames
110+
},
111+
null,
112+
2
113+
)
114+
)
115+
116+
await browser.close()

scripts/perf/measure-zoom.mjs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/** Frame cost of a continuous wheel zoom in/out, Vue vs legacy. */
2+
import { readFileSync } from 'node:fs'
3+
import { chromium } from '@playwright/test'
4+
const W = JSON.parse(
5+
readFileSync(process.env.WORKFLOW ?? 'temp/huge-workflow.json', 'utf8')
6+
)
7+
const browser = await chromium.launch()
8+
9+
async function run(vueNodes) {
10+
const p = await browser.newPage({ viewport: { width: 1440, height: 900 } })
11+
await p.goto('http://localhost:5173', { waitUntil: 'domcontentloaded' })
12+
await p.waitForFunction(() => window.app?.canvas != null, { timeout: 60000 })
13+
await p.evaluate(async (v) => {
14+
const s = window.app.extensionManager.setting
15+
await s.set('Comfy.VueNodes.Enabled', v)
16+
await s.set('Comfy.Minimap.Visible', true)
17+
}, vueNodes)
18+
await p.reload({ waitUntil: 'domcontentloaded' })
19+
await p.waitForFunction(() => window.app?.canvas != null, { timeout: 60000 })
20+
await p.evaluate(async (w) => window.app.loadGraphData(w), W)
21+
await p.waitForTimeout(8000)
22+
23+
// Start at a working zoom centred on the graph.
24+
await p.evaluate(() => {
25+
const ds = window.app.canvas.ds,
26+
ns = window.app.graph._nodes
27+
const cx = ns.reduce((a, n) => a + n.pos[0], 0) / ns.length
28+
const cy = ns.reduce((a, n) => a + n.pos[1], 0) / ns.length
29+
ds.scale = 0.6
30+
ds.offset[0] = -cx + window.innerWidth / 2 / 0.6
31+
ds.offset[1] = -cy + window.innerHeight / 2 / 0.6
32+
window.app.canvas.setDirty(true, true)
33+
})
34+
await p.waitForTimeout(3000)
35+
36+
const stats = await p.evaluate(async () => {
37+
const canvas = window.app.canvas
38+
const deltas = []
39+
const samples = []
40+
const el = canvas.canvas
41+
const fire = (dy) =>
42+
el.dispatchEvent(
43+
new WheelEvent('wheel', {
44+
deltaY: dy,
45+
clientX: window.innerWidth / 2,
46+
clientY: window.innerHeight / 2,
47+
bubbles: true,
48+
cancelable: true
49+
})
50+
)
51+
await new Promise((resolve) => {
52+
let last = performance.now(),
53+
i = 0
54+
const tick = () => {
55+
const now = performance.now()
56+
deltas.push(now - last)
57+
last = now
58+
// 60 out then 60 back in
59+
fire(i < 60 ? 120 : -120)
60+
if (i % 10 === 0)
61+
samples.push({
62+
i,
63+
scale: +canvas.ds.scale.toFixed(3),
64+
dom: document.querySelectorAll('[data-node-id]').length,
65+
ms: +deltas[deltas.length - 1].toFixed(0)
66+
})
67+
if (++i >= 120) return resolve()
68+
requestAnimationFrame(tick)
69+
}
70+
requestAnimationFrame(tick)
71+
})
72+
window.__samples = samples
73+
const s = deltas.slice(10).sort((a, b) => a - b)
74+
const at = (q) => +s[Math.floor(s.length * q)].toFixed(1)
75+
return {
76+
median: at(0.5),
77+
p95: at(0.95),
78+
worst: +s[s.length - 1].toFixed(1),
79+
finalScale: +canvas.ds.scale.toFixed(3),
80+
domNodes: document.querySelectorAll('[data-node-id]').length,
81+
samples
82+
}
83+
})
84+
await p.close()
85+
return stats
86+
}
87+
88+
const legacy = await run(false)
89+
const vue = await run(true)
90+
console.log(JSON.stringify({ legacy, vue }, null, 2))
91+
await browser.close()

0 commit comments

Comments
 (0)