mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-14 01:20:03 +00:00
fix: cache ctx.measureText results to avoid redundant calls in draw loop (#9404)
## What Add a per-frame text measurement cache for all hot-path ctx.measureText() calls. ## Why drawTruncatingText() in BaseWidget calls ctx.measureText() per widget per frame with zero caching. For a 50-node workflow at 60fps: ~78,000-243,000 measureText calls/sec. Text labels rarely change between frames. ## How Global Map<string, number> cache keyed by font+text, cleared once per frame at the start of drawFrontCanvas(). Replaces direct ctx.measureText() calls in BaseWidget.drawTruncatingText, draw.ts truncateTextToWidth/drawTextInArea, LGraphBadge.getWidth, LGraphButton.getWidth, and textUtils.truncateText. ## Perf Impact Expected: ~95% reduction in measureText calls (only cache misses on first frame and value changes). Firefox has slower measureText than Chrome, so this disproportionately benefits Firefox. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9404-fix-cache-ctx-measureText-results-to-avoid-redundant-calls-in-draw-loop-31a6d73d3650814e9cdac16949c55cb7) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
179
.claude/skills/perf-fix-with-proof/SKILL.md
Normal file
179
.claude/skills/perf-fix-with-proof/SKILL.md
Normal file
@@ -0,0 +1,179 @@
|
||||
---
|
||||
name: perf-fix-with-proof
|
||||
description: 'Ships performance fixes with CI-proven improvement using stacked PRs. PR1 adds a @perf test (establishes baseline on main), PR2 adds the fix (CI shows delta). Use when implementing a perf optimization and wanting to prove it in CI.'
|
||||
---
|
||||
|
||||
# Performance Fix with Proof
|
||||
|
||||
Ships perf fixes as two stacked PRs so CI automatically proves the improvement.
|
||||
|
||||
## Why Two PRs
|
||||
|
||||
The `ci-perf-report.yaml` workflow compares PR metrics against the **base branch baseline**. If you add a new `@perf` test in the same PR as the fix, that test doesn't exist on main yet — no baseline, no delta, no proof. Stacking solves this:
|
||||
|
||||
1. **PR1 (test-only)** — adds the `@perf` test that exercises the bottleneck. Merges to main. CI runs it on main → baseline established.
|
||||
2. **PR2 (fix)** — adds the optimization. CI runs the same test → compares against PR1's baseline → delta shows improvement.
|
||||
|
||||
## Workflow
|
||||
|
||||
### Step 1: Create the test branch
|
||||
|
||||
```bash
|
||||
git worktree add <worktree-path> -b perf/test-<name> origin/main
|
||||
```
|
||||
|
||||
### Step 2: Write the `@perf` test
|
||||
|
||||
Add a test to `browser_tests/tests/performance.spec.ts` (or a new file with `@perf` tag). The test should stress the specific bottleneck.
|
||||
|
||||
**Test structure:**
|
||||
|
||||
```typescript
|
||||
test('<descriptive name>', async ({ comfyPage }) => {
|
||||
// 1. Load a workflow that exercises the bottleneck
|
||||
await comfyPage.workflow.loadWorkflow('<workflow>')
|
||||
|
||||
// 2. Start measuring
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// 3. Perform the action that triggers the bottleneck (at scale)
|
||||
for (let i = 0; i < N; i++) {
|
||||
// ... stress the hot path ...
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
|
||||
// 4. Stop measuring and record
|
||||
const m = await comfyPage.perf.stopMeasuring('<metric-name>')
|
||||
recordMeasurement(m)
|
||||
console.log(`<name>: ${m.styleRecalcs} recalcs, ${m.layouts} layouts`)
|
||||
})
|
||||
```
|
||||
|
||||
**Available metrics** (from `PerformanceHelper`):
|
||||
|
||||
- `m.styleRecalcs` / `m.styleRecalcDurationMs` — style recalculation count and time
|
||||
- `m.layouts` / `m.layoutDurationMs` — forced layout count and time
|
||||
- `m.taskDurationMs` — total main-thread JS execution time
|
||||
- `m.heapDeltaBytes` — memory pressure delta
|
||||
|
||||
**Key helpers** (from `ComfyPage`):
|
||||
|
||||
- `comfyPage.perf.startMeasuring()` / `.stopMeasuring(name)` — CDP metrics capture
|
||||
- `comfyPage.nextFrame()` — wait one animation frame
|
||||
- `comfyPage.workflow.loadWorkflow(name)` — load a test workflow from `browser_tests/assets/`
|
||||
- `comfyPage.canvas` — the canvas locator
|
||||
- `comfyPage.page.mouse.move(x, y)` — mouse interaction
|
||||
|
||||
### Step 3: Add test workflow asset (if needed)
|
||||
|
||||
If the bottleneck needs a specific workflow (e.g., 50+ nodes, many DOM widgets), add it to `browser_tests/assets/`. Keep it minimal — only the structure needed to trigger the bottleneck.
|
||||
|
||||
### Step 4: Verify locally
|
||||
|
||||
```bash
|
||||
pnpm exec playwright test --project=performance --grep "<test name>"
|
||||
```
|
||||
|
||||
Confirm the test runs and produces reasonable metric values.
|
||||
|
||||
### Step 5: Create PR1 (test-only)
|
||||
|
||||
```bash
|
||||
pnpm typecheck:browser
|
||||
pnpm lint
|
||||
git add browser_tests/
|
||||
git commit -m "test: add perf test for <bottleneck description>"
|
||||
git push -u origin perf/test-<name>
|
||||
gh pr create --title "test: add perf test for <bottleneck>" \
|
||||
--body "Adds a @perf test to establish a baseline for <bottleneck>.
|
||||
|
||||
This is PR 1 of 2. The fix will follow in a separate PR once this baseline is established on main.
|
||||
|
||||
## What
|
||||
Adds \`<test-name>\` to the performance test suite measuring <metric> during <action>.
|
||||
|
||||
## Why
|
||||
Needed to prove the improvement from the upcoming fix for backlog item #<N>." \
|
||||
--base main
|
||||
```
|
||||
|
||||
### Step 6: Get PR1 merged
|
||||
|
||||
Once PR1 merges, CI runs the test on main → baseline artifact saved.
|
||||
|
||||
### Step 7: Create PR2 (fix) on top of main
|
||||
|
||||
```bash
|
||||
git worktree add <worktree-path> -b perf/fix-<name> origin/main
|
||||
```
|
||||
|
||||
Implement the fix. The `@perf` test from PR1 is now on main and will run automatically. CI will:
|
||||
|
||||
1. Run the test on the PR branch
|
||||
2. Download the baseline from main (which includes PR1's test results)
|
||||
3. Post a PR comment showing the delta
|
||||
|
||||
### Step 8: Verify the improvement shows in CI
|
||||
|
||||
The `ci-perf-report.yaml` posts a comment like:
|
||||
|
||||
```markdown
|
||||
## ⚡ Performance Report
|
||||
|
||||
| Metric | Baseline | PR (n=3) | Δ | Sig |
|
||||
| --------------------- | -------- | -------- | ---- | --- |
|
||||
| <name>: style recalcs | 450 | 12 | -97% | 🟢 |
|
||||
```
|
||||
|
||||
If Δ is negative for the target metric, the fix is proven.
|
||||
|
||||
## Test Design Guidelines
|
||||
|
||||
1. **Stress the specific bottleneck** — don't measure everything, isolate the hot path
|
||||
2. **Use enough iterations** — the test should run long enough that the metric difference is clear (100+ frames for idle tests, 50+ interactions for event tests)
|
||||
3. **Keep it deterministic** — avoid timing-dependent assertions; measure counts not durations when possible
|
||||
4. **Match the backlog entry** — reference the backlog item number in the test name or PR description
|
||||
|
||||
## Examples
|
||||
|
||||
**Testing DOM widget reactive mutations (backlog #8):**
|
||||
|
||||
```typescript
|
||||
test('DOM widget positioning recalculations', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('default')
|
||||
await comfyPage.perf.startMeasuring()
|
||||
// Idle for 120 frames — DOM widgets update position every frame
|
||||
for (let i = 0; i < 120; i++) {
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
const m = await comfyPage.perf.stopMeasuring('dom-widget-idle')
|
||||
recordMeasurement(m)
|
||||
})
|
||||
```
|
||||
|
||||
**Testing measureText caching (backlog #4):**
|
||||
|
||||
```typescript
|
||||
test('canvas text rendering with many nodes', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('large-workflow-50-nodes')
|
||||
await comfyPage.perf.startMeasuring()
|
||||
for (let i = 0; i < 60; i++) {
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
const m = await comfyPage.perf.stopMeasuring('text-rendering-50-nodes')
|
||||
recordMeasurement(m)
|
||||
})
|
||||
```
|
||||
|
||||
## Reference
|
||||
|
||||
| Resource | Path |
|
||||
| ----------------- | ----------------------------------------------------- |
|
||||
| Perf test file | `browser_tests/tests/performance.spec.ts` |
|
||||
| PerformanceHelper | `browser_tests/fixtures/helpers/PerformanceHelper.ts` |
|
||||
| Perf reporter | `browser_tests/helpers/perfReporter.ts` |
|
||||
| CI workflow | `.github/workflows/ci-perf-report.yaml` |
|
||||
| Report generator | `scripts/perf-report.ts` |
|
||||
| Stats utilities | `scripts/perf-stats.ts` |
|
||||
| Backlog | `docs/perf/BACKLOG.md` (local only, not committed) |
|
||||
| Playbook | `docs/perf/PLAYBOOK.md` (local only, not committed) |
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
|
||||
import { LGraphIcon } from './LGraphIcon'
|
||||
import type { LGraphIconOptions } from './LGraphIcon'
|
||||
import { cachedMeasureText } from './utils/textMeasureCache'
|
||||
|
||||
export enum BadgePosition {
|
||||
TopLeft = 'top-left',
|
||||
@@ -80,11 +81,11 @@ export class LGraphBadge {
|
||||
iconWidth = this.icon.size + this.padding
|
||||
} else if (this.icon.unicode) {
|
||||
ctx.font = `${this.icon.fontSize}px '${this.icon.fontFamily}'`
|
||||
iconWidth = ctx.measureText(this.icon.unicode).width + this.padding
|
||||
iconWidth = cachedMeasureText(ctx, this.icon.unicode) + this.padding
|
||||
}
|
||||
}
|
||||
ctx.font = `${this.fontSize}px sans-serif`
|
||||
const textWidth = this.text ? ctx.measureText(this.text).width : 0
|
||||
const textWidth = this.text ? cachedMeasureText(ctx, this.text) : 0
|
||||
ctx.font = font
|
||||
return iconWidth + textWidth + this.padding * 2
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { LGraphBadge } from './LGraphBadge'
|
||||
import type { LGraphBadgeOptions } from './LGraphBadge'
|
||||
import { Rectangle } from './infrastructure/Rectangle'
|
||||
import { cachedMeasureText } from './utils/textMeasureCache'
|
||||
|
||||
export interface LGraphButtonOptions extends LGraphBadgeOptions {
|
||||
name?: string // To identify the button
|
||||
@@ -22,7 +23,7 @@ export class LGraphButton extends LGraphBadge {
|
||||
ctx.font = `${this.fontSize}px 'PrimeIcons'`
|
||||
|
||||
// For icon buttons, just measure the text width without padding
|
||||
const textWidth = this.text ? ctx.measureText(this.text).width : 0
|
||||
const textWidth = this.text ? cachedMeasureText(ctx, this.text) : 0
|
||||
|
||||
ctx.font = font
|
||||
return textWidth
|
||||
|
||||
@@ -26,6 +26,10 @@ import type { RerouteId } from './Reroute'
|
||||
import { LinkConnector } from './canvas/LinkConnector'
|
||||
import { isOverNodeInput, isOverNodeOutput } from './canvas/measureSlots'
|
||||
import { strokeShape } from './draw'
|
||||
import {
|
||||
cachedMeasureText,
|
||||
clearTextMeasureCache
|
||||
} from './utils/textMeasureCache'
|
||||
import type {
|
||||
CustomEventDispatcher,
|
||||
ICustomEventTarget
|
||||
@@ -4893,6 +4897,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
* draws the front canvas (the one containing all the nodes)
|
||||
*/
|
||||
drawFrontCanvas(): void {
|
||||
clearTextMeasureCache()
|
||||
this.dirty_canvas = false
|
||||
|
||||
const { ctx, canvas, graph } = this
|
||||
@@ -5622,8 +5627,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
text = text.substring(0, 30)
|
||||
|
||||
ctx.font = '14px Courier New'
|
||||
const info = ctx.measureText(text)
|
||||
const w = info.width + 20
|
||||
const w = cachedMeasureText(ctx, text) + 20
|
||||
const h = 24
|
||||
ctx.shadowColor = 'black'
|
||||
ctx.shadowOffsetX = 2
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
} from '@/lib/litegraph/src/utils/type'
|
||||
|
||||
import { SUBGRAPH_OUTPUT_ID } from '@/lib/litegraph/src/constants'
|
||||
import { cachedMeasureText } from '@/lib/litegraph/src/utils/textMeasureCache'
|
||||
import type { DragAndScale } from './DragAndScale'
|
||||
import type { LGraph } from './LGraph'
|
||||
import { BadgePosition, LGraphBadge } from './LGraphBadge'
|
||||
@@ -2083,7 +2084,7 @@ export class LGraphNode
|
||||
this._collapsed_width = Math.min(
|
||||
this.size[0],
|
||||
ctx
|
||||
? ctx.measureText(this.getTitle() ?? '').width +
|
||||
? cachedMeasureText(ctx, this.getTitle() ?? '') +
|
||||
LiteGraph.NODE_TITLE_HEIGHT * 2
|
||||
: 0
|
||||
)
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { Rectangle } from './infrastructure/Rectangle'
|
||||
import type { CanvasColour } from './interfaces'
|
||||
import { LiteGraph } from './litegraph'
|
||||
import { RenderShape, TitleMode } from './types/globalEnums'
|
||||
import { cachedMeasureText } from './utils/textMeasureCache'
|
||||
|
||||
const ELLIPSIS = '\u2026'
|
||||
const TWO_DOT_LEADER = '\u2025'
|
||||
@@ -161,17 +162,17 @@ function truncateTextToWidth(
|
||||
if (!(maxWidth > 0)) return ''
|
||||
|
||||
// Text fits
|
||||
const fullWidth = ctx.measureText(text).width
|
||||
const fullWidth = cachedMeasureText(ctx, text)
|
||||
if (fullWidth <= maxWidth) return text
|
||||
|
||||
const ellipsisWidth = ctx.measureText(ELLIPSIS).width * 0.75
|
||||
const ellipsisWidth = cachedMeasureText(ctx, ELLIPSIS) * 0.75
|
||||
|
||||
// Can't even fit ellipsis
|
||||
if (ellipsisWidth > maxWidth) {
|
||||
const twoDotsWidth = ctx.measureText(TWO_DOT_LEADER).width * 0.75
|
||||
const twoDotsWidth = cachedMeasureText(ctx, TWO_DOT_LEADER) * 0.75
|
||||
if (twoDotsWidth < maxWidth) return TWO_DOT_LEADER
|
||||
|
||||
const oneDotWidth = ctx.measureText(ONE_DOT_LEADER).width * 0.75
|
||||
const oneDotWidth = cachedMeasureText(ctx, ONE_DOT_LEADER) * 0.75
|
||||
return oneDotWidth < maxWidth ? ONE_DOT_LEADER : ''
|
||||
}
|
||||
|
||||
@@ -190,7 +191,7 @@ function truncateTextToWidth(
|
||||
}
|
||||
|
||||
const sub = text.substring(0, mid)
|
||||
const currentWidth = ctx.measureText(sub).width + ellipsisWidth
|
||||
const currentWidth = cachedMeasureText(ctx, sub) + ellipsisWidth
|
||||
|
||||
if (currentWidth <= maxWidth) {
|
||||
// This length fits, try potentially longer
|
||||
@@ -217,7 +218,7 @@ export function drawTextInArea({
|
||||
const { left, right, bottom, width, centreX } = area
|
||||
|
||||
// Text already fits
|
||||
const fullWidth = ctx.measureText(text).width
|
||||
const fullWidth = cachedMeasureText(ctx, text)
|
||||
if (fullWidth <= width) {
|
||||
ctx.textAlign = align
|
||||
const x = align === 'left' ? left : align === 'right' ? right : centreX
|
||||
@@ -237,5 +238,5 @@ export function drawTextInArea({
|
||||
// Draw the ellipsis, right-aligned to the button
|
||||
ctx.textAlign = 'right'
|
||||
const ellipsis = truncated.at(-1)!
|
||||
ctx.fillText(ellipsis, right, bottom, ctx.measureText(ellipsis).width * 0.75)
|
||||
ctx.fillText(ellipsis, right, bottom, cachedMeasureText(ctx, ellipsis) * 0.75)
|
||||
}
|
||||
|
||||
63
src/lib/litegraph/src/utils/textMeasureCache.test.ts
Normal file
63
src/lib/litegraph/src/utils/textMeasureCache.test.ts
Normal file
@@ -0,0 +1,63 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { cachedMeasureText, clearTextMeasureCache } from './textMeasureCache'
|
||||
|
||||
function createMockCtx(font = '12px sans-serif'): CanvasRenderingContext2D {
|
||||
return {
|
||||
font,
|
||||
measureText: vi.fn((text: string) => ({ width: text.length * 7 }))
|
||||
} as unknown as CanvasRenderingContext2D
|
||||
}
|
||||
|
||||
describe('textMeasureCache', () => {
|
||||
beforeEach(() => {
|
||||
clearTextMeasureCache()
|
||||
})
|
||||
|
||||
it('returns the measured width', () => {
|
||||
const ctx = createMockCtx()
|
||||
const width = cachedMeasureText(ctx, 'hello')
|
||||
expect(width).toBe(35)
|
||||
expect(ctx.measureText).toHaveBeenCalledWith('hello')
|
||||
})
|
||||
|
||||
it('returns cached result on second call without re-measuring', () => {
|
||||
const ctx = createMockCtx()
|
||||
const first = cachedMeasureText(ctx, 'hello')
|
||||
const second = cachedMeasureText(ctx, 'hello')
|
||||
|
||||
expect(first).toBe(second)
|
||||
expect(ctx.measureText).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('uses font as part of the cache key', () => {
|
||||
const ctx1 = createMockCtx('12px sans-serif')
|
||||
const ctx2 = createMockCtx('24px monospace')
|
||||
|
||||
cachedMeasureText(ctx1, 'hello')
|
||||
cachedMeasureText(ctx2, 'hello')
|
||||
|
||||
expect(ctx1.measureText).toHaveBeenCalledTimes(1)
|
||||
expect(ctx2.measureText).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('clearTextMeasureCache resets the cache', () => {
|
||||
const ctx = createMockCtx()
|
||||
cachedMeasureText(ctx, 'hello')
|
||||
expect(ctx.measureText).toHaveBeenCalledTimes(1)
|
||||
|
||||
clearTextMeasureCache()
|
||||
|
||||
cachedMeasureText(ctx, 'hello')
|
||||
expect(ctx.measureText).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('caches different text strings separately', () => {
|
||||
const ctx = createMockCtx()
|
||||
const w1 = cachedMeasureText(ctx, 'abc')
|
||||
const w2 = cachedMeasureText(ctx, 'abcd')
|
||||
|
||||
expect(w1).not.toBe(w2)
|
||||
expect(ctx.measureText).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
18
src/lib/litegraph/src/utils/textMeasureCache.ts
Normal file
18
src/lib/litegraph/src/utils/textMeasureCache.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
const cache = new Map<string, number>()
|
||||
|
||||
export function cachedMeasureText(
|
||||
ctx: CanvasRenderingContext2D,
|
||||
text: string
|
||||
): number {
|
||||
const key = `${ctx.font}\0${text}`
|
||||
const cached = cache.get(key)
|
||||
if (cached !== undefined) return cached
|
||||
|
||||
const width = ctx.measureText(text).width
|
||||
cache.set(key, width)
|
||||
return width
|
||||
}
|
||||
|
||||
export function clearTextMeasureCache(): void {
|
||||
cache.clear()
|
||||
}
|
||||
@@ -1,3 +1,5 @@
|
||||
import { cachedMeasureText } from './textMeasureCache'
|
||||
|
||||
/**
|
||||
* Truncates text to fit within a given width using binary search for optimal performance.
|
||||
* @param ctx The canvas rendering context used for text measurement
|
||||
@@ -12,13 +14,13 @@ export function truncateText(
|
||||
maxWidth: number,
|
||||
ellipsis: string = '...'
|
||||
): string {
|
||||
const textWidth = ctx.measureText(text).width
|
||||
const textWidth = cachedMeasureText(ctx, text)
|
||||
|
||||
if (textWidth <= maxWidth || maxWidth <= 0) {
|
||||
return text
|
||||
}
|
||||
|
||||
const ellipsisWidth = ctx.measureText(ellipsis).width
|
||||
const ellipsisWidth = cachedMeasureText(ctx, ellipsis)
|
||||
const availableWidth = maxWidth - ellipsisWidth
|
||||
|
||||
if (availableWidth <= 0) {
|
||||
@@ -33,7 +35,7 @@ export function truncateText(
|
||||
while (low <= high) {
|
||||
const mid = Math.floor((low + high) / 2)
|
||||
const testText = text.substring(0, mid)
|
||||
const testWidth = ctx.measureText(testText).width
|
||||
const testWidth = cachedMeasureText(ctx, testText)
|
||||
|
||||
if (testWidth <= availableWidth) {
|
||||
bestFit = mid
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { t } from '@/i18n'
|
||||
import { drawTextInArea } from '@/lib/litegraph/src/draw'
|
||||
import { cachedMeasureText } from '@/lib/litegraph/src/utils/textMeasureCache'
|
||||
import { Rectangle } from '@/lib/litegraph/src/infrastructure/Rectangle'
|
||||
import type { Point } from '@/lib/litegraph/src/interfaces'
|
||||
import type { NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
@@ -349,8 +350,8 @@ export abstract class BaseWidget<TWidget extends IBaseWidget = IBaseWidget>
|
||||
|
||||
// Measure label and value
|
||||
const { displayName, _displayValue } = this
|
||||
const labelWidth = ctx.measureText(displayName).width
|
||||
const valueWidth = ctx.measureText(_displayValue).width
|
||||
const labelWidth = cachedMeasureText(ctx, displayName)
|
||||
const valueWidth = cachedMeasureText(ctx, _displayValue)
|
||||
|
||||
const gap = BaseWidget.labelValueGap
|
||||
const x = margin * 2 + leftPadding
|
||||
|
||||
Reference in New Issue
Block a user