Skip to content

Commit a1123dc

Browse files
Clean up bench comment: remove setup noise and add summary table
- Move bench-compare.mjs setup messages (setup, install, CPU pinning) to stderr so only mitata output reaches stdout - Suppress pnpm install stdout in control dir setup - Remove 2>&1 from workflow pipe so stderr flows to CI logs, not output file - Add summary table above <details> by parsing JSON bench results - Add safety-net regex to strip any leaked setup lines from raw output - Pass BENCH_JSON_OUTPUT env var to format step Co-authored-by: NullVoxPopuli <[email protected]>
1 parent c74769b commit a1123dc

3 files changed

Lines changed: 109 additions & 12 deletions

File tree

.github/workflows/bench-compare.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ concurrency:
1010

1111
jobs:
1212
bench-compare:
13-
name: "Benchmark Comparison"
13+
name: 'Benchmark Comparison'
1414
runs-on: ubuntu-latest
1515
permissions:
1616
pull-requests: write
@@ -30,12 +30,13 @@ jobs:
3030
BENCH_JSON_OUTPUT: ${{ runner.temp }}/bench-results.json
3131
run: |
3232
set -o pipefail
33-
pnpm bench:compare 2>&1 | sed 's/\x1b\[[0-9;]*m//g' > "$RUNNER_TEMP/bench-output.txt"
33+
pnpm bench:compare | sed 's/\x1b\[[0-9;]*m//g' > "$RUNNER_TEMP/bench-output.txt"
3434
3535
- name: Format PR comment
3636
if: always()
3737
env:
3838
BENCH_OUTPUT_FILE: ${{ runner.temp }}/bench-output.txt
39+
BENCH_JSON_OUTPUT: ${{ runner.temp }}/bench-results.json
3940
BENCH_JOB_SUCCESS: ${{ job.status == 'success' }}
4041
run: node scripts/format-bench-comment.mjs > "$RUNNER_TEMP/bench-comment.md"
4142

scripts/bench-compare.mjs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ function resolveRef(branch) {
5858
const ROOT = process.cwd();
5959
const CONTROL_DIR = join(tmpdir(), `bench-control-${BASE_BRANCH}-${Date.now()}`);
6060

61-
console.log(`\n🔧 Setting up control (${BASE_BRANCH}) in ${CONTROL_DIR}\n`);
61+
console.error(`\n🔧 Setting up control (${BASE_BRANCH}) in ${CONTROL_DIR}\n`);
6262

6363
const BASE_REF = resolveRef(BASE_BRANCH);
64-
console.log(` Resolved ${BASE_BRANCH}${BASE_REF.slice(0, 10)}\n`);
64+
console.error(` Resolved ${BASE_BRANCH}${BASE_REF.slice(0, 10)}\n`);
6565

6666
// Clean up temp dir on exit
6767
function cleanup() {
@@ -87,11 +87,14 @@ try {
8787
);
8888

8989
// ── 2. Install dependencies in control dir ───────────────────────────────
90-
console.log(`\n📦 Installing dependencies for control (${BASE_BRANCH})…\n`);
91-
run('pnpm install --frozen-lockfile', { cwd: CONTROL_DIR });
90+
console.error(`\n📦 Installing dependencies for control (${BASE_BRANCH})…\n`);
91+
run('pnpm install --frozen-lockfile', {
92+
cwd: CONTROL_DIR,
93+
stdio: ['inherit', 'pipe', 'inherit'],
94+
});
9295

9396
// ── 3. Run mitata bench with --control-dir ───────────────────────────────
94-
console.log(`\n🏎️ Running benchmarks (experiment vs control)…\n`);
97+
console.error(`\n🏎️ Running benchmarks (experiment vs control)…\n`);
9598

9699
const benchScript = join(ROOT, 'tests/parser.bench.mjs');
97100
const benchArgs = ['--expose-gc', benchScript, '--control-dir', CONTROL_DIR];
@@ -104,7 +107,7 @@ try {
104107
const fullArgs = HAS_TASKSET ? ['-c', '0', 'node', ...benchArgs] : benchArgs;
105108

106109
if (HAS_TASKSET) {
107-
console.log('📌 CPU pinning enabled (taskset -c 0)\n');
110+
console.error('📌 CPU pinning enabled (taskset -c 0)\n');
108111
}
109112

110113
const result = spawnSync(cmd, fullArgs, {
@@ -118,7 +121,7 @@ try {
118121
process.exit(1);
119122
}
120123

121-
console.log('\n✅ Benchmark comparison complete.\n');
124+
console.error('\n✅ Benchmark comparison complete.\n');
122125
} catch (e) {
123126
console.error('❌ Error:', e.message);
124127
process.exit(1);

scripts/format-bench-comment.mjs

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,25 @@
11
/**
22
* Format benchmark comparison results into a GitHub PR comment.
33
*
4-
* Reads the plain-text mitata output from bench-compare.mjs and wraps it in a
5-
* GitHub-flavored markdown comment.
4+
* Reads the plain-text mitata output and (optionally) the JSON results from
5+
* the bench run, then produces a GitHub-flavored markdown comment with:
6+
* 1. A summary table (when comparison data is available)
7+
* 2. Full mitata output in a collapsible <details> section
68
*
79
* Environment variables:
810
* BENCH_OUTPUT_FILE - Path to the plain-text bench output
11+
* BENCH_JSON_OUTPUT - Path to the JSON bench results (optional)
912
* BENCH_JOB_SUCCESS - Set to "true" if the benchmark job succeeded
1013
*/
1114

1215
import { readFileSync } from 'node:fs';
1316

1417
const marker = '<!-- bench-compare -->';
1518

19+
// ---------------------------------------------------------------------------
20+
// Read raw mitata output
21+
// ---------------------------------------------------------------------------
22+
1623
let rawOutput;
1724
try {
1825
rawOutput = readFileSync(process.env.BENCH_OUTPUT_FILE, 'utf8').trim();
@@ -21,13 +28,99 @@ try {
2128
rawOutput = '(no output — benchmark may have failed to start)';
2229
}
2330

31+
// Strip any lines before the mitata header (safety net for leaked setup messages)
32+
const benchStart = rawOutput.search(/^(clk:|benchmark\b)/m);
33+
if (benchStart > 0) {
34+
rawOutput = rawOutput.slice(benchStart);
35+
}
36+
37+
// ---------------------------------------------------------------------------
38+
// Read JSON results (if available) and build summary
39+
// ---------------------------------------------------------------------------
40+
41+
let summarySection = '';
42+
const jsonPath = process.env.BENCH_JSON_OUTPUT;
43+
44+
if (jsonPath) {
45+
try {
46+
const json = JSON.parse(readFileSync(jsonPath, 'utf8'));
47+
summarySection = buildSummary(json);
48+
} catch {
49+
// JSON not available or malformed — skip summary
50+
}
51+
}
52+
53+
function formatTime(ns) {
54+
if (ns >= 1e6) return `${(ns / 1e6).toFixed(2)} ms`;
55+
if (ns >= 1e3) return `${(ns / 1e3).toFixed(2)} µs`;
56+
return `${ns.toFixed(2)} ns`;
57+
}
58+
59+
function deltaEmoji(pct) {
60+
const abs = Math.abs(pct);
61+
// negative pct means experiment is faster (lower time = better)
62+
if (abs < 1) return '⚪';
63+
if (pct <= -5) return '🟢';
64+
if (pct >= 5) return '🔴';
65+
return '🟡';
66+
}
67+
68+
function buildSummary(json) {
69+
const benchmarks = json.benchmarks || [];
70+
71+
// In comparison mode, benchmarks come in pairs inside summary groups.
72+
// Each benchmark alias is like "gts small (control)" / "gts small (experiment)".
73+
// Group them by stripping the suffix.
74+
const pairs = new Map();
75+
76+
for (const trial of benchmarks) {
77+
for (const r of trial.runs || []) {
78+
if (!r.stats) continue;
79+
const m = r.name.match(/^(.+)\s+\((control|experiment)\)$/);
80+
if (!m) continue;
81+
const [, key, role] = m;
82+
if (!pairs.has(key)) pairs.set(key, {});
83+
pairs.get(key)[role] = r.stats;
84+
}
85+
}
86+
87+
if (pairs.size === 0) return '';
88+
89+
const rows = [];
90+
for (const [name, { control, experiment }] of pairs) {
91+
if (!control || !experiment) continue;
92+
const delta = ((experiment.avg - control.avg) / control.avg) * 100;
93+
const emoji = deltaEmoji(delta);
94+
const sign = delta > 0 ? '+' : '';
95+
rows.push(
96+
`| ${emoji} | ${name} | ${formatTime(control.avg)} | ${formatTime(experiment.avg)} | ${sign}${delta.toFixed(1)}% |`
97+
);
98+
}
99+
100+
if (rows.length === 0) return '';
101+
102+
return [
103+
'',
104+
'| | Benchmark | Control (avg) | Experiment (avg) | Δ |',
105+
'|---|---|---:|---:|---:|',
106+
...rows,
107+
'',
108+
'> 🟢 faster · 🔴 slower · 🟡 within 5% · ⚪ within 1%',
109+
'',
110+
].join('\n');
111+
}
112+
113+
// ---------------------------------------------------------------------------
114+
// Assemble comment
115+
// ---------------------------------------------------------------------------
116+
24117
const success = process.env.BENCH_JOB_SUCCESS === 'true';
25118
const heading = success ? '## 🏎️ Benchmark Comparison' : '## ❌ Benchmark Comparison (failed)';
26119

27120
const body = [
28121
marker,
29122
heading,
30-
'',
123+
summarySection,
31124
'<details>',
32125
'<summary>Full mitata output</summary>',
33126
'',

0 commit comments

Comments
 (0)