-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
test: stabilize codspeed memory benchmarks #7988
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
base: main
Are you sure you want to change the base?
Changes from all commits
300581b
6fc9f20
ce69001
e7c1d50
d1c9a08
0acf8c4
5ff0128
618a7af
c460937
ea7c024
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { afterEach, beforeEach, bench, describe } from 'vitest' | ||
| import { IsolatedMemoryProcess } from '../shared/isolated-process.ts' | ||
| import { memoryBenchOptions } from './bench-utils.ts' | ||
| import type { IsolatedMemoryBenchmarkKind } from '../shared/isolated-process.ts' | ||
|
|
||
| type RegisterIsolatedClientMemoryBenchmarkOptions = { | ||
| name: string | ||
| setupUrl: URL | ||
| } | ||
|
|
||
| const kind = 'client' satisfies IsolatedMemoryBenchmarkKind | ||
|
|
||
| export function registerIsolatedClientMemoryBenchmark( | ||
| options: RegisterIsolatedClientMemoryBenchmarkOptions, | ||
| ) { | ||
| const isolatedProcess = new IsolatedMemoryProcess({ | ||
| kind, | ||
| setupUrl: options.setupUrl, | ||
| workloadNames: [options.name], | ||
| }) | ||
|
|
||
| const run = async () => { | ||
| try { | ||
| await isolatedProcess.run(0) | ||
| } catch (error) { | ||
| await isolatedProcess.stop().catch(() => {}) | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| describe('memory', () => { | ||
| beforeEach(() => isolatedProcess.start()) | ||
| afterEach(() => isolatedProcess.stop()) | ||
|
|
||
| bench(options.name, run, { | ||
| ...memoryBenchOptions, | ||
| setup: () => isolatedProcess.start(), | ||
| teardown: () => isolatedProcess.stop(), | ||
|
Comment on lines
+31
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: In Vitest 4.1.4, the behavior of suite-level hooks and Tinybench setup/teardown in benchmark files is as follows: Suite-Level Hooks (beforeEach/afterEach) Historically, Vitest's suite-level beforeEach and afterEach hooks did not run for bench tasks [1]. While there have been community efforts and pull requests (such as PR Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Locate files:"
fd -a 'isolated-benchmark\.ts$|isolated.*memory.*\.ts$|memory.*\.ts$' . | sed 's#^\./##' | head -100
echo
echo "Target file outline:"
ast-grep outline benchmarks/memory/client/isolated-benchmark.ts --view compact || true
echo
echo "Target file lines:"
cat -n benchmarks/memory/client/isolated-benchmark.ts | sed -n '1,120p'
echo
echo "Search isolated process definitions/usages:"
rg -n "class IsolatedMemoryProcess|IsolatedMemoryProcess|start\(|stop\(|isolatedProcess|bench\\(" -S benchmarks . --glob '*.ts' --glob '!node_modules' | head -200Repository: TanStack/router Length of output: 25556 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Vitest version declarations:"
for f in package.json pnpm-lock.yaml yarn.lock package-lock.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
if [ "$f" = "package.json" ]; then
node -e "const p=require('./package.json'); console.log({devDependencies:p.devDependencies?.vitest, dependencies:p.dependencies?.vitest, scripts:p.scripts});" 2>/dev/null || sed -n '/vitest/p' "$f"
else
rg -n "vitest(@|:)|version:" "$f" | head -80
fi
fi
done
echo
echo "Shared process start/stop implementation:"
cat -n benchmarks/memory/shared/isolated-process.ts | sed -n '140,235p'
echo
echo "Isolated process tests around duplicate start:"
cat -n benchmarks/memory/server/isolated-process.test.ts | sed -n '1,130p'
echo
echo "Memory bench options:"
cat -n benchmarks/memory/client/bench-utils.ts | sed -n '1,120p'
echo
echo "Server isolated benchmark counterpart:"
cat -n benchmarks/memory/server/isolated-benchmark.ts | sed -n '1,70p'Repository: TanStack/router Length of output: 10199 Use Tinybench setup/teardown for the isolated lifecycle. In Vitest 4 benchmark mode, keep the 🤖 Prompt for AI Agents |
||
| }) | ||
| }) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,6 @@ | ||
| import { afterAll, beforeAll, bench, describe } from 'vitest' | ||
| import { memoryBenchOptions } from '#memory-client/bench-utils' | ||
| import { workload } from './setup' | ||
| import { registerIsolatedClientMemoryBenchmark } from '#memory-client/isolated-benchmark' | ||
|
|
||
| await workload.sanity() | ||
|
|
||
| describe('memory', () => { | ||
| if (workload.before && workload.after) { | ||
| beforeAll(workload.before) | ||
| afterAll(workload.after) | ||
|
|
||
| bench(workload.name, workload.run, { | ||
| ...memoryBenchOptions, | ||
| setup: workload.before, | ||
| teardown: workload.after, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| bench(workload.name, workload.run, memoryBenchOptions) | ||
| registerIsolatedClientMemoryBenchmark({ | ||
| name: 'mem client interrupted-navigations (react)', | ||
| setupUrl: new URL('./setup.ts', import.meta.url), | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,6 @@ | ||
| import { afterAll, beforeAll, bench, describe } from 'vitest' | ||
| import { memoryBenchOptions } from '#memory-client/bench-utils' | ||
| import { workload } from './setup' | ||
| import { registerIsolatedClientMemoryBenchmark } from '#memory-client/isolated-benchmark' | ||
|
|
||
| await workload.sanity() | ||
|
|
||
| describe('memory', () => { | ||
| if (workload.before && workload.after) { | ||
| beforeAll(workload.before) | ||
| afterAll(workload.after) | ||
|
|
||
| bench(workload.name, workload.run, { | ||
| ...memoryBenchOptions, | ||
| setup: workload.before, | ||
| teardown: workload.after, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| bench(workload.name, workload.run, memoryBenchOptions) | ||
| registerIsolatedClientMemoryBenchmark({ | ||
| name: 'mem client interrupted-navigations (solid)', | ||
| setupUrl: new URL('./setup.ts', import.meta.url), | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,6 @@ | ||
| import { afterAll, beforeAll, bench, describe } from 'vitest' | ||
| import { memoryBenchOptions } from '#memory-client/bench-utils' | ||
| import { workload } from './setup' | ||
| import { registerIsolatedClientMemoryBenchmark } from '#memory-client/isolated-benchmark' | ||
|
|
||
| await workload.sanity() | ||
|
|
||
| describe('memory', () => { | ||
| if (workload.before && workload.after) { | ||
| beforeAll(workload.before) | ||
| afterAll(workload.after) | ||
|
|
||
| bench(workload.name, workload.run, { | ||
| ...memoryBenchOptions, | ||
| setup: workload.before, | ||
| teardown: workload.after, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| bench(workload.name, workload.run, memoryBenchOptions) | ||
| registerIsolatedClientMemoryBenchmark({ | ||
| name: 'mem client interrupted-navigations (vue)', | ||
| setupUrl: new URL('./setup.ts', import.meta.url), | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate paired lifecycle hooks before the early return.
Line 11 bypasses the
beforeandafterpairing check whenwarmupis absent. A workload with onlybeforecan then create measured state without cleanup. Validate the pair before returning.Proposed fix
export async function warmClientMemoryWorkload(workload: ClientMemoryWorkload) { - if (!workload.warmup) { - return - } - if (Boolean(workload.before) !== Boolean(workload.after)) { throw new Error( `Client memory workload ${workload.name} must define both before and after when it defines either hook`, ) } + if (!workload.warmup) { + return + } + await workload.before?.()📝 Committable suggestion
🤖 Prompt for AI Agents