Benchmarks with base-branch comparison on pull requests - #52
Conversation
Cowritten by Claude
There was a problem hiding this comment.
🔵 Needs a closer look
It changes build configuration (tsconfig.json types) affecting the shipped artifact and adds a CI workflow that builds/runs base-branch code, so it warrants human review alongside the flagged comment-pagination bug.
Pull request overview
This PR adds a performance-benchmarking harness (using mitata) with base-branch comparison, targeted at guarding the regressions fixed in #51/#48. It benchmarks both in-process extractAttrTypeMap calls and whole dist/run.js runs over examples/, and wires a CI workflow that posts (or, for fork PRs, summarizes) the comparison on every pull request. It also adds "types": ["node"] to tsconfig.json so the sources compile under the typescript-7 tsc used since #48.
Changes:
- New
mitatabenchmarks (test/validate.bench.mjs) plus a base-branch comparison harness (scripts/bench-compare.mjs) that git-archives, builds, and runs the base ref side by side. - Formatting utilities for the comparison (
format-bench-comment.mjsfor PR markdown,format-bench-cli.mjsfor terminal,local-bench-summary.shfor local runs) and a CI workflow to publish results. - Supporting config:
tsconfig.jsontypes: ["node"],mitatadependency,package.jsonbench scripts,.gitignore, and README docs.
File summaries
| File | Description |
|---|---|
| test/validate.bench.mjs | New mitata benchmarks (in-process extraction + whole-process runs) with optional control-dir comparison and JSON output. |
| scripts/bench-compare.mjs | Exports/installs/builds the base ref into a temp dir and runs the benchmarks against it, with taskset pinning. |
| scripts/format-bench-comment.mjs | Builds a GitHub markdown comment (summary table + collapsible mitata output) from the JSON/text results. |
| scripts/format-bench-cli.mjs | Renders a terminal-friendly comparison table from the JSON results. |
| scripts/local-bench-summary.sh | Local helper that warns on CPU tuning and prints the CLI summary. |
| .github/workflows/bench-compare.yml | PR workflow that runs the comparison and posts/summarizes it (fork-safe). |
| tsconfig.json | Declares types: ["node"] so sources compile under the TS7 tsc. |
| package.json / pnpm-lock.yaml | Adds the pinned mitata dev dependency and bench/bench:compare/bench:summary scripts. |
| README.md | Documents the new benchmark workflow. |
| .gitignore | Ignores bench-results.json. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 8/11 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🏎️ Benchmark Comparison
Full output |
…er helpers The first CI run took ten minutes: mitata's minimum of twelve samples on four multi-second whole-process cases, twice. Those cases are now timed with three samples each (min and median) over a fixed 20-file subset and reported in the same JSON shape; mitata keeps the in-process benchmarks. Locally the full comparison takes ~35 s including the control build. The workflow runs only for pull requests labelled `run-bench` (or by hand), like the ecosystem workflow. Comment lookup paginates; the two formatters share their helpers. Cowritten by Claude
Cowritten by Claude
Two builds in one V8 heap measured 10-15 % apart on identical code; the comparison now runs the bench script once per side and merges the results. Cowritten by Claude
Cowritten by Claude
mitata benchmarks (
pnpm bench,test/validate.bench.mjs) and a comparison harness adapted from ember-estree (pnpm bench:compare --base <branch>;.github/workflows/bench-compare.ymlposts the comparison on every pull request, or writes it to the job summary for fork PRs).Two kinds of benchmark, because the regressions #51 fixes lived in different places and
pnpm testcould not see either:extractAttrTypeMapper fixture (small, medium, cross-file resolution), disk cache offdist/run.jsoverexamples/— cold (cache off), warm (all cached), one cached file,--no-glintMain vs main on an M1 Max for the noise floor:
Also:
tsconfig.jsonnow declarestypes: ["node"]. The build only found@types/nodethrough vite's type references (viavitest.config.tsmatchinginclude: ["*.ts"]); an export of the sources alone did not compile under thetscthatpnpm buildresolves to, which is thetypescript-7alias's binary since #48.Cowritten by Claude