-
Notifications
You must be signed in to change notification settings - Fork 518
feat: add performance testing infrastructure with CDP metrics #9170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
a440e46
feat: add performance testing infrastructure with CDP metrics
christian-byrne 75a6981
fix: address CI failures and review feedback
christian-byrne 33d8b73
[automated] Apply ESLint and Oxfmt fixes
actions-user fe24533
fix: address remaining CodeRabbit feedback
christian-byrne 18ab1ee
fix: report job npm/catalog protocol error, add contents:read perm, a…
christian-byrne d6aa946
Merge branch 'main' into perf/testing-infrastructure
DrJKL 492eb08
fix: address review feedback on perf helpers
christian-byrne 02e014e
fix: guard against nested startMeasuring() calls
christian-byrne b7cd12b
[automated] Apply ESLint and Oxfmt fixes
actions-user c5aefce
fix: clear snapshot and add try/finally in dispose()
christian-byrne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| name: 'CI: Performance Report' | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, core/*] | ||
| paths-ignore: ['**/*.md'] | ||
| pull_request: | ||
| branches-ignore: [wip/*, draft/*, temp/*] | ||
| paths-ignore: ['**/*.md'] | ||
|
|
||
| concurrency: | ||
| group: perf-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| jobs: | ||
| perf-tests: | ||
| if: github.repository == 'Comfy-Org/ComfyUI_frontend' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 30 | ||
| container: | ||
| image: ghcr.io/comfy-org/comfyui-ci-container:0.0.12 | ||
| credentials: | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
| permissions: | ||
| contents: read | ||
| packages: read | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup frontend | ||
| uses: ./.github/actions/setup-frontend | ||
| with: | ||
| include_build_step: true | ||
|
|
||
| - name: Start ComfyUI server | ||
| uses: ./.github/actions/start-comfyui-server | ||
|
|
||
| - name: Run performance tests | ||
| id: perf | ||
| continue-on-error: true | ||
| run: pnpm exec playwright test --project=performance --workers=1 | ||
|
|
||
| - name: Upload perf metrics | ||
| if: always() | ||
| uses: actions/upload-artifact@v6 | ||
| with: | ||
| name: perf-metrics | ||
| path: test-results/perf-metrics.json | ||
| retention-days: 30 | ||
| if-no-files-found: warn | ||
|
|
||
| report: | ||
| needs: perf-tests | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
christian-byrne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| - name: Setup Node | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 22 | ||
|
|
||
| - name: Download PR perf metrics | ||
| continue-on-error: true | ||
| uses: actions/download-artifact@v7 | ||
| with: | ||
| name: perf-metrics | ||
| path: test-results/ | ||
|
|
||
| - name: Download baseline perf metrics | ||
| uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12 | ||
| with: | ||
| branch: ${{ github.event.pull_request.base.ref }} | ||
| workflow: ci-perf-report.yaml | ||
| event: push | ||
| name: perf-metrics | ||
| path: temp/perf-baseline/ | ||
| if_no_artifact_found: warn | ||
|
|
||
| - name: Generate perf report | ||
| run: npx --yes tsx scripts/perf-report.ts > perf-report.md | ||
|
|
||
| - name: Read perf report | ||
| id: perf-report | ||
| uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7 | ||
| with: | ||
| path: ./perf-report.md | ||
|
|
||
| - name: Create or update PR comment | ||
| uses: actions-cool/maintain-one-comment@4b2dbf086015f892dcb5e8c1106f5fccd6c1476b # v3.2.0 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| number: ${{ github.event.pull_request.number }} | ||
| body: | | ||
| ${{ steps.perf-report.outputs.content }} | ||
| <!-- COMFYUI_FRONTEND_PERF --> | ||
| body-include: '<!-- COMFYUI_FRONTEND_PERF -->' | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| import type { CDPSession, Page } from '@playwright/test' | ||
|
|
||
| interface PerfSnapshot { | ||
| RecalcStyleCount: number | ||
| RecalcStyleDuration: number | ||
| LayoutCount: number | ||
| LayoutDuration: number | ||
| TaskDuration: number | ||
| JSHeapUsedSize: number | ||
| Timestamp: number | ||
| } | ||
|
|
||
| export interface PerfMeasurement { | ||
| name: string | ||
| durationMs: number | ||
| styleRecalcs: number | ||
| styleRecalcDurationMs: number | ||
| layouts: number | ||
| layoutDurationMs: number | ||
| taskDurationMs: number | ||
| heapDeltaBytes: number | ||
| } | ||
|
|
||
| export class PerformanceHelper { | ||
| private cdp: CDPSession | null = null | ||
| private snapshot: PerfSnapshot | null = null | ||
|
|
||
| constructor(private readonly page: Page) {} | ||
|
|
||
| async init(): Promise<void> { | ||
| this.cdp = await this.page.context().newCDPSession(this.page) | ||
| await this.cdp.send('Performance.enable') | ||
| } | ||
|
|
||
| async dispose(): Promise<void> { | ||
| this.snapshot = null | ||
| if (this.cdp) { | ||
| try { | ||
| await this.cdp.send('Performance.disable') | ||
| } finally { | ||
| await this.cdp.detach() | ||
| this.cdp = null | ||
| } | ||
christian-byrne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private async getSnapshot(): Promise<PerfSnapshot> { | ||
| if (!this.cdp) throw new Error('PerformanceHelper not initialized') | ||
| const { metrics } = (await this.cdp.send('Performance.getMetrics')) as { | ||
| metrics: { name: string; value: number }[] | ||
| } | ||
| function get(name: string): number { | ||
| return metrics.find((m) => m.name === name)?.value ?? 0 | ||
| } | ||
| return { | ||
| RecalcStyleCount: get('RecalcStyleCount'), | ||
| RecalcStyleDuration: get('RecalcStyleDuration'), | ||
| LayoutCount: get('LayoutCount'), | ||
| LayoutDuration: get('LayoutDuration'), | ||
| TaskDuration: get('TaskDuration'), | ||
| JSHeapUsedSize: get('JSHeapUsedSize'), | ||
| Timestamp: get('Timestamp') | ||
christian-byrne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| async startMeasuring(): Promise<void> { | ||
| if (this.snapshot) { | ||
| throw new Error( | ||
| 'Measurement already in progress — call stopMeasuring() first' | ||
| ) | ||
| } | ||
| this.snapshot = await this.getSnapshot() | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| async stopMeasuring(name: string): Promise<PerfMeasurement> { | ||
| if (!this.snapshot) throw new Error('Call startMeasuring() first') | ||
| const after = await this.getSnapshot() | ||
| const before = this.snapshot | ||
| this.snapshot = null | ||
|
|
||
| function delta(key: keyof PerfSnapshot): number { | ||
| return after[key] - before[key] | ||
| } | ||
|
|
||
| return { | ||
| name, | ||
| durationMs: delta('Timestamp') * 1000, | ||
| styleRecalcs: delta('RecalcStyleCount'), | ||
| styleRecalcDurationMs: delta('RecalcStyleDuration') * 1000, | ||
| layouts: delta('LayoutCount'), | ||
| layoutDurationMs: delta('LayoutDuration') * 1000, | ||
| taskDurationMs: delta('TaskDuration') * 1000, | ||
| heapDeltaBytes: delta('JSHeapUsedSize') | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { mkdirSync, readdirSync, readFileSync, writeFileSync } from 'fs' | ||
| import { join } from 'path' | ||
|
|
||
| import type { PerfMeasurement } from '../fixtures/helpers/PerformanceHelper' | ||
|
|
||
| export interface PerfReport { | ||
| timestamp: string | ||
| gitSha: string | ||
| branch: string | ||
| measurements: PerfMeasurement[] | ||
| } | ||
|
|
||
| const TEMP_DIR = join('test-results', 'perf-temp') | ||
|
|
||
| export function recordMeasurement(m: PerfMeasurement) { | ||
| mkdirSync(TEMP_DIR, { recursive: true }) | ||
| const filename = `${m.name}-${Date.now()}.json` | ||
| writeFileSync(join(TEMP_DIR, filename), JSON.stringify(m)) | ||
| } | ||
|
|
||
| export function writePerfReport( | ||
| gitSha = process.env.GITHUB_SHA ?? 'local', | ||
| branch = process.env.GITHUB_HEAD_REF ?? 'local' | ||
| ) { | ||
| if (!readdirSync('test-results', { withFileTypes: true }).length) return | ||
|
|
||
| let tempFiles: string[] | ||
| try { | ||
| tempFiles = readdirSync(TEMP_DIR).filter((f) => f.endsWith('.json')) | ||
| } catch { | ||
| return | ||
| } | ||
| if (tempFiles.length === 0) return | ||
|
|
||
| const measurements: PerfMeasurement[] = tempFiles.map((f) => | ||
| JSON.parse(readFileSync(join(TEMP_DIR, f), 'utf-8')) | ||
| ) | ||
|
|
||
| const report: PerfReport = { | ||
| timestamp: new Date().toISOString(), | ||
| gitSha, | ||
| branch, | ||
| measurements | ||
| } | ||
| writeFileSync( | ||
| join('test-results', 'perf-metrics.json'), | ||
| JSON.stringify(report, null, 2) | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { comfyPageFixture as test } from '../fixtures/ComfyPage' | ||
| import { recordMeasurement } from '../helpers/perfReporter' | ||
|
|
||
| test.describe('Performance', { tag: ['@perf'] }, () => { | ||
| test('canvas idle style recalculations', async ({ comfyPage }) => { | ||
| await comfyPage.workflow.loadWorkflow('default') | ||
| await comfyPage.perf.startMeasuring() | ||
|
|
||
| // Let the canvas idle for 2 seconds — no user interaction. | ||
| // Measures baseline style recalcs from reactive state + render loop. | ||
| for (let i = 0; i < 120; i++) { | ||
| await comfyPage.nextFrame() | ||
| } | ||
|
|
||
| const m = await comfyPage.perf.stopMeasuring('canvas-idle') | ||
| recordMeasurement(m) | ||
| console.log( | ||
| `Canvas idle: ${m.styleRecalcs} style recalcs, ${m.layouts} layouts` | ||
| ) | ||
| }) | ||
|
|
||
| test('canvas mouse interaction style recalculations', async ({ | ||
| comfyPage | ||
| }) => { | ||
| await comfyPage.workflow.loadWorkflow('default') | ||
| await comfyPage.perf.startMeasuring() | ||
|
|
||
| const canvas = comfyPage.canvas | ||
| const box = await canvas.boundingBox() | ||
| if (!box) throw new Error('Canvas bounding box not available') | ||
|
|
||
| // Sweep mouse across the canvas — crosses nodes, empty space, slots | ||
| for (let i = 0; i < 100; i++) { | ||
| await comfyPage.page.mouse.move( | ||
| box.x + (box.width * i) / 100, | ||
| box.y + (box.height * (i % 3)) / 3 | ||
| ) | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const m = await comfyPage.perf.stopMeasuring('canvas-mouse-sweep') | ||
| recordMeasurement(m) | ||
| console.log( | ||
| `Mouse sweep: ${m.styleRecalcs} style recalcs, ${m.layouts} layouts` | ||
| ) | ||
| }) | ||
|
|
||
| test('DOM widget clipping during node selection', async ({ comfyPage }) => { | ||
| // Load default workflow which has DOM widgets (text inputs, combos) | ||
| await comfyPage.workflow.loadWorkflow('default') | ||
| await comfyPage.perf.startMeasuring() | ||
|
|
||
| // Select and deselect nodes rapidly to trigger clipping recalculation | ||
| const canvas = comfyPage.canvas | ||
| const box = await canvas.boundingBox() | ||
| if (!box) throw new Error('Canvas bounding box not available') | ||
|
|
||
| for (let i = 0; i < 20; i++) { | ||
| // Click on canvas area (nodes occupy various positions) | ||
| await comfyPage.page.mouse.click( | ||
| box.x + box.width / 3 + (i % 5) * 30, | ||
| box.y + box.height / 3 + (i % 4) * 30 | ||
| ) | ||
| await comfyPage.nextFrame() | ||
| } | ||
|
|
||
| const m = await comfyPage.perf.stopMeasuring('dom-widget-clipping') | ||
| recordMeasurement(m) | ||
| console.log(`Clipping: ${m.layouts} forced layouts`) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.