diff --git a/.github/workflows/ci-performance.yml b/.github/workflows/ci-performance.yml index 47ba3e0f14..90f6abbd44 100644 --- a/.github/workflows/ci-performance.yml +++ b/.github/workflows/ci-performance.yml @@ -22,33 +22,30 @@ jobs: performance-check: name: Benchmarks runs-on: ubuntu-latest - timeout-minutes: 30 + # Allows flagged benchmarks to be measured again, see benchmark/compare.js + timeout-minutes: 45 steps: - - name: Checkout PR branch (for benchmark script) + - name: Checkout PR branch uses: actions/checkout@v4 with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 1 - - name: Save PR benchmark script - run: | - mkdir -p /tmp/pr-benchmark - cp -r benchmark /tmp/pr-benchmark/ || echo "No benchmark directory" - cp package.json /tmp/pr-benchmark/ || true - + # The base branch is kept in a separate directory, so that both builds are available to + # measure flagged benchmarks again - name: Checkout base branch uses: actions/checkout@v4 with: ref: ${{ github.base_ref }} fetch-depth: 1 - clean: true + path: base - - name: Restore PR benchmark script + - name: Use PR benchmark script for base branch run: | - if [ -d "/tmp/pr-benchmark/benchmark" ]; then - rm -rf benchmark - cp -r /tmp/pr-benchmark/benchmark . + if [ -d benchmark ]; then + rm -rf base/benchmark + cp -r benchmark base/ fi - name: Setup Node.js @@ -56,15 +53,27 @@ jobs: with: node-version: ${{ env.NODE_VERSION }} cache: 'npm' + cache-dependency-path: | + package-lock.json + base/package-lock.json - name: Install dependencies (base) + working-directory: base run: npm ci - name: Build Parse Server (base) + working-directory: base + run: npm run build + + - name: Install dependencies (PR) + run: npm ci + + - name: Build Parse Server (PR) run: npm run build - name: Run baseline benchmarks id: baseline + working-directory: base env: NODE_ENV: production run: | @@ -73,54 +82,24 @@ jobs: if [ ! -f "benchmark/performance.js" ]; then echo "⚠️ Benchmark script not found - this is expected for new features" echo "Skipping baseline benchmark" - echo '[]' > baseline.json - echo "Baseline: N/A (no benchmark script)" > baseline-output.txt + echo '[]' > ../baseline.json + echo "Baseline: N/A (no benchmark script)" > ../baseline-output.txt exit 0 fi - taskset -c 0 npm run benchmark 2>&1 | tee baseline-output.txt || npm run benchmark 2>&1 | tee baseline-output.txt || true + taskset -c 0 npm run benchmark 2>&1 | tee ../baseline-output.txt || npm run benchmark 2>&1 | tee ../baseline-output.txt || true # Extract JSON from output (everything between first [ and last ]) - sed -n '/^\[/,/^\]/p' baseline-output.txt > baseline.json || echo '[]' > baseline.json + sed -n '/^\[/,/^\]/p' ../baseline-output.txt > ../baseline.json || echo '[]' > ../baseline.json continue-on-error: true - - name: Save baseline results to temp location - run: | - mkdir -p /tmp/benchmark-results - cp baseline.json /tmp/benchmark-results/ || echo '[]' > /tmp/benchmark-results/baseline.json - cp baseline-output.txt /tmp/benchmark-results/ || echo 'No baseline output' > /tmp/benchmark-results/baseline-output.txt - - name: Upload baseline results uses: actions/upload-artifact@v4 with: name: baseline-benchmark path: | - /tmp/benchmark-results/baseline.json - /tmp/benchmark-results/baseline-output.txt + baseline.json + baseline-output.txt retention-days: 7 - - name: Checkout PR branch - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.head.sha }} - fetch-depth: 1 - clean: true - - - name: Restore baseline results - run: | - cp /tmp/benchmark-results/baseline.json ./ || echo '[]' > baseline.json - cp /tmp/benchmark-results/baseline-output.txt ./ || echo 'No baseline output' > baseline-output.txt - - - name: Setup Node.js (PR) - uses: actions/setup-node@v4 - with: - node-version: ${{ env.NODE_VERSION }} - cache: 'npm' - - - name: Install dependencies (PR) - run: npm ci - - - name: Build Parse Server (PR) - run: npm run build - - name: Run PR benchmarks id: pr-bench env: @@ -158,93 +137,27 @@ jobs: - name: Compare benchmark results id: compare + env: + NODE_ENV: production + BENCHMARK_BASE_DIR: base + BENCHMARK_COMMAND: taskset -c 0 npm run benchmark run: | set -o pipefail - node -e " - const fs = require('fs'); - - let baseline, pr; - try { - baseline = JSON.parse(fs.readFileSync('baseline.json', 'utf8')); - pr = JSON.parse(fs.readFileSync('pr.json', 'utf8')); - } catch (e) { - console.log('⚠️ Could not parse benchmark results'); - process.exit(0); - } - - // Handle case where baseline doesn't exist (new feature) - if (!Array.isArray(baseline) || baseline.length === 0) { - if (!Array.isArray(pr) || pr.length === 0) { - console.log('⚠️ Benchmark results are empty or invalid'); - process.exit(0); - } - console.log('# Performance Benchmark Results\n'); - console.log('> ℹ️ Baseline not available - this appears to be a new feature\n'); - console.log('| Benchmark | Value | Details |'); - console.log('|-----------|-------|---------|'); - pr.forEach(result => { - console.log(\`| \${result.name} | \${result.value.toFixed(2)} ms | \${result.extra} |\`); - }); - console.log(''); - console.log('✅ **New benchmarks established for this feature.**'); - process.exit(0); - } - - if (!Array.isArray(pr) || pr.length === 0) { - console.log('⚠️ PR benchmark results are empty or invalid'); - process.exit(0); - } - - console.log('# Performance Comparison\n'); - console.log('| Benchmark | Baseline | PR | Change | Status |'); - console.log('|-----------|----------|----|---------| ------ |'); - - let hasRegression = false; - let hasImprovement = false; - - baseline.forEach(baseResult => { - const prResult = pr.find(p => p.name === baseResult.name); - if (!prResult) { - console.log(\`| \${baseResult.name} | \${baseResult.value.toFixed(2)} ms | N/A | - | ⚠️ Missing |\`); - return; - } - - const baseValue = parseFloat(baseResult.value); - const prValue = parseFloat(prResult.value); - const change = ((prValue - baseValue) / baseValue * 100); - const changeStr = change > 0 ? \`+\${change.toFixed(1)}%\` : \`\${change.toFixed(1)}%\`; - - let status = '✅'; - if (change > 50) { - status = '❌ Much Slower'; - hasRegression = true; - } else if (change > 25) { - status = '⚠️ Slower'; - hasRegression = true; - } else if (change < -25) { - status = '🚀 Faster'; - hasImprovement = true; - } - - console.log(\`| \${baseResult.name} | \${baseValue.toFixed(2)} ms | \${prValue.toFixed(2)} ms | \${changeStr} | \${status} |\`); - }); - - console.log(''); - if (hasRegression) { - console.log('⚠️ **Performance regressions detected.** Please review the changes.'); - process.exitCode = 1; - } else if (hasImprovement) { - console.log('🚀 **Performance improvements detected!** Great work!'); - } else { - console.log('✅ **No significant performance changes.**'); - } - " | tee comparison.md + # A PR branch that predates the comparison script has to be updated to be compared + if [ ! -f benchmark/compare.js ]; then + echo "⚠️ Benchmarks cannot be compared because this branch does not contain \`benchmark/compare.js\`. Update the branch with the base branch." | tee comparison.md + exit 1 + fi + node benchmark/compare.js baseline.json pr.json | tee comparison.md - name: Upload comparison + if: always() uses: actions/upload-artifact@v4 with: name: benchmark-comparison - path: comparison.md + path: | + comparison.md + retest-output.txt retention-days: 30 - name: Generate job summary diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 41a2f87de0..ee0ac482f0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -306,6 +306,8 @@ Note that the script above will ONLY be executed during initialization of the co Parse Server includes an automated performance benchmarking system that runs on every pull request to detect performance regressions and track improvements over time. +A benchmark is flagged as a regression if it is more than 25% slower than on the base branch. To rule out noise of the CI runner, a flagged benchmark is measured again 3 times for the base branch and 3 times for the pull request, alternating between them. The check only fails if the regression persists in the medians of these measurements, or if any of these measurements fails. + #### Adding Tests You should consider adding performance benchmarks if your contribution: @@ -342,6 +344,7 @@ Performance benchmarks are located in [`benchmark/performance.js`](benchmark/per ```bash npm run benchmark:quick # Quick test with 10 iterations npm run benchmark # Full test with 10,000 iterations + BENCHMARK_NAMES='["Feature Name"]' npm run benchmark # Run only the listed benchmarks ``` For new features where no baseline exists, the CI will establish new benchmarks that future PRs will be compared against. diff --git a/benchmark/compare.js b/benchmark/compare.js new file mode 100644 index 0000000000..d68b74ed2c --- /dev/null +++ b/benchmark/compare.js @@ -0,0 +1,308 @@ +/** + * Benchmark Comparison + * + * Compares the benchmark results of a pull request with those of its base branch. A benchmark + * that is flagged as a regression is measured again several times for both branches, alternating + * between them, so that a slowdown of the CI runner while the pull request was measured is not + * mistaken for a regression. The comparison only fails if the regression persists in the retest. + * + * Run with: node benchmark/compare.js [baseline.json] [pr.json] + * + * Environment variables: + * - BENCHMARK_BASE_DIR: Directory of the base branch build; flagged benchmarks are only measured + * again if this is set. + * - BENCHMARK_PR_DIR: Directory of the pull request build; defaults to the current directory. + * - BENCHMARK_COMMAND: Command that runs the benchmarks; defaults to `npm run benchmark`. + */ + +const fs = require('fs'); +const { spawnSync } = require('child_process'); + +// Change in percent above which a benchmark is flagged +const SLOWER_THRESHOLD = 25; +const MUCH_SLOWER_THRESHOLD = 50; +const FASTER_THRESHOLD = -25; + +// Number of times a flagged benchmark is measured again for each branch; each measurement runs a +// fraction of the iterations so that the retest takes about as long as the initial measurement +const RETEST_ROUNDS = 3; + +/** + * Calculate the median of values. + * @param {number[]} values The values. + * @returns {number} The median. + */ +function median(values) { + const sorted = [...values].sort((a, b) => a - b); + const middle = Math.floor(sorted.length / 2); + return sorted.length % 2 ? sorted[middle] : (sorted[middle - 1] + sorted[middle]) / 2; +} + +/** + * Compare benchmark results of the base branch with those of the pull request. + * @param {Object[]} baseline The results of the base branch. + * @param {Object[]} pr The results of the pull request. + * @returns {{ rows: Object[], regressions: string[], hasImprovement: boolean }} The comparison + * per benchmark, the names of benchmarks flagged as regression and whether any benchmark improved. + */ +function compareResults(baseline, pr) { + const rows = baseline.map(baseResult => { + const baseValue = parseFloat(baseResult.value); + const prResult = pr.find(p => p.name === baseResult.name); + if (!prResult) { + return { name: baseResult.name, baseValue, status: '⚠️ Missing' }; + } + const prValue = parseFloat(prResult.value); + const change = ((prValue - baseValue) / baseValue) * 100; + const row = { name: baseResult.name, baseValue, prValue, change, status: '✅' }; + if (change > MUCH_SLOWER_THRESHOLD) { + row.status = '❌ Much Slower'; + row.isRegression = true; + } else if (change > SLOWER_THRESHOLD) { + row.status = '⚠️ Slower'; + row.isRegression = true; + } else if (change < FASTER_THRESHOLD) { + row.status = '🚀 Faster'; + row.isImprovement = true; + } + return row; + }); + return { + rows, + regressions: rows.filter(row => row.isRegression).map(row => row.name), + hasImprovement: rows.some(row => row.isImprovement), + }; +} + +/** + * Render compared benchmark results as a markdown table. + * @param {Object[]} rows The rows returned by `compareResults`. + * @returns {string[]} The table lines. + */ +function formatTable(rows) { + const formatValue = value => (value === undefined ? 'N/A' : `${value.toFixed(2)} ms`); + const formatChange = change => { + if (change === undefined) { + return '-'; + } + return change > 0 ? `+${change.toFixed(1)}%` : `${change.toFixed(1)}%`; + }; + return [ + '| Benchmark | Baseline | PR | Change | Status |', + '|-----------|----------|----|---------| ------ |', + ...rows.map( + row => + `| ${row.name} | ${formatValue(row.baseValue)} | ${formatValue(row.prValue)} | ${formatChange(row.change)} | ${row.status} |` + ), + ]; +} + +/** + * Extract the results from the output of a benchmark run, which logs the results as a JSON array. + * @param {string} output The output of the benchmark run. + * @returns {Object[]|null} The results, or null if the output contains no results. + */ +function extractResults(output) { + const lines = output.split('\n'); + const start = lines.findIndex(line => line.startsWith('[')); + if (start === -1) { + return null; + } + for (let end = start; end < lines.length; end++) { + if (end !== start && !lines[end].startsWith(']')) { + continue; + } + try { + const results = JSON.parse(lines.slice(start, end + 1).join('\n')); + if (Array.isArray(results)) { + return results; + } + } catch { + // Not the end of the results yet + } + } + return null; +} + +/** + * Measure flagged benchmarks again, alternating between base branch and pull request. + * @param {Object} options The options. + * @param {string[]} options.names The names of the benchmarks to measure again. + * @param {number} options.rounds The number of measurements per branch. + * @param {Function} options.runBenchmark Runs benchmarks for a side (`base` or `pr`) and returns + * the results, or null if the run failed. + * @param {Function} options.log Logs progress. + * @returns {Object[]} The comparison of the medians of the measurements per benchmark. + */ +function retest({ names, rounds, runBenchmark, log }) { + const values = { base: {}, pr: {} }; + for (let round = 0; round < rounds; round++) { + // Alternate the order in each round so that a gradual change of the runner speed affects both + // branches equally + const sides = round % 2 === 0 ? ['base', 'pr'] : ['pr', 'base']; + for (const side of sides) { + log(`Retest ${round + 1}/${rounds}: measuring ${side}...`); + for (const result of runBenchmark(side, names) || []) { + if (names.includes(result.name)) { + values[side][result.name] = values[side][result.name] || []; + values[side][result.name].push(parseFloat(result.value)); + } + } + } + } + return names.map(name => { + // Only clear a regression if every measurement of both branches succeeded + const isComplete = side => (values[side][name] || []).length >= rounds; + if (!isComplete('base') || !isComplete('pr')) { + return { name, status: '⚠️ Retest failed', isRegression: true }; + } + const baseline = [{ name, value: median(values.base[name]) }]; + const pr = [{ name, value: median(values.pr[name]) }]; + return compareResults(baseline, pr).rows[0]; + }); +} + +/** + * Evaluate benchmark results and print the comparison as markdown. + * @param {Object} options The options. + * @param {Object[]} options.baseline The results of the base branch. + * @param {Object[]} options.pr The results of the pull request. + * @param {Function} options.print Prints a line of the comparison. + * @param {Function} [options.runBenchmark] Runs benchmarks for a side (`base` or `pr`) and returns + * the results; if not set, flagged benchmarks are not measured again. + * @param {number} [options.rounds] The number of measurements per branch in a retest. + * @param {Function} [options.log] Logs progress. + * @returns {number} The exit code; 1 if a regression was detected, otherwise 0. + */ +function evaluate({ baseline, pr, print, runBenchmark, rounds = RETEST_ROUNDS, log = () => {} }) { + // Handle case where baseline doesn't exist (new feature) + if (!Array.isArray(baseline) || baseline.length === 0) { + if (!Array.isArray(pr) || pr.length === 0) { + print('⚠️ Benchmark results are empty or invalid'); + return 0; + } + print('# Performance Benchmark Results'); + print(''); + print('> ℹ️ Baseline not available - this appears to be a new feature'); + print(''); + print('| Benchmark | Value | Details |'); + print('|-----------|-------|---------|'); + pr.forEach(result => { + print(`| ${result.name} | ${result.value.toFixed(2)} ms | ${result.extra} |`); + }); + print(''); + print('✅ **New benchmarks established for this feature.**'); + return 0; + } + + if (!Array.isArray(pr) || pr.length === 0) { + print('⚠️ PR benchmark results are empty or invalid'); + return 0; + } + + const { rows, regressions, hasImprovement } = compareResults(baseline, pr); + print('# Performance Comparison'); + print(''); + formatTable(rows).forEach(line => print(line)); + print(''); + + let confirmedRegressions = regressions; + if (regressions.length > 0 && runBenchmark) { + print('## Retest'); + print(''); + print( + `> Flagged benchmarks were measured again ${rounds} times for the base branch and for the PR, alternating between them. The values are the medians of these measurements.` + ); + print(''); + const retestRows = retest({ names: regressions, rounds, runBenchmark, log }); + formatTable(retestRows).forEach(line => print(line)); + print(''); + confirmedRegressions = retestRows.filter(row => row.isRegression).map(row => row.name); + } + + if (confirmedRegressions.length > 0) { + print('⚠️ **Performance regressions detected.** Please review the changes.'); + return 1; + } + if (hasImprovement) { + print('🚀 **Performance improvements detected!** Great work!'); + } else { + print('✅ **No significant performance changes.**'); + } + return 0; +} + +/** + * Create a function that runs benchmarks in the build directory of the base branch or the pull + * request. + * @param {Object} options The options. + * @param {{ base: string, pr: string }} options.dirs The build directories. + * @param {string} options.command The command that runs the benchmarks. + * @param {number} options.iterationsFactor The factor applied to the iterations of each benchmark. + * @param {string} options.logFile The file the output of each run is appended to. + * @returns {Function} Runs the benchmarks with the given names for a side (`base` or `pr`) and + * returns the results, or null if the run produced no results. + */ +function createBenchmarkRunner({ dirs, command, iterationsFactor, logFile }) { + return (side, names) => { + const run = spawnSync(command, { + cwd: dirs[side], + shell: true, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + env: { + ...process.env, + BENCHMARK_NAMES: JSON.stringify(names), + BENCHMARK_ITERATIONS_FACTOR: String(iterationsFactor), + }, + }); + const stdout = run.stdout || ''; + fs.appendFileSync(logFile, `=== ${side} (exit code ${run.status}) ===\n${stdout}${run.stderr || ''}\n`); + return extractResults(stdout); + }; +} + +function main() { + const [baselinePath = 'baseline.json', prPath = 'pr.json'] = process.argv.slice(2); + const print = line => process.stdout.write(`${line}\n`); + + let baseline, pr; + try { + baseline = JSON.parse(fs.readFileSync(baselinePath, 'utf8')); + pr = JSON.parse(fs.readFileSync(prPath, 'utf8')); + } catch { + print('⚠️ Could not parse benchmark results'); + return; + } + + const baseDir = process.env.BENCHMARK_BASE_DIR; + const runBenchmark = baseDir + ? createBenchmarkRunner({ + dirs: { base: baseDir, pr: process.env.BENCHMARK_PR_DIR || '.' }, + command: process.env.BENCHMARK_COMMAND || 'npm run benchmark', + iterationsFactor: 1 / RETEST_ROUNDS, + logFile: 'retest-output.txt', + }) + : undefined; + + process.exitCode = evaluate({ + baseline, + pr, + print, + runBenchmark, + log: message => process.stderr.write(`${message}\n`), + }); +} + +if (require.main === module) { + main(); +} + +module.exports = { + compareResults, + createBenchmarkRunner, + evaluate, + extractResults, + formatTable, + median, +}; diff --git a/benchmark/performance.js b/benchmark/performance.js index 84221e0cbd..6b19bd139a 100644 --- a/benchmark/performance.js +++ b/benchmark/performance.js @@ -19,6 +19,10 @@ const SERVER_URL = 'http://localhost:1337/parse'; const APP_ID = 'benchmark-app-id'; const MASTER_KEY = 'benchmark-master-key'; const ITERATIONS = process.env.BENCHMARK_ITERATIONS ? parseInt(process.env.BENCHMARK_ITERATIONS, 10) : undefined; +// Factor applied to the iterations of each benchmark, e.g. to measure a benchmark several times +const ITERATIONS_FACTOR = process.env.BENCHMARK_ITERATIONS_FACTOR ? parseFloat(process.env.BENCHMARK_ITERATIONS_FACTOR) : 1; +// JSON array of benchmark names to run; runs all benchmarks if not set +const NAMES = process.env.BENCHMARK_NAMES ? JSON.parse(process.env.BENCHMARK_NAMES) : undefined; const LOG_ITERATIONS = false; // Parse Server instance @@ -115,8 +119,8 @@ function resetParseServer() { * this benchmark. */ async function measureOperation({ name, operation, iterations, skipWarmup = false, dbLatency }) { - // Override iterations if global ITERATIONS is set - iterations = ITERATIONS || iterations; + // Use global ITERATIONS if set, otherwise apply ITERATIONS_FACTOR + iterations = ITERATIONS || Math.ceil(iterations * ITERATIONS_FACTOR); // Determine warmup count (20% of iterations) const warmupCount = skipWarmup ? 0 : Math.floor(iterations * 0.2); @@ -214,8 +218,8 @@ async function measureOperation({ name, operation, iterations, skipWarmup = fals async function measureMemoryOperation({ name, operation, iterations, skipWarmup = false }) { const { PerformanceObserver } = require('node:perf_hooks'); - // Override iterations if global ITERATIONS is set - iterations = ITERATIONS || iterations; + // Use global ITERATIONS if set, otherwise apply ITERATIONS_FACTOR + iterations = ITERATIONS || Math.ceil(iterations * ITERATIONS_FACTOR); // Determine warmup count (20% of iterations) const warmupCount = skipWarmup ? 0 : Math.floor(iterations * 0.2); @@ -901,12 +905,13 @@ async function runBenchmarks() { { name: 'Query $regex', fn: benchmarkQueryRegex }, { name: 'LiveQuery $regex', fn: benchmarkLiveQueryRegex }, ]; + const selectedBenchmarks = NAMES ? benchmarks.filter(benchmark => NAMES.includes(benchmark.name)) : benchmarks; // Run each benchmark with database cleanup const suiteStart = performance.now(); - for (let idx = 0; idx < benchmarks.length; idx++) { - const benchmark = benchmarks[idx]; - const label = `[${idx + 1}/${benchmarks.length}] ${benchmark.name}`; + for (let idx = 0; idx < selectedBenchmarks.length; idx++) { + const benchmark = selectedBenchmarks[idx]; + const label = `[${idx + 1}/${selectedBenchmarks.length}] ${benchmark.name}`; logGroup(label); try { logInfo('Resetting database...'); diff --git a/spec/BenchmarkCompare.spec.js b/spec/BenchmarkCompare.spec.js new file mode 100644 index 0000000000..f18efc0c42 --- /dev/null +++ b/spec/BenchmarkCompare.spec.js @@ -0,0 +1,356 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const { + compareResults, + createBenchmarkRunner, + evaluate, + extractResults, + formatTable, + median, +} = require('../benchmark/compare'); + +const result = (name, value) => ({ name, value, unit: 'ms', range: '-', extra: 'p95: -' }); + +describe('Benchmark comparison', () => { + describe('compareResults', () => { + it('flags a benchmark that is more than 25% slower as a regression', () => { + const { rows, regressions } = compareResults([result('A', 1)], [result('A', 1.3)]); + expect(rows[0].status).toBe('⚠️ Slower'); + expect(regressions).toEqual(['A']); + }); + + it('flags a benchmark that is more than 50% slower as much slower', () => { + const { rows, regressions } = compareResults([result('A', 1)], [result('A', 1.6)]); + expect(rows[0].status).toBe('❌ Much Slower'); + expect(regressions).toEqual(['A']); + }); + + it('does not flag a benchmark that is exactly 25% slower', () => { + const { rows, regressions } = compareResults([result('A', 100)], [result('A', 125)]); + expect(rows[0].status).toBe('✅'); + expect(regressions).toEqual([]); + }); + + it('reports a benchmark that is more than 25% faster as an improvement', () => { + const { rows, regressions, hasImprovement } = compareResults([result('A', 1)], [result('A', 0.7)]); + expect(rows[0].status).toBe('🚀 Faster'); + expect(regressions).toEqual([]); + expect(hasImprovement).toBe(true); + }); + + it('reports a benchmark that is missing in the PR results without flagging it', () => { + const { rows, regressions } = compareResults([result('A', 1), result('B', 2)], [result('A', 1)]); + expect(rows[1].status).toBe('⚠️ Missing'); + expect(regressions).toEqual([]); + }); + }); + + describe('formatTable', () => { + it('renders the comparison as a markdown table', () => { + const { rows } = compareResults([result('A', 1), result('B', 2)], [result('A', 1.3)]); + expect(formatTable(rows)).toEqual([ + '| Benchmark | Baseline | PR | Change | Status |', + '|-----------|----------|----|---------| ------ |', + '| A | 1.00 ms | 1.30 ms | +30.0% | ⚠️ Slower |', + '| B | 2.00 ms | N/A | - | ⚠️ Missing |', + ]); + }); + }); + + describe('extractResults', () => { + it('extracts the results from the benchmark output', () => { + const results = [result('A', 1.23)]; + const output = [ + '> parse-server@9.0.0 benchmark:only', + '::group::[1/1] A', + 'Result: 1.23 ms (p95: -)', + '::endgroup::', + JSON.stringify(results, null, 2), + '::group::Summary', + 'Total duration: 1.0s', + ].join('\n'); + expect(extractResults(output)).toEqual(results); + }); + + it('returns null if the output contains no results', () => { + expect(extractResults('Error running benchmarks')).toBeNull(); + }); + }); + + describe('median', () => { + it('returns the middle value of an odd number of values', () => { + expect(median([3, 1, 2])).toBe(2); + }); + + it('returns the mean of the two middle values of an even number of values', () => { + expect(median([4, 1, 3, 2])).toBe(2.5); + }); + }); + + describe('evaluate', () => { + const run = options => { + const lines = []; + const exitCode = evaluate({ print: line => lines.push(line), ...options }); + return { exitCode, output: lines.join('\n') }; + }; + + // Returns a benchmark runner that yields the given values per side, one per call + const runnerWith = values => { + const calls = []; + const runBenchmark = (side, names) => { + calls.push([side, names]); + const value = values[side].shift(); + return value === null ? null : names.map(name => result(name, value)); + }; + return { calls, runBenchmark }; + }; + + it('passes without retest if no benchmark is flagged', () => { + const runner = runnerWith({ base: [], pr: [] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.1)], + runBenchmark: runner.runBenchmark, + }); + expect(exitCode).toBe(0); + expect(runner.calls).toEqual([]); + expect(output).toContain('✅ **No significant performance changes.**'); + }); + + it('retests only the flagged benchmarks, alternating between base and PR', () => { + const runner = runnerWith({ base: [1, 1, 1], pr: [1, 1, 1] }); + run({ + baseline: [result('A', 1), result('B', 1)], + pr: [result('A', 1.3), result('B', 1)], + runBenchmark: runner.runBenchmark, + rounds: 3, + }); + expect(runner.calls).toEqual([ + ['base', ['A']], + ['pr', ['A']], + ['pr', ['A']], + ['base', ['A']], + ['base', ['A']], + ['pr', ['A']], + ]); + }); + + it('passes if the regression does not persist in the retest', () => { + const runner = runnerWith({ base: [1, 1, 1], pr: [1.02, 0.99, 1.01] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: runner.runBenchmark, + rounds: 3, + }); + expect(exitCode).toBe(0); + expect(output).toContain('| A | 1.00 ms | 1.01 ms | +1.0% | ✅ |'); + expect(output).toContain('✅ **No significant performance changes.**'); + expect(output).not.toContain('Performance regressions detected'); + }); + + it('fails if the regression persists in the retest', () => { + const runner = runnerWith({ base: [1, 1, 1], pr: [1.3, 1.3, 1.3] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: runner.runBenchmark, + rounds: 3, + }); + expect(exitCode).toBe(1); + expect(output).toContain('⚠️ **Performance regressions detected.** Please review the changes.'); + }); + + it('uses the median of the retest measurements', () => { + const outlier = runnerWith({ base: [1, 1, 1], pr: [1, 5, 1] }); + expect(run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: outlier.runBenchmark, + rounds: 3, + }).exitCode).toBe(0); + + const persistent = runnerWith({ base: [1, 1, 1], pr: [1.3, 1, 1.3] }); + expect(run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: persistent.runBenchmark, + rounds: 3, + }).exitCode).toBe(1); + }); + + it('fails if the retest yields no results for a flagged benchmark', () => { + const runner = runnerWith({ base: [null, null, null], pr: [1, 1, 1] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: runner.runBenchmark, + rounds: 3, + }); + expect(exitCode).toBe(1); + expect(output).toContain('| A | N/A | N/A | - | ⚠️ Retest failed |'); + }); + + it('fails if any retest measurement yields no results for a flagged benchmark', () => { + const runner = runnerWith({ base: [null, 1, 1], pr: [1, 1, 1] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + runBenchmark: runner.runBenchmark, + rounds: 3, + }); + expect(exitCode).toBe(1); + expect(output).toContain('| A | N/A | N/A | - | ⚠️ Retest failed |'); + }); + + it('fails without retest if flagged benchmarks cannot be measured again', () => { + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 1.3)], + }); + expect(exitCode).toBe(1); + expect(output).toContain('⚠️ **Performance regressions detected.** Please review the changes.'); + }); + + it('reports improvements without retest', () => { + const runner = runnerWith({ base: [], pr: [] }); + const { exitCode, output } = run({ + baseline: [result('A', 1)], + pr: [result('A', 0.7)], + runBenchmark: runner.runBenchmark, + }); + expect(exitCode).toBe(0); + expect(runner.calls).toEqual([]); + expect(output).toContain('🚀 **Performance improvements detected!** Great work!'); + }); + + it('establishes new benchmarks if there is no baseline', () => { + const { exitCode, output } = run({ baseline: [], pr: [result('A', 1)] }); + expect(exitCode).toBe(0); + expect(output).toContain('✅ **New benchmarks established for this feature.**'); + }); + + it('passes with a warning if the PR results are empty', () => { + const { exitCode, output } = run({ baseline: [result('A', 1)], pr: [] }); + expect(exitCode).toBe(0); + expect(output).toBe('⚠️ PR benchmark results are empty or invalid'); + }); + }); + + describe('createBenchmarkRunner', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'benchmark-compare-')); + fs.mkdirSync(path.join(tmpDir, 'base')); + fs.mkdirSync(path.join(tmpDir, 'pr')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('runs the benchmark command for the selected benchmarks in the directory of the side', () => { + // Prints the directory name, the requested benchmark names and the iterations factor as results + const script = [ + 'const names = JSON.parse(process.env.BENCHMARK_NAMES);', + 'const dir = require("path").basename(process.cwd());', + 'const factor = process.env.BENCHMARK_ITERATIONS_FACTOR;', + 'console.log("log line");', + 'console.log(JSON.stringify(names.map(name => ({ name, value: 1, extra: dir + " " + factor })), null, 2));', + ].join(' '); + fs.writeFileSync(path.join(tmpDir, 'fake-benchmark.js'), script); + const logFile = path.join(tmpDir, 'retest-output.txt'); + const runBenchmark = createBenchmarkRunner({ + dirs: { base: path.join(tmpDir, 'base'), pr: path.join(tmpDir, 'pr') }, + command: `node ${JSON.stringify(path.join(tmpDir, 'fake-benchmark.js'))}`, + iterationsFactor: 0.5, + logFile, + }); + + expect(runBenchmark('base', ['A', 'B'])).toEqual([ + { name: 'A', value: 1, extra: 'base 0.5' }, + { name: 'B', value: 1, extra: 'base 0.5' }, + ]); + expect(runBenchmark('pr', ['A'])).toEqual([{ name: 'A', value: 1, extra: 'pr 0.5' }]); + expect(fs.readFileSync(logFile, 'utf8')).toContain('log line'); + }); + + it('returns null if the benchmark command produces no results', () => { + const runBenchmark = createBenchmarkRunner({ + dirs: { base: path.join(tmpDir, 'base'), pr: path.join(tmpDir, 'pr') }, + command: 'node -e "process.exit(1)"', + iterationsFactor: 1, + logFile: path.join(tmpDir, 'retest-output.txt'), + }); + expect(runBenchmark('base', ['A'])).toBeNull(); + }); + }); + + describe('command line', () => { + const script = path.join(__dirname, '../benchmark/compare.js'); + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'benchmark-compare-')); + fs.mkdirSync(path.join(tmpDir, 'base')); + fs.mkdirSync(path.join(tmpDir, 'pr')); + fs.writeFileSync(path.join(tmpDir, 'baseline.json'), JSON.stringify([result('A', 1)])); + fs.writeFileSync(path.join(tmpDir, 'pr.json'), JSON.stringify([result('A', 1.3)])); + // Reports the value of the environment variable of the side, named after its directory + const benchmark = [ + 'const side = require("path").basename(process.cwd()).toUpperCase();', + 'const names = JSON.parse(process.env.BENCHMARK_NAMES);', + 'const value = parseFloat(process.env["VALUE_" + side]);', + 'console.log(JSON.stringify(names.map(name => ({ name, value })), null, 2));', + ].join(' '); + fs.writeFileSync(path.join(tmpDir, 'fake-benchmark.js'), benchmark); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + const compare = env => + spawnSync(process.execPath, [script, 'baseline.json', 'pr.json'], { + cwd: tmpDir, + encoding: 'utf8', + env: { + ...process.env, + BENCHMARK_BASE_DIR: 'base', + BENCHMARK_PR_DIR: 'pr', + BENCHMARK_COMMAND: `node ${JSON.stringify(path.join(tmpDir, 'fake-benchmark.js'))}`, + ...env, + }, + }); + + it('exits with 1 if the regression persists in the retest', () => { + const run = compare({ VALUE_BASE: '1', VALUE_PR: '1.3' }); + expect(run.status).toBe(1); + expect(run.stdout).toContain('## Retest'); + expect(run.stdout).toContain('⚠️ **Performance regressions detected.** Please review the changes.'); + }); + + it('exits with 0 if the regression does not persist in the retest', () => { + const run = compare({ VALUE_BASE: '1', VALUE_PR: '1' }); + expect(run.status).toBe(0); + expect(run.stdout).toContain('## Retest'); + expect(run.stdout).toContain('✅ **No significant performance changes.**'); + }); + + it('exits with 1 without retest if the base branch build is not set', () => { + const run = compare({ BENCHMARK_BASE_DIR: '', VALUE_BASE: '1', VALUE_PR: '1' }); + expect(run.status).toBe(1); + expect(run.stdout).not.toContain('## Retest'); + }); + + it('exits with 0 if the results cannot be parsed', () => { + fs.writeFileSync(path.join(tmpDir, 'pr.json'), 'not json'); + const run = compare({ VALUE_BASE: '1', VALUE_PR: '1' }); + expect(run.status).toBe(0); + expect(run.stdout).toBe('⚠️ Could not parse benchmark results\n'); + }); + }); +});