From 637fe11fb5a91c6d0bcf2a02a04225e4f438e75c Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 14:09:18 +0900 Subject: [PATCH 1/7] refactor(benchmark): move `sampleCount` and `median` to `BenchmarkResult` --- .../src/node/reporters/benchmark/table/index.ts | 17 ++--------------- .../reporters/benchmark/table/tableRender.ts | 7 ++----- .../vitest/src/runtime/runners/benchmark.ts | 8 ++++++++ packages/vitest/src/runtime/types/benchmark.ts | 2 ++ 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/packages/vitest/src/node/reporters/benchmark/table/index.ts b/packages/vitest/src/node/reporters/benchmark/table/index.ts index 99dae30d86a2..866a5a1d9019 100644 --- a/packages/vitest/src/node/reporters/benchmark/table/index.ts +++ b/packages/vitest/src/node/reporters/benchmark/table/index.ts @@ -167,10 +167,8 @@ interface FormattedBenchmarkGroup { benchmarks: FormattedBenchmarkResult[] } -export type FormattedBenchmarkResult = Omit & { +export type FormattedBenchmarkResult = BenchmarkResult & { id: string - sampleCount: number - median: number } function createFormattedBenchmarkReport(files: File[]) { @@ -183,18 +181,7 @@ function createFormattedBenchmarkReport(files: File[]) { for (const t of task.tasks) { const benchmark = t.meta.benchmark && t.result?.benchmark if (benchmark) { - const { samples, ...rest } = benchmark - benchmarks.push({ - id: t.id, - sampleCount: samples.length, - median: - samples.length % 2 - ? samples[Math.floor(samples.length / 2)] - : (samples[samples.length / 2] - + samples[samples.length / 2 - 1]) - / 2, - ...rest, - }) + benchmarks.push({ id: t.id, ...benchmark, samples: [] }) } } if (benchmarks.length) { diff --git a/packages/vitest/src/node/reporters/benchmark/table/tableRender.ts b/packages/vitest/src/node/reporters/benchmark/table/tableRender.ts index 8c9b21ba0790..483454956874 100644 --- a/packages/vitest/src/node/reporters/benchmark/table/tableRender.ts +++ b/packages/vitest/src/node/reporters/benchmark/table/tableRender.ts @@ -68,7 +68,7 @@ function renderBenchmarkItems(result: BenchmarkResult) { formatNumber(result.p995 || 0), formatNumber(result.p999 || 0), `±${(result.rme || 0).toFixed(2)}%`, - result.samples.length.toString(), + (result.sampleCount || 0).toString(), ] } @@ -124,10 +124,7 @@ export function renderTree( } const baseline = options.compare?.[t.id] if (baseline) { - benchMap[t.id].baseline = { - ...baseline, - samples: Array.from({ length: baseline.sampleCount }), - } + benchMap[t.id].baseline = baseline } } } diff --git a/packages/vitest/src/runtime/runners/benchmark.ts b/packages/vitest/src/runtime/runners/benchmark.ts index cc6edb95c824..cb1bd9d5a399 100644 --- a/packages/vitest/src/runtime/runners/benchmark.ts +++ b/packages/vitest/src/runtime/runners/benchmark.ts @@ -133,6 +133,14 @@ async function runBenchmarkSuite(suite: Suite, runner: NodeBenchmarkRunner) { if (benchmark) { const result = benchmark.result!.benchmark! result.rank = Number(idx) + 1 + + const samples = result.samples + result.sampleCount = samples.length + result.median = samples.length % 2 + ? samples[Math.floor(samples.length / 2)] + : (samples[samples.length / 2] + samples[samples.length / 2 - 1]) / 2 + // TODO: config to clear samples before sending results + // result.samples = [] updateTask(benchmark) } }) diff --git a/packages/vitest/src/runtime/types/benchmark.ts b/packages/vitest/src/runtime/types/benchmark.ts index fb83535b3f56..34535bce7582 100644 --- a/packages/vitest/src/runtime/types/benchmark.ts +++ b/packages/vitest/src/runtime/types/benchmark.ts @@ -18,6 +18,8 @@ export interface Benchmark extends Custom { export interface BenchmarkResult extends TinybenchResult { name: string rank: number + sampleCount: number + median: number } export type BenchFunction = (this: BenchFactory) => Promise | void From 938204b51aab6f984affcdeb0bd91a02f79f60d9 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 14:30:29 +0900 Subject: [PATCH 2/7] fix: clear samples --- packages/vitest/src/runtime/runners/benchmark.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vitest/src/runtime/runners/benchmark.ts b/packages/vitest/src/runtime/runners/benchmark.ts index cb1bd9d5a399..6d11363a80e2 100644 --- a/packages/vitest/src/runtime/runners/benchmark.ts +++ b/packages/vitest/src/runtime/runners/benchmark.ts @@ -139,8 +139,8 @@ async function runBenchmarkSuite(suite: Suite, runner: NodeBenchmarkRunner) { result.median = samples.length % 2 ? samples[Math.floor(samples.length / 2)] : (samples[samples.length / 2] + samples[samples.length / 2 - 1]) / 2 - // TODO: config to clear samples before sending results - // result.samples = [] + // clear samples array before sending a result to reduce memory usage + result.samples = [] updateTask(benchmark) } }) From 4a3dfb975667b191787ac0ee7fc55a095c45914f Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 14:54:19 +0900 Subject: [PATCH 3/7] test: add test --- test/benchmark/test/reporter.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/benchmark/test/reporter.test.ts b/test/benchmark/test/reporter.test.ts index 35ee2e496a91..14c830bc55c6 100644 --- a/test/benchmark/test/reporter.test.ts +++ b/test/benchmark/test/reporter.test.ts @@ -1,4 +1,4 @@ -import { expect, it } from 'vitest' +import { assert, expect, it } from 'vitest' import * as pathe from 'pathe' import { runVitest } from '../../test-utils' @@ -27,3 +27,20 @@ it('non-tty', async () => { ` expect(lines).toMatchObject(expected.trim().split('\n').map(s => expect.stringContaining(s))) }) + +it('no samples in results', async () => { + const root = pathe.join(import.meta.dirname, '../fixtures/reporter') + const result = await runVitest({ root }, ['summary.bench.ts'], 'benchmark') + assert(result.ctx) + const allSamples = [...result.ctx.state.idMap.values()] + .filter(t => t.meta.benchmark) + .map(t => t.result?.benchmark?.samples) + expect(allSamples).toMatchInlineSnapshot(` + [ + [], + [], + [], + [], + ] + `) +}) From 5e39ed669721d0fd5f5d1b3aef3623177731f208 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 16:04:54 +0900 Subject: [PATCH 4/7] feat: add `benchmark.includeSampels` option --- packages/vitest/src/defaults.ts | 1 + .../vitest/src/node/config/serializeConfig.ts | 3 +++ packages/vitest/src/node/types/benchmark.ts | 7 +++++++ packages/vitest/src/runtime/config.ts | 3 +++ .../vitest/src/runtime/runners/benchmark.ts | 5 +++-- test/benchmark/test/reporter.test.ts | 19 +++++++++++-------- 6 files changed, 28 insertions(+), 10 deletions(-) diff --git a/packages/vitest/src/defaults.ts b/packages/vitest/src/defaults.ts index 07bc35a37910..fdb3ecf995c1 100644 --- a/packages/vitest/src/defaults.ts +++ b/packages/vitest/src/defaults.ts @@ -24,6 +24,7 @@ export const benchmarkConfigDefaults: Required< exclude: defaultExclude, includeSource: [], reporters: ['default'], + includeSamples: false, } const defaultCoverageExcludes = [ diff --git a/packages/vitest/src/node/config/serializeConfig.ts b/packages/vitest/src/node/config/serializeConfig.ts index 03e78489c54a..249f4bbdaa1a 100644 --- a/packages/vitest/src/node/config/serializeConfig.ts +++ b/packages/vitest/src/node/config/serializeConfig.ts @@ -160,5 +160,8 @@ export function serializeConfig( standalone: config.standalone, printConsoleTrace: config.printConsoleTrace ?? coreConfig.printConsoleTrace, + benchmark: config.benchmark && { + includeSamples: config.benchmark.includeSamples, + }, } } diff --git a/packages/vitest/src/node/types/benchmark.ts b/packages/vitest/src/node/types/benchmark.ts index e2ee6c53a884..e2575e7ce7ed 100644 --- a/packages/vitest/src/node/types/benchmark.ts +++ b/packages/vitest/src/node/types/benchmark.ts @@ -50,4 +50,11 @@ export interface BenchmarkUserOptions { * benchmark output file */ outputJson?: string + + /** + * Include `samples` array of benchmark results for API or custom reporter usages. + * This is diabled by default to reduce memory usage. + * @default false + */ + includeSamples?: boolean } diff --git a/packages/vitest/src/runtime/config.ts b/packages/vitest/src/runtime/config.ts index 0ecbf4cd1628..14ebe708c67b 100644 --- a/packages/vitest/src/runtime/config.ts +++ b/packages/vitest/src/runtime/config.ts @@ -129,6 +129,9 @@ export interface SerializedConfig { standalone: boolean logHeapUsage: boolean | undefined coverage: SerializedCoverageConfig + benchmark?: { + includeSamples: boolean + } } export interface SerializedCoverageConfig { diff --git a/packages/vitest/src/runtime/runners/benchmark.ts b/packages/vitest/src/runtime/runners/benchmark.ts index 6d11363a80e2..dfa52d6bb5df 100644 --- a/packages/vitest/src/runtime/runners/benchmark.ts +++ b/packages/vitest/src/runtime/runners/benchmark.ts @@ -139,8 +139,9 @@ async function runBenchmarkSuite(suite: Suite, runner: NodeBenchmarkRunner) { result.median = samples.length % 2 ? samples[Math.floor(samples.length / 2)] : (samples[samples.length / 2] + samples[samples.length / 2 - 1]) / 2 - // clear samples array before sending a result to reduce memory usage - result.samples = [] + if (!runner.config.benchmark?.includeSamples) { + result.samples = [] + } updateTask(benchmark) } }) diff --git a/test/benchmark/test/reporter.test.ts b/test/benchmark/test/reporter.test.ts index 14c830bc55c6..c163421d0014 100644 --- a/test/benchmark/test/reporter.test.ts +++ b/test/benchmark/test/reporter.test.ts @@ -35,12 +35,15 @@ it('no samples in results', async () => { const allSamples = [...result.ctx.state.idMap.values()] .filter(t => t.meta.benchmark) .map(t => t.result?.benchmark?.samples) - expect(allSamples).toMatchInlineSnapshot(` - [ - [], - [], - [], - [], - ] - `) + expect(allSamples[0]).toEqual([]) +}) + +it('includeSamples', async () => { + const root = pathe.join(import.meta.dirname, '../fixtures/reporter') + const result = await runVitest({ root, benchmark: { includeSamples: true } }, ['summary.bench.ts'], 'benchmark') + assert(result.ctx) + const allSamples = [...result.ctx.state.idMap.values()] + .filter(t => t.meta.benchmark) + .map(t => t.result?.benchmark?.samples) + expect(allSamples[0]).not.toEqual([]) }) From f1aa090b776bcc557158cd4ad146038c6c5d747d Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 16:27:26 +0900 Subject: [PATCH 5/7] test: tweak --- test/benchmark/test/reporter.test.ts | 29 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/benchmark/test/reporter.test.ts b/test/benchmark/test/reporter.test.ts index c163421d0014..0e3ddebe2dd1 100644 --- a/test/benchmark/test/reporter.test.ts +++ b/test/benchmark/test/reporter.test.ts @@ -28,22 +28,23 @@ it('non-tty', async () => { expect(lines).toMatchObject(expected.trim().split('\n').map(s => expect.stringContaining(s))) }) -it('no samples in results', async () => { - const root = pathe.join(import.meta.dirname, '../fixtures/reporter') - const result = await runVitest({ root }, ['summary.bench.ts'], 'benchmark') - assert(result.ctx) - const allSamples = [...result.ctx.state.idMap.values()] - .filter(t => t.meta.benchmark) - .map(t => t.result?.benchmark?.samples) - expect(allSamples[0]).toEqual([]) -}) - -it('includeSamples', async () => { - const root = pathe.join(import.meta.dirname, '../fixtures/reporter') - const result = await runVitest({ root, benchmark: { includeSamples: true } }, ['summary.bench.ts'], 'benchmark') +it.for([true, false])('includeSamples %s', async (includeSamples) => { + const result = await runVitest( + { + root: pathe.join(import.meta.dirname, '../fixtures/reporter'), + benchmark: { includeSamples }, + }, + ['summary.bench.ts'], + 'benchmark', + ) assert(result.ctx) const allSamples = [...result.ctx.state.idMap.values()] .filter(t => t.meta.benchmark) .map(t => t.result?.benchmark?.samples) - expect(allSamples[0]).not.toEqual([]) + if (includeSamples) { + expect(allSamples[0]).not.toEqual([]) + } + else { + expect(allSamples[0]).toEqual([]) + } }) From 65dc21d82e51826f19a71d84711fdf9f42904b28 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 16:38:32 +0900 Subject: [PATCH 6/7] chore: typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ari Perkkiö --- packages/vitest/src/node/types/benchmark.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vitest/src/node/types/benchmark.ts b/packages/vitest/src/node/types/benchmark.ts index e2575e7ce7ed..46796b8f4657 100644 --- a/packages/vitest/src/node/types/benchmark.ts +++ b/packages/vitest/src/node/types/benchmark.ts @@ -53,7 +53,7 @@ export interface BenchmarkUserOptions { /** * Include `samples` array of benchmark results for API or custom reporter usages. - * This is diabled by default to reduce memory usage. + * This is disabled by default to reduce memory usage. * @default false */ includeSamples?: boolean From 97ec6fe3187557780583294f9afb0dc6020c7d69 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 21 Sep 2024 17:05:20 +0900 Subject: [PATCH 7/7] fix: free samples earlier --- .../vitest/src/runtime/runners/benchmark.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/vitest/src/runtime/runners/benchmark.ts b/packages/vitest/src/runtime/runners/benchmark.ts index dfa52d6bb5df..9dad217ed277 100644 --- a/packages/vitest/src/runtime/runners/benchmark.ts +++ b/packages/vitest/src/runtime/runners/benchmark.ts @@ -72,6 +72,15 @@ async function runBenchmarkSuite(suite: Suite, runner: NodeBenchmarkRunner) { const taskRes = task.result! const result = benchmark.result!.benchmark! Object.assign(result, taskRes) + // compute extra stats and free raw samples as early as possible + const samples = result.samples + result.sampleCount = samples.length + result.median = samples.length % 2 + ? samples[Math.floor(samples.length / 2)] + : (samples[samples.length / 2] + samples[samples.length / 2 - 1]) / 2 + if (!runner.config.benchmark?.includeSamples) { + result.samples.length = 0 + } updateTask(benchmark) }, { @@ -133,15 +142,6 @@ async function runBenchmarkSuite(suite: Suite, runner: NodeBenchmarkRunner) { if (benchmark) { const result = benchmark.result!.benchmark! result.rank = Number(idx) + 1 - - const samples = result.samples - result.sampleCount = samples.length - result.median = samples.length % 2 - ? samples[Math.floor(samples.length / 2)] - : (samples[samples.length / 2] + samples[samples.length / 2 - 1]) / 2 - if (!runner.config.benchmark?.includeSamples) { - result.samples = [] - } updateTask(benchmark) } })