From 3ff4b9dc17dd0854797ac4c454d6ed973e19d352 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 3 Nov 2026 15:28:08 +0530 Subject: [PATCH 1/2] ci(bench): cover foreign runtime costs (Refs #776) Add dedicated Python, browser, Node and C jobs with exact core provenance, complete JSON metric checks and isolated Bencher testbeds. Co-Authored-By: Codex --- .github/bench/bindings-report.cjs | 48 +++++ .github/bench/bindings-report.test.cjs | 45 +++++ .github/bench/bindings.sh | 60 ++++++ .github/workflows/bench-report.yml | 2 +- .github/workflows/bench.yml | 251 +++++++++++++++++++++++++ docs/BINDING_BENCHMARKS.md | 93 +++++++++ 6 files changed, 498 insertions(+), 1 deletion(-) create mode 100644 .github/bench/bindings-report.cjs create mode 100644 .github/bench/bindings-report.test.cjs create mode 100644 .github/bench/bindings.sh create mode 100644 docs/BINDING_BENCHMARKS.md diff --git a/.github/bench/bindings-report.cjs b/.github/bench/bindings-report.cjs new file mode 100644 index 000000000..1940e790a --- /dev/null +++ b/.github/bench/bindings-report.cjs @@ -0,0 +1,48 @@ +// Binding jobs use Bencher JSON (nanoseconds), with separate foreign testbeds. +const fs = require('node:fs'); +const { audit } = require('./report.cjs'); +function expected(binding) { + if (!['python', 'browser', 'node', 'c'].includes(binding)) throw new Error('unknown binding'); + const suffixes = ['prepared_execute/1k', 'cursor_batch_1/1k', 'cursor_batch_256/1k', 'set_roundtrip/2', 'predicate/1k', 'aggregate/1k']; + if (binding === 'browser') suffixes.push('indexeddb_write/1k', 'indexeddb_reopen/1k'); + return suffixes.map(s => `binding/${binding}/${s}`); +} +function validate(binding, metrics) { + const ids = expected(binding); + if (JSON.stringify(Object.keys(metrics).sort()) !== JSON.stringify([...ids].sort())) throw new Error('binding IDs do not match shortlist'); + for (const id of ids) { + const value = metrics[id]?.latency?.value; + if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) throw new Error(`invalid latency: ${id}`); + } + return ids; +} +function main(command) { + const read = p => fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : ''; + const binding = process.env.BINDING; + const ids = expected(binding); + if (command === 'validate') { + validate(binding, JSON.parse(read('binding/binding-results.json'))); + const executed = read('binding-execution.txt').split('\n').filter(s => s.startsWith('ok: ')).map(s => s.slice(4)); + if (executed.length !== ids.length || new Set(executed).size !== ids.length || executed.some(id => !ids.includes(id))) throw new Error('executed IDs do not match shortlist'); + return; + } + if (command !== 'finish') throw new Error('unknown command'); + // Reuse the tested upload/alert parser without representing these as native + // Criterion measurements: only the list of executed IDs is adapted. + const execution = read('binding-execution.txt').replace(/^ok: (.+)$/gm, 'Benchmarking $1: Analyzing'); + const result = audit(ids, execution, read('binding-bencher.txt'), process.env.BENCHER_OUTCOME); + const report = { + binding, coreSha: process.env.BENCH_SHA, bindingSha: read('binding-sha.txt').trim(), + workflowSha: process.env.WORKFLOW_SHA, ref: process.env.BENCH_REF, + testbed: `binding-${binding}-ubuntu24-v1`, selected: ids, ...result, + }; + fs.writeFileSync('binding-report.json', JSON.stringify(report, null, 2) + '\n'); + const summary = `### Binding benchmark: ${binding}\n\nCore: ${report.coreSha}; binding: ${report.bindingSha}; workflow: ${report.workflowSha}\n\nTestbed: ${report.testbed}; executed ${result.executed.length}/${ids.length}; uploaded ${result.uploaded.length}/${ids.length}; alerts ${result.alerts.length}; upload ${result.uploadStatus}\n\n${result.errors.join('\n')}\n`; + console.log(summary); + if (process.env.GITHUB_STEP_SUMMARY) fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary); + if (result.errors.length) process.exitCode = 1; +} +module.exports = { expected, validate }; +if (require.main === module) { + try { main(process.argv[2]); } catch (e) { console.error(e.message); process.exitCode = 1; } +} diff --git a/.github/bench/bindings-report.test.cjs b/.github/bench/bindings-report.test.cjs new file mode 100644 index 000000000..1f36634a0 --- /dev/null +++ b/.github/bench/bindings-report.test.cjs @@ -0,0 +1,45 @@ +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { expected, validate } = require('./bindings-report.cjs'); +const data = binding => Object.fromEntries(expected(binding).map(id => [id, { latency: { value: 100 } }])); +test('exact dedicated routes and finite positive latency only', () => { + for (const binding of ['python', 'browser', 'node', 'c']) { + assert.equal(validate(binding, data(binding)).length, binding === 'browser' ? 8 : 6); + const missing = data(binding); delete missing[expected(binding)[0]]; + assert.throws(() => validate(binding, missing), /IDs/); + const extra = data(binding); extra['native/query'] = { latency: { value: 1 } }; + assert.throws(() => validate(binding, extra), /IDs/); + for (const value of [0, -1, null, Infinity, '12']) { + const bad = data(binding); bad[expected(binding)[0]].latency.value = value; + assert.throws(() => validate(binding, bad), /latency/); + } + } + assert.throws(() => expected('unknown'), /binding/); +}); + +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +test('CLI verifies execution and records provenance even when upload is partial', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'p16-report-')); + try { + fs.mkdirSync(path.join(dir, 'binding')); + fs.writeFileSync(path.join(dir, 'binding/binding-results.json'), JSON.stringify(data('python'))); + fs.writeFileSync(path.join(dir, 'binding-execution.txt'), expected('python').map(id => `ok: ${id}`).join('\n')); + fs.writeFileSync(path.join(dir, 'binding-sha.txt'), 'binding-commit\n'); + const env = { ...process.env, BINDING: 'python', BENCH_SHA: 'core-commit', WORKFLOW_SHA: 'workflow-commit', BENCH_REF: 'v3', BENCHER_OUTCOME: 'success' }; + const run = command => spawnSync(process.execPath, [path.join(__dirname, 'bindings-report.cjs'), command], { cwd: dir, env, encoding: 'utf8' }); + assert.equal(run('validate').status, 0); + fs.appendFileSync(path.join(dir, 'binding-execution.txt'), '\nok: binding/python/aggregate/1k'); + assert.notEqual(run('validate').status, 0); + assert.notEqual(run('finish').status, 0); + const report = JSON.parse(fs.readFileSync(path.join(dir, 'binding-report.json'))); + assert.equal(report.coreSha, 'core-commit'); + assert.equal(report.bindingSha, 'binding-commit'); + assert.equal(report.workflowSha, 'workflow-commit'); + assert.equal(report.testbed, 'binding-python-ubuntu24-v1'); + assert.equal(report.uploaded.length, 0); + assert.ok(report.errors.length); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); diff --git a/.github/bench/bindings.sh b/.github/bench/bindings.sh new file mode 100644 index 000000000..e0170e257 --- /dev/null +++ b/.github/bench/bindings.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Called from the core checkout. Never changes checked-in binding dependencies. +set -euo pipefail +mode=$1 +binding=$2 +core_dir=$PWD +export CARGO_TARGET_DIR="$PWD/binding/target/p16" +export CARGO_BUILD_JOBS=2 +cd binding +if [[ "$mode" = build ]]; then + mkdir -p .cargo + # Benchmark the exact measured core checkout, not the binding's release pin. + cat > .cargo/config.toml < target-metadata.json + python3 - "$core_dir" <<'PY' +import json, pathlib, sys +packages = json.load(open('target-metadata.json'))['packages'] +core = [p for p in packages if p['name'] == 'minigraf'] +assert len(core) == 1 and core[0]['source'] is None +assert pathlib.Path(core[0]['manifest_path']).parent == pathlib.Path(sys.argv[1]) +PY + case "$binding" in + python) + python3 -m venv target/bench-venv + source target/bench-venv/bin/activate + pip install 'maturin>=1.5,<2' + cargo build --release --bin uniffi-bindgen + maturin develop --release + ;; + browser) + cargo install wasm-pack --locked --version 0.13.1 + wasm-pack build --target web --out-dir pkg -- --features browser + npm install --no-save --package-lock=false playwright@1.55.0 + npx playwright install --with-deps chromium + ;; + node) + cargo build --release --lib + cp "$CARGO_TARGET_DIR/release/libminigraf_node.so" minigraf.linux-x64-gnu.node + ;; + c) + cargo build --release --lib + cc -O2 -std=c11 -Wall -Wextra -Werror -Iinclude bench/nightly.c \ + "$CARGO_TARGET_DIR/release/libminigraf.a" -lpthread -ldl -lm -o "$CARGO_TARGET_DIR/nightly" + ;; + *) exit 1 ;; + esac +else + # --test is forwarded verbatim and never writes timing data. + shift 2 + case "$binding" in + python) target/bench-venv/bin/python bench/nightly.py "$@" ;; + browser|node) node bench/nightly.mjs "$@" ;; + c) "$CARGO_TARGET_DIR/nightly" "$@" ;; + *) exit 1 ;; + esac +fi diff --git a/.github/workflows/bench-report.yml b/.github/workflows/bench-report.yml index 09b17dbb1..ccdd53aa6 100644 --- a/.github/workflows/bench-report.yml +++ b/.github/workflows/bench-report.yml @@ -19,4 +19,4 @@ jobs: - uses: actions/setup-node@v6 with: node-version: '24' - - run: node --test .github/bench/report.test.cjs + - run: node --test .github/bench/*.test.cjs diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 31d9f2338..9ffc831f4 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -20,6 +20,11 @@ on: required: true default: main + binding-ref: + description: Binding ref for all P16 repos (phase B may select the PR branch) + required: false + default: v3 + permissions: contents: read issues: write @@ -544,3 +549,249 @@ jobs: bench-report.json ${{ matrix.file }} bencher_out.txt + + # Actual foreign runtimes. Deliberately separate from Criterion/native history. + # Reserve 5–15 minutes of measurement per binding plus build/setup, unverified. + bindings: + name: bench / binding_${{ matrix.binding }} + needs: compile + runs-on: ubuntu-24.04 + timeout-minutes: 60 + strategy: + fail-fast: false + matrix: + include: + - { binding: python, repo: minigraf-python } + - { binding: browser, repo: minigraf-wasm } + - { binding: node, repo: minigraf-node } + - { binding: c, repo: minigraf-c } + env: + BINDING: ${{ matrix.binding }} + BENCH_SHA: ${{ needs.compile.outputs.sha }} + WORKFLOW_SHA: ${{ github.workflow_sha }} + steps: + - uses: actions/checkout@v7 + with: + ref: ${{ needs.compile.outputs.sha }} + - name: Check v3 binding suite availability + id: source + run: | + if [ -f benches/minigraf_bench/prepared.rs ]; then + echo 'enabled=true' >> "$GITHUB_OUTPUT" + else + echo 'Skipping v3-only binding jobs for this measured checkout.' >> "$GITHUB_STEP_SUMMARY" + fi + - uses: actions/checkout@v7 + if: steps.source.outputs.enabled == 'true' + with: + ref: ${{ github.workflow_sha }} + path: .bench-workflow + sparse-checkout-cone-mode: false + sparse-checkout: /.github/bench/ + persist-credentials: false + - uses: actions/checkout@v7 + if: steps.source.outputs.enabled == 'true' + with: + repository: project-minigraf/${{ matrix.repo }} + ref: ${{ inputs.binding-ref || 'v3' }} + path: binding + persist-credentials: false + - name: Pin binding source + if: steps.source.outputs.enabled == 'true' + run: git -C binding rev-parse HEAD | tee binding-sha.txt + - uses: dtolnay/rust-toolchain@stable + if: steps.source.outputs.enabled == 'true' + with: + targets: wasm32-unknown-unknown + - uses: actions/setup-python@v6 + if: steps.source.outputs.enabled == 'true' + with: + python-version: '3.11' + - uses: actions/setup-node@v6 + if: steps.source.outputs.enabled == 'true' + with: + node-version: '20' + - name: Build binding with measured core + if: steps.source.outputs.enabled == 'true' + run: bash .bench-workflow/.github/bench/bindings.sh build "$BINDING" + - name: Verify every binding case (untimed) + if: steps.source.outputs.enabled == 'true' + run: bash .bench-workflow/.github/bench/bindings.sh run "$BINDING" --test | tee binding-test.txt + - name: Measure binding shortlist + id: measurement + if: steps.source.outputs.enabled == 'true' + run: | + set -o pipefail + bash .bench-workflow/.github/bench/bindings.sh run "$BINDING" | tee binding-execution.txt + node .bench-workflow/.github/bench/bindings-report.cjs validate + - uses: bencherdev/bencher@v0.4.25 + if: steps.measurement.outcome == 'success' + - name: Upload foreign runtime metrics + id: bencher + if: steps.measurement.outcome == 'success' + continue-on-error: true + run: | + set -o pipefail + bencher run --project minigraf --token "${{ secrets.BENCHER_API_TOKEN }}" \ + --branch "$BENCH_REF" --hash "$BENCH_SHA" \ + --testbed "binding-$BINDING-ubuntu24-v1" \ + --threshold-measure latency --threshold-test t_test \ + --threshold-max-sample-size 30 --threshold-upper-boundary "$BENCH_THRESHOLD" \ + --err --adapter json --file binding/binding-results.json 2>&1 | tee binding-bencher.txt + - name: Report regression + if: steps.bencher.outcome == 'failure' + uses: actions/github-script@v9 + env: + SUITE: binding_${{ matrix.binding }} + with: + script: | + const fs = require('fs'); + const { owner, repo } = context.repo; + const suite = process.env.SUITE; + const ref = process.env.BENCH_REF; + const jobName = `bench / ${suite}`; + const label = `${suite}, ${ref}`; + + // Bencher lists each alert as "- (Latency): <.../alerts/>". + // The "View results" lines use /perf? URLs, so they do not match. + const alertRe = /- (\S+) \(Latency\): https:\/\/bencher\.dev\/console\/projects\/[^/\s]+\/alerts\//g; + const parseAlerts = text => new Set([...text.matchAll(alertRe)].map(m => m[1])); + + const current = parseAlerts(fs.readFileSync('binding-bencher.txt', 'utf8')); + const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; + + let body; + if (current.size === 0) { + // Bencher failed without listing alerts (API or upload error). + // Report it at once so a broken setup does not go unnoticed. + body = [ + `Bencher failed in the nightly benchmark run (${label}) but listed no alerts. This is likely an API or upload error.`, + '', + `**Branch:** \`${ref}\``, + `**Measured checkout:** \`${process.env.BENCH_SHA}\``, + `**Run:** ${runUrl}`, + ].join('\n'); + } else { + // Find this suite's job in the previous completed run of the same + // branch (by run name, see run-name) and read its alerts. Runs + // from before the branch was in the run name count as main. + let previous = new Set(); + let previousUrl = null; + try { + const { data: runs } = await github.rest.actions.listWorkflowRuns({ + owner, repo, + workflow_id: 'bench.yml', + branch: 'main', + status: 'completed', + per_page: 20, + }); + const runRef = r => { + const m = /\(([^)]+)\)$/.exec(r.display_title || ''); + return m ? m[1] : 'main'; + }; + const prevRun = runs.workflow_runs.find( + r => r.id !== context.runId && runRef(r) === ref); + if (prevRun) { + previousUrl = prevRun.html_url; + const { data: jobs } = await github.rest.actions.listJobsForWorkflowRun({ + owner, repo, run_id: prevRun.id, per_page: 100, + }); + const prevJob = jobs.jobs.find(j => j.name === jobName); + if (prevJob) { + const { data: log } = await github.rest.actions.downloadJobLogsForWorkflowRun({ + owner, repo, job_id: prevJob.id, + }); + previous = parseAlerts(typeof log === 'string' ? log : Buffer.from(log).toString('utf8')); + } + } + } catch (err) { + core.warning(`Could not read the previous run's alerts: ${err.message}`); + } + + const repeated = [...current].filter(b => previous.has(b)).sort(); + console.log(`Alerts this run: ${[...current].join(', ')}`); + console.log(`Alerts in previous run: ${[...previous].join(', ') || 'none'}`); + if (repeated.length === 0) { + const msg = `Bencher flagged ${current.size} benchmark(s) in ${label}, but none alerted on the previous run too. Not reporting.`; + core.warning(msg); + await core.summary + .addHeading(`Benchmark alerts not reported (${label})`, 3) + .addRaw(msg, true) + .addList([...current].sort().map(b => `${b}`)) + .write(); + return; + } + + body = [ + `Bencher flagged these benchmarks in the nightly run (${label}) on two runs in a row:`, + '', + ...repeated.map(b => `- \`${b}\``), + '', + `**Branch:** \`${ref}\``, + `**Measured checkout:** \`${process.env.BENCH_SHA}\``, + `**Run:** ${runUrl}`, + `**Previous run:** ${previousUrl}`, + '', + 'Please review the Bencher dashboard to confirm the regression.', + ].join('\n'); + } + + const { data: openIssues } = await github.rest.issues.listForRepo({ + owner, repo, + labels: 'performance', + state: 'open', + per_page: 100, + }); + const title = `Benchmark regression: ${suite} (${ref})`; + // Issues for main used to be titled without the branch. Keep + // commenting on such an open issue, and give it the new title. + const legacyTitle = ref === 'main' ? `Benchmark regression: ${suite}` : null; + const existing = + openIssues.find(i => !i.pull_request && i.title === title) || + openIssues.find(i => !i.pull_request && legacyTitle && i.title === legacyTitle); + let reported; + if (existing) { + if (existing.title !== title) { + await github.rest.issues.update({ + owner, repo, issue_number: existing.number, title, + }); + } + await github.rest.issues.createComment({ + owner, repo, issue_number: existing.number, body, + }); + reported = `Commented on #${existing.number}`; + } else { + const { data: issue } = await github.rest.issues.create({ + owner, repo, title, body, + labels: ['performance'], + }); + reported = `Opened #${issue.number}`; + } + core.warning(`${title}. ${reported}.`); + await core.summary + .addHeading(`Benchmark regression reported (${label})`, 3) + .addRaw(`${reported}.`, true) + .addBreak() + .addRaw(body, true) + .write(); + + - name: Verify binding reporting + if: always() && steps.source.outputs.enabled == 'true' + env: + BENCHER_OUTCOME: ${{ steps.bencher.outcome }} + run: node .bench-workflow/.github/bench/bindings-report.cjs finish + - name: Retain binding evidence + if: always() && steps.source.outputs.enabled == 'true' + uses: actions/upload-artifact@v6 + with: + name: binding-evidence-${{ matrix.binding }} + retention-days: 30 + if-no-files-found: error + path: | + binding-sha.txt + binding-test.txt + binding-execution.txt + binding-bencher.txt + binding-report.json + binding/binding-results.json + binding/target-metadata.json diff --git a/docs/BINDING_BENCHMARKS.md b/docs/BINDING_BENCHMARKS.md new file mode 100644 index 000000000..e7d53e9b4 --- /dev/null +++ b/docs/BINDING_BENCHMARKS.md @@ -0,0 +1,93 @@ +# Binding benchmarks (#776 P16) + +These are foreign-runtime measurements, independent of native Criterion data. +The `bindings` matrix in `.github/workflows/bench.yml` selects the whole fixed +shortlist in each binding's `bench/nightly.*` entry point. Core and binding PRs +must land before enabling the default-v3 route. The default-main workflow needs +its identical companion; schedules then select v3 at 14:00 UTC. The 02:00 main +checkout skips all four jobs before building or uploading. Manual runs may use +`binding-ref` to select the common `bench/776-p16-bindings` PR branch during +phase B; no dispatch is part of phase A. + +All cases use 1,000 deterministic in-memory integer facts, plus one two-element +set fact. The suffixes below expand under `binding/{python,browser,node,c}/`: + +| Suffix | Measured operation | +| --- | --- | +| `prepared_execute/1k` | Prepared integer binding, point query, foreign result allocation and JSON decoding | +| `cursor_batch_1/1k` | Prepared cursor open, full drain of 1k rows in batches of 1, decode and close | +| `cursor_batch_256/1k` | Same drain in batches of up to 256; final partial batch and EOF included | +| `set_roundtrip/2` | Host set (C: typed JSON) binding, exact set lookup, set returned/decoded | +| `predicate/1k` | 1k foreign predicate callbacks returning the 500 even values | +| `aggregate/1k` | Foreign accumulator init, 1k steps, finish and returned scalar | + +Browser adds `binding/browser/indexeddb_write/1k` (one 1k-fact commit including +its write-through flush) and `binding/browser/indexeddb_reopen/1k` (open an +already flushed database). Each persistence sample has a fresh database, with +creation/seeding outside measurement as appropriate; the row count is checked +after measurement. Reopen uses a fresh BrowserDb over IndexedDB in the same +page, not a cold browser/page startup. Each run has a fresh Chromium context. + +Fixture creation, query preparation and callback registration are outside all +measurements. Results are checked against fixed counts AND sums, exact point +results or the exact returned set, rather than treating an empty result as +success. Python/Node/browser decoding is part of the round trip. C decodes the +fixed integer result shape itself, checks cursor errors and frees every +returned string/handle. Preparation and foreign callback registration latency +are not measured by these representative cases. + +Python and the JS in-memory cases collect ten samples averaging 20 invocations +following three untimed warmups; this amortizes browser clock resolution. +C and browser persistence collect ten single-operation samples. Medians are +serialized as Bencher JSON `latency.value` in **nanoseconds**. There are no +Criterion IDs or fabricated Criterion output: only the executed-ID list is +adapted internally to reuse the existing upload completeness checker. + +Run from each binding worktree after building it: + +```sh +# No timers, one invocation of every new ID, no metrics file written. +python bench/nightly.py --test # Python (installed local extension) +node bench/nightly.mjs --test # Node; browser with pkg/ and Playwright +./target/p16/nightly --test # C (compile recipe in bench/nightly.c) +``` + +The core helper `.github/bench/bindings.sh` builds a binding under `binding/` +with a Cargo patch pointing at the exact measured core checkout and verifies +Cargo metadata resolves that source. Release pins/manifests are unchanged. +Artifacts retain both commit hashes, workflow hash, resolved Cargo metadata, +executed IDs, raw metrics, upload output and the completeness report. Bencher +uses `binding-{python,browser,node,c}-ubuntu24-v1` testbeds, Python 3.11, +Node 20 and Playwright 1.55.0 Chromium on Ubuntu 24.04. Native histories are not +inherited. Each testbed has its own 0.999 t-test threshold with a 30-point +window; alerts use the existing two-consecutive-run issue policy, while missing +metrics, partial uploads and API failures fail the job immediately. + +Reserve **5–15 job-minutes per binding plus build/runtime setup**, unverified. +The job timeout is 60 minutes including builds. Phase B must dispatch serially, +confirm every ID and upload, establish the new testbed histories, and revise +this allowance from actual Actions evidence. No latency or RSS measurement was +performed locally. These latency cases do not enforce RSS bounds. + +## Deliberate dispositions requiring release sign-off + +- Java/JVM: no JVM latency history yet. Python exercises the shared UniFFI + adapter, but does not measure JVM allocation, JNI or JVM callbacks. JVM + timings remain a manual release check, not claimed nightly coverage. +- Android: ART/ABI and device storage timing require a fixed physical device; + emulator timings on a hosted Linux runner are not a substitute. Manual + device benchmarks and release sign-off remain required. +- Swift: ARC/callback and Apple-device storage timing require stable Apple + hardware. Manual macOS/iOS benchmarks and release sign-off remain required; + Linux UniFFI tests do not provide these timings. +- WASI: browser Wasm does not measure the WASI runtime. Tier 2 WASI REPL startup + and runtime overhead remain manual; it exposes no matching foreign prepared + object or synchronous host callback API in the audited package. +- Options validation/error paths and fact-log/LogWriter conversions are outside + this shortlist. Native tests/benchmarks cover engine work, not these foreign + conversions. They remain explicitly unmeasured follow-up scope. Browser + persistence coverage is write-through/open only; migration and pinned-reader + churn remain manual. Deliberate callback errors stay correctness tests. + +G15 therefore gains scheduled representatives for four host runtimes; the +listed manual/excluded paths need explicit release sign-off before #776 closes. From fef01c5aca3962db056eadc2834efc2e25716648 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 3 Nov 2026 15:34:55 +0530 Subject: [PATCH 2/2] ci(bench): preserve verification and isolate binding experiments (Refs #776) Apply verified review findings: preserve failed untimed checks and keep experimental binding refs out of nightly history and issue reporting. Co-Authored-By: Codex --- .github/bench/bindings-report.cjs | 4 ++-- .github/bench/bindings-report.test.cjs | 3 ++- .github/workflows/bench.yml | 18 ++++++++++++++---- docs/BINDING_BENCHMARKS.md | 4 +++- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/bench/bindings-report.cjs b/.github/bench/bindings-report.cjs index 1940e790a..21c98217b 100644 --- a/.github/bench/bindings-report.cjs +++ b/.github/bench/bindings-report.cjs @@ -33,11 +33,11 @@ function main(command) { const result = audit(ids, execution, read('binding-bencher.txt'), process.env.BENCHER_OUTCOME); const report = { binding, coreSha: process.env.BENCH_SHA, bindingSha: read('binding-sha.txt').trim(), - workflowSha: process.env.WORKFLOW_SHA, ref: process.env.BENCH_REF, + workflowSha: process.env.WORKFLOW_SHA, ref: process.env.BENCH_REF, historyBranch: process.env.BINDING_HISTORY, testbed: `binding-${binding}-ubuntu24-v1`, selected: ids, ...result, }; fs.writeFileSync('binding-report.json', JSON.stringify(report, null, 2) + '\n'); - const summary = `### Binding benchmark: ${binding}\n\nCore: ${report.coreSha}; binding: ${report.bindingSha}; workflow: ${report.workflowSha}\n\nTestbed: ${report.testbed}; executed ${result.executed.length}/${ids.length}; uploaded ${result.uploaded.length}/${ids.length}; alerts ${result.alerts.length}; upload ${result.uploadStatus}\n\n${result.errors.join('\n')}\n`; + const summary = `### Binding benchmark: ${binding}\n\nCore: ${report.coreSha}; binding: ${report.bindingSha}; workflow: ${report.workflowSha}\n\nHistory branch: ${report.historyBranch}; testbed: ${report.testbed}; executed ${result.executed.length}/${ids.length}; uploaded ${result.uploaded.length}/${ids.length}; alerts ${result.alerts.length}; upload ${result.uploadStatus}\n\n${result.errors.join('\n')}\n`; console.log(summary); if (process.env.GITHUB_STEP_SUMMARY) fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary); if (result.errors.length) process.exitCode = 1; diff --git a/.github/bench/bindings-report.test.cjs b/.github/bench/bindings-report.test.cjs index 1f36634a0..cdf921d40 100644 --- a/.github/bench/bindings-report.test.cjs +++ b/.github/bench/bindings-report.test.cjs @@ -28,7 +28,7 @@ test('CLI verifies execution and records provenance even when upload is partial' fs.writeFileSync(path.join(dir, 'binding/binding-results.json'), JSON.stringify(data('python'))); fs.writeFileSync(path.join(dir, 'binding-execution.txt'), expected('python').map(id => `ok: ${id}`).join('\n')); fs.writeFileSync(path.join(dir, 'binding-sha.txt'), 'binding-commit\n'); - const env = { ...process.env, BINDING: 'python', BENCH_SHA: 'core-commit', WORKFLOW_SHA: 'workflow-commit', BENCH_REF: 'v3', BENCHER_OUTCOME: 'success' }; + const env = { ...process.env, BINDING: 'python', BENCH_SHA: 'core-commit', WORKFLOW_SHA: 'workflow-commit', BENCH_REF: 'v3', BINDING_HISTORY: 'binding-experiment/v3/pr', BENCHER_OUTCOME: 'success' }; const run = command => spawnSync(process.execPath, [path.join(__dirname, 'bindings-report.cjs'), command], { cwd: dir, env, encoding: 'utf8' }); assert.equal(run('validate').status, 0); fs.appendFileSync(path.join(dir, 'binding-execution.txt'), '\nok: binding/python/aggregate/1k'); @@ -39,6 +39,7 @@ test('CLI verifies execution and records provenance even when upload is partial' assert.equal(report.bindingSha, 'binding-commit'); assert.equal(report.workflowSha, 'workflow-commit'); assert.equal(report.testbed, 'binding-python-ubuntu24-v1'); + assert.equal(report.historyBranch, 'binding-experiment/v3/pr'); assert.equal(report.uploaded.length, 0); assert.ok(report.errors.length); } finally { fs.rmSync(dir, { recursive: true, force: true }); } diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 9ffc831f4..031b2eea6 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -567,6 +567,7 @@ jobs: - { binding: c, repo: minigraf-c } env: BINDING: ${{ matrix.binding }} + BINDING_REF: ${{ inputs.binding-ref || 'v3' }} BENCH_SHA: ${{ needs.compile.outputs.sha }} WORKFLOW_SHA: ${{ github.workflow_sha }} steps: @@ -598,7 +599,14 @@ jobs: persist-credentials: false - name: Pin binding source if: steps.source.outputs.enabled == 'true' - run: git -C binding rev-parse HEAD | tee binding-sha.txt + run: | + set -o pipefail + git -C binding rev-parse HEAD | tee binding-sha.txt + history="$BENCH_REF" + if [ "$BINDING_REF" != v3 ]; then + history="binding-experiment/$BENCH_REF/$BINDING_REF" + fi + echo "BINDING_HISTORY=$history" >> "$GITHUB_ENV" - uses: dtolnay/rust-toolchain@stable if: steps.source.outputs.enabled == 'true' with: @@ -616,7 +624,9 @@ jobs: run: bash .bench-workflow/.github/bench/bindings.sh build "$BINDING" - name: Verify every binding case (untimed) if: steps.source.outputs.enabled == 'true' - run: bash .bench-workflow/.github/bench/bindings.sh run "$BINDING" --test | tee binding-test.txt + run: | + set -o pipefail + bash .bench-workflow/.github/bench/bindings.sh run "$BINDING" --test | tee binding-test.txt - name: Measure binding shortlist id: measurement if: steps.source.outputs.enabled == 'true' @@ -633,13 +643,13 @@ jobs: run: | set -o pipefail bencher run --project minigraf --token "${{ secrets.BENCHER_API_TOKEN }}" \ - --branch "$BENCH_REF" --hash "$BENCH_SHA" \ + --branch "$BINDING_HISTORY" --hash "$BENCH_SHA" \ --testbed "binding-$BINDING-ubuntu24-v1" \ --threshold-measure latency --threshold-test t_test \ --threshold-max-sample-size 30 --threshold-upper-boundary "$BENCH_THRESHOLD" \ --err --adapter json --file binding/binding-results.json 2>&1 | tee binding-bencher.txt - name: Report regression - if: steps.bencher.outcome == 'failure' + if: steps.bencher.outcome == 'failure' && env.BINDING_REF == 'v3' uses: actions/github-script@v9 env: SUITE: binding_${{ matrix.binding }} diff --git a/docs/BINDING_BENCHMARKS.md b/docs/BINDING_BENCHMARKS.md index e7d53e9b4..e06b4c99c 100644 --- a/docs/BINDING_BENCHMARKS.md +++ b/docs/BINDING_BENCHMARKS.md @@ -59,7 +59,9 @@ Artifacts retain both commit hashes, workflow hash, resolved Cargo metadata, executed IDs, raw metrics, upload output and the completeness report. Bencher uses `binding-{python,browser,node,c}-ubuntu24-v1` testbeds, Python 3.11, Node 20 and Playwright 1.55.0 Chromium on Ubuntu 24.04. Native histories are not -inherited. Each testbed has its own 0.999 t-test threshold with a 30-point +inherited. An experimental `binding-ref` uses a separate +`binding-experiment//` history branch and does not post +production regression issues. Each testbed has its own 0.999 t-test threshold with a 30-point window; alerts use the existing two-consecutive-run issue policy, while missing metrics, partial uploads and API failures fail the job immediately.