ci(perf): opt-in perf on PRs, run after merge, adaptive bench sampling - #3651
Conversation
Two changes to how CodSpeed perf benchmarks run: 1. Gating policy. Benchmarks no longer run on every PR push. They run automatically after merge (push to main/canary) and on workflow_dispatch; on PRs they are opt-in via a `RUN_CODSPEED=1` / `run-perf` / `/perf` marker in the PR title, body, or any commit message. A new `perf-pr-notice` job keeps a single sticky comment on each PR that either confirms benchmarks were triggered or explains how to opt in. merge_group is always skipped. 2. Adaptive sampling. Replace walltime mode's fixed 100 samples/bench with a ~2s per-bench wall-time budget (DIVAN_MAX_TIME), so divan runs as many samples as fit: ~100 for cheap benches, down to 1 for the heavy O(n^2)/1M-iter workloads. At fixed 100 samples the suite blew past the 30-min timeout — compute::bubble_sort_5k alone is ~14s/sample on the (~5x slower than local) codspeed-macro runner, i.e. ~23min for one bench. With the budget the full 36-bench suite finishes in ~2min (81s measured locally). The budget is set in CI via env and defaulted in the bench harness main() so `cargo bench` is adaptive too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughExports a new ChangesPerformance Benchmark Opt-In Policy
Sequence DiagramsequenceDiagram
participant Trigger as Workflow Trigger
participant CheckPerf as check_perf Step
participant Determine as determine_changes Job
participant BuildJob as benchmarks-build Job
participant RunJob as benchmarks-run Job
participant PRNotice as perf-pr-notice Job
Trigger->>CheckPerf: event_name, PR title/body, commits
CheckPerf->>CheckPerf: detect perf markers / evaluate event type
CheckPerf->>Determine: outputs.run = true|false
Determine->>BuildJob: needs.determine_changes.outputs.run_perf
Determine->>RunJob: needs.determine_changes.outputs.run_perf
alt run_perf == true
BuildJob->>RunJob: proceed to build and run benchmarks
RunJob->>RunJob: execute with DIVAN_MAX_TIME=2
else run_perf == false
PRNotice->>PRNotice: upsert sticky PR comment with opt-in instructions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🏎️ Performance benchmarks are running for this PRCodSpeed perf benchmarks were triggered because this PR opted in. Results will appear in the CodSpeed check / dashboard once they finish. |
Empty commit to trigger the opt-in perf gate (synchronize event) so the benchmark path runs end-to-end on this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 718-764: The perf PR notice script should skip attempting to write
comments for forked pull requests to avoid 403s; add a guard at the top of the
script (before calling
github.paginate/github.rest.issues.updateComment/createComment) that checks
whether the PR is from a fork (e.g., inspect
context.payload.pull_request.head.repo.fork or compare
context.payload.pull_request.head.repo.owner.login to context.repo.owner) and,
if true, skip posting/updating the comment (log or set body to a no-op). Ensure
the check runs before using context.issue.number and the github.rest.issues.*
methods so forks don’t call updateComment/createComment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8ba03c7c-435f-4db4-b405-128050c7807c
📒 Files selected for processing (2)
.github/workflows/ci.yamlbaml_language/crates/baml_tests/benches/runtime_benchmark.rs
Merging this PR will create unknown performance changes
Performance Changes
Comparing 
Footnotes
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: hellovai <vaibhavtheory@gmail.com>
After-merge perf runs (push to main/canary) previously fired on every push. Now check_perf diffs the pushed range (before..after) against baml_language/** and skips the benchmarks when the merge didn't touch it. Diff the push payload's before/after directly rather than reuse determine_changes' `code` flag: on a push the merge-base of HEAD and origin/<branch> collapses to HEAD, so that diff is always empty. before..after is the real merged range. Falls back to running when before is the zero SHA (branch creation / unknown previous tip). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What & why
Two changes to how CodSpeed perf benchmarks run in
ci.yaml.1. Perf is opt-in on PRs, automatic after merge
Previously benchmarks ran on every push to any PR that touched
baml_language/. Now:main/canary) and onworkflow_dispatch→ always run.RUN_CODSPEED=1,run-perf, or/perf.merge_group→ never (already settled on the PR).A new
perf-pr-noticejob maintains a single sticky comment on each PR (hidden marker, edited in place — no spam) that either confirms benchmarks were triggered or tells the author how to opt in.Mechanically: a
check_perfstep indetermine_changescomputes arun_perfoutput; bothbenchmarks-buildandbenchmarks-rungate on it.2. Adaptive sampling — fixes the 30-min CI timeout
The perf job was timing out at
timeout-minutes: 30and getting cancelled mid-run. Not a deadlock — benches completed one-by-one until GitHub killed the job.Root cause: CodSpeed walltime mode runs a fixed 100 samples/bench across 36 benches, several of them heavy. The long pole is
compute::bubble_sort_5k(O(n²) ≈ 12.5M VM iterations): ~14s/sample on the codspeed-macro runner (which is ~5× slower than local) × 100 samples ≈ 23 min for that one bench.Fix: bound each bench to a ~2s adaptive wall-time budget (
DIVAN_MAX_TIME). divan then runs as many samples as fit — ~100 for cheap benches (full statistical power kept), down to 1 for the heavy workloads (whose run-to-run variance is already tiny).Set in CI via the step
env(tunable from the workflow) and defaulted in the bench harnessmain()so localcargo bench/cargo codspeed runare adaptive too.Measured (local,
cargo codspeed run)DIVAN_MAX_TIME=2bubble_sort_5kalonemixed_ops,parallel_sum)Verified the in-
maindefault applies with no env var set: suite ran 81s,bubble_sort=1 sample, cheap benches=100. Projected CI: ~2 min, down from a >30-min timeout.Notes
continue-on-error, not a required gate) — this never blocks a merge.bubble_sort_5k's single ~14s sample on the macro runner (a time budget can't interrupt a sample mid-flight). Shrinking that O(n²) workload is a possible follow-up, but it's shared with the cross-language speedtest corpus, so left as-is.🤖 Generated with Claude Code
Summary by CodeRabbit
RUN_CODSPEED=1 / run-perf