Skip to content

ci: informational benchmark workflow - #1475

Open
duncanista wants to merge 2 commits into
jordan.gonzalez/bench-suite/featurefrom
jordan.gonzalez/ci-bench/feature
Open

duncanista wants to merge 2 commits into
jordan.gonzalez/bench-suite/featurefrom
jordan.gonzalez/ci-bench/feature

Conversation

@duncanista

@duncanista duncanista commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR: CI builds the benchmarks on PRs, and runs them on the bench label and nightly on main.

Why: It keeps the benchmarks compiling and gives a main baseline to compare a PR against. It is informational only and never blocks a merge.

Overview

Jira: SVLS-9977

Stacked on #1474 (jordan.gonzalez/bench-suite/feature).

Add .github/workflows/bench.yml. No job blocks a PR.

  • bench-build: every PR that touches bottlecap/. cargo bench --no-run --locked with LTO
    off. A failure gives a warning.
  • bench-measure: PRs with the bench label, nightly on main, and manual. Production
    profile, a fixed smoke list with --quick (scripts/ci/bench-measure.sh). Nightly runs save
    the baseline artifact bench-baseline-main; a PR run compares with it (critcmp main pr -t 10) and writes the table to the step summary.

The bench README now has a "CI" section with the wired part and the full plan.

Size: 245 core lines, 279 lines in total.

Testing

  • applies on the previous PR of the stack: pass
  • actionlint 1.7.12 with shellcheck 0.10.0 on the changed workflows: pass
  • shellcheck 0.10.0 on the new scripts: pass
  • The measure path and the baseline download did not run outside GitHub.

@duncanista
duncanista requested a review from a team as a code owner October 28, 2026 18:57
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T21:21:37.776404Z c70178f New commits
🔒 Security Review ✅ Completed 2026-10-28T21:25:00.849847Z c70178f New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f82bb2637b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with:
name: bench-baseline-main
path: bottlecap/.bench-export/main.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include the hidden benchmark exports in artifacts

Every nightly/full run writes main.json beneath the hidden .bench-export directory, but actions/upload-artifact@v7 excludes files inside dot-directories unless include-hidden-files: true is set, as documented in the action's hidden-files guidance. Consequently this step only warns that no file was found, no bench-baseline-main artifact is created, and subsequent smoke runs can never perform the intended comparison; the exported-numbers upload at line 161 has the same problem. Enable hidden files on both uploads or write exports to a non-hidden directory.

Useful? React with 👍 / 👎.

Comment thread scripts/ci/bench-measure.sh Outdated
Comment on lines +26 to +27
run_id="$(gh run list --workflow bench.yml --branch main --event schedule --status success --limit 1 \
--json databaseId --jq '.[0].databaseId // empty' || true)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back to the latest run that actually has a baseline

When a scheduled benchmark panics or fails to build, the measure step is allowed to fail and the missing-file upload only warns, so that nightly workflow is still classified as successful but has no bench-baseline-main artifact. This command then selects only that newest successful run (--limit 1, per the gh run list options); after its download fails, it does not try an earlier successful nightly that still has a valid artifact, leaving every smoke run without a comparison until the next good nightly. Search backward for the newest run containing the artifact, or prevent an artifact-less nightly from qualifying as the baseline source.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/bench.yml Outdated
Comment on lines +101 to +104
if: >-
github.event_name == 'schedule' ||
github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'bench'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict labeled runs to the bench label

Once a PR already has the bench label, every subsequent labeled event satisfies this condition, even when the newly added label is unrelated. Adding labels such as a team or release label therefore launches another expensive production-profile measurement and, because the concurrency group is shared by the PR ref, can cancel a measurement already in progress. For labeled events, also require github.event.label.name == 'bench' while retaining the current label-presence check for synchronize/reopen events.

Useful? React with 👍 / 👎.

@datadog-prod-us1-6

This comment has been minimized.

@duncanista duncanista changed the title [SVLS-9977] ci: informational benchmark workflow ci: informational benchmark workflow Oct 28, 2026
- Upload the exports with include-hidden-files, so the .bench-export
  directory reaches the artifact.
- download-baseline: walk back through the last 10 green nightlies to the
  newest one that has a bench-baseline-main artifact.
- bench-measure: on a labeled event, run only for the bench label, and give
  other label events their own concurrency group so they do not cancel a
  running measurement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant