This repository was archived by the owner on May 14, 2026. It is now read-only.
ci(integrated-benchmark): extract compilation step#390
Merged
Conversation
The "Benchmark: Frozen Lockfile" step has been responsible for compiling both revisions for both timed steps, which on a cold Rust build cache edges close to its 10-minute timeout. Add a `--build-only` flag to the integrated-benchmark binary that runs `WorkEnv::build()` (clone, fetch, checkout, `cargo build --release --bin=pacquet`) without `init()`, proxy-cache priming, or hyperfine, and call it from a new "Precompile benchmark revisions" CI step before the two timed steps. Both timed steps now hit a near-no-op incremental build, and the cold- cache compile cost is isolated in its own step with its own timeout.
📝 WalkthroughWalkthroughA "build-only" mode is added to the integrated benchmark CLI tool, allowing pre-compilation of benchmark revisions without running scenarios. The CLI now accepts ChangesBenchmark Build-Only Mode
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #390 +/- ##
=======================================
Coverage 81.03% 81.03%
=======================================
Files 64 64
Lines 3517 3517
=======================================
Hits 2850 2850
Misses 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Micro-Benchmark ResultsLinux |
…d-only Use clap's `required_unless_present` + `conflicts_with` to express "exactly one of `--scenario` or `--build-only` must be given" purely through field-level attributes. This drops the redundant `--scenario=frozen-lockfile` from the precompile CI step and keeps clap as the sole authority on the flag's required-ness — no runtime checks. `WorkEnv.scenario` becomes `Option<BenchmarkScenario>`; `init()` and `benchmark()` unwrap it (they're only reached on the non-`--build-only` path, where clap guarantees `--scenario` is set).
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.3784095015599993,
"stddev": 0.07922768489384564,
"median": 2.39201534466,
"user": 2.48489064,
"system": 3.2769306,
"min": 2.26758791866,
"max": 2.48647895466,
"times": [
2.36649080266,
2.42723610266,
2.35455439266,
2.42781061566,
2.28610906666,
2.46713626466,
2.48647895466,
2.41753988666,
2.26758791866,
2.28315101066
]
},
{
"command": "pacquet@main",
"mean": 2.3612352952599998,
"stddev": 0.051794648534639054,
"median": 2.33063181216,
"user": 2.47458294,
"system": 3.2692129999999997,
"min": 2.31275448866,
"max": 2.44337903866,
"times": [
2.32474612766,
2.36554323266,
2.32995694166,
2.42117714266,
2.32640457266,
2.44337903866,
2.33130668266,
2.31275448866,
2.43518527466,
2.32189945066
]
},
{
"command": "pnpm",
"mean": 6.09743259646,
"stddev": 0.12108845002341861,
"median": 6.05779383316,
"user": 8.82574624,
"system": 4.360614099999999,
"min": 5.94341595866,
"max": 6.28845834166,
"times": [
6.28498093966,
6.05904600566,
6.28845834166,
6.15813594066,
6.02157339166,
5.94341595866,
6.16230849866,
5.98032542666,
6.05654166066,
6.01953980066
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.73834407698,
"stddev": 0.03224447243529431,
"median": 0.73289576658,
"user": 0.25313732,
"system": 1.3883405,
"min": 0.71163747058,
"max": 0.82057827458,
"times": [
0.82057827458,
0.74180064658,
0.74840542158,
0.72741987558,
0.71868768158,
0.74805743358,
0.73837165758,
0.71243761358,
0.71163747058,
0.71604469458
]
},
{
"command": "pacquet@main",
"mean": 0.7750859015800001,
"stddev": 0.07118281722743486,
"median": 0.74089749708,
"user": 0.26308222,
"system": 1.4112679,
"min": 0.7207696985800001,
"max": 0.93698042258,
"times": [
0.82280857958,
0.7207696985800001,
0.7322223235800001,
0.84703883158,
0.73709444358,
0.7447005505800001,
0.73086192258,
0.93698042258,
0.75040806658,
0.72797417658
]
},
{
"command": "pnpm",
"mean": 2.56035258838,
"stddev": 0.09350488261088342,
"median": 2.52866510608,
"user": 2.9988653199999997,
"system": 2.2234581999999996,
"min": 2.39593579158,
"max": 2.68714509358,
"times": [
2.6270820715800003,
2.51221509158,
2.39593579158,
2.66236993258,
2.50895860158,
2.5364396085800003,
2.52089060358,
2.65725730558,
2.49523178358,
2.68714509358
]
}
]
} |
KSXGitHub
commented
May 5, 2026
…y wrapper Address review on #390: - Make `WorkEnv::build` public and call it from `main.rs` directly, instead of routing through a `build_only()` wrapper that did nothing but rename the call. - Strip the redundant doc/code comments that just restated what the surrounding code or clap's `--help` output already says. - Shorten the `--build-only` help text to one line, matching the other flag descriptions.
Address review on #390. The earlier change weakened the program-presence guard for `--build-only` mode on the assumption it's an unrelated entry point. It isn't: CI runs `--build-only` immediately before the benchmark steps in the same job, after every required tool has already been installed, so the check is always a no-op cost-wise. Gating it only delays a missing-tool failure from the start of the precompile step to 15 min later in the benchmark step, which is strictly worse. The verdaccio gating stays — that one is required, since the precompile step doesn't have a registry running yet.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the integrated-benchmark CI workflow to separate “compilation of benchmark revisions” from the timed benchmark runs, reducing the risk that the timed steps hit their 10-minute timeout due to cold Rust caches.
Changes:
- Add a
--build-onlymode to theintegrated-benchmarkbinary to clone/fetch/checkout andcargo build --release --bin=pacquetfor each revision, then exit. - Make
--scenariooptional at the type level and enforce “exactly one of--scenarioor--build-only” via clap attributes (no runtime validation). - Update the GitHub Actions workflow to precompile
HEADandmainin a dedicated step with its own timeout before running the timed hyperfine scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tasks/integrated-benchmark/src/cli_args.rs |
Adds --build-only and enforces mutual exclusivity / requiredness with --scenario using clap attributes. |
tasks/integrated-benchmark/src/main.rs |
Skips registry/verdaccio setup in build-only mode and dispatches to env.build() vs env.run(). |
tasks/integrated-benchmark/src/work_env.rs |
Makes build() public and updates scenario handling to Option, unwrapping only when entering init/benchmark paths. |
.github/workflows/integrated-benchmark.yml |
Adds a precompile step (just integrated-benchmark --build-only HEAD main) ahead of the two timed benchmark steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zkochan
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The "Benchmark: Frozen Lockfile" CI step has been responsible for compiling both revisions (HEAD and
main) for both timed benchmark steps. With the Rust build cache hit it takes ~5 min — half its 10-min timeout. On a cache miss it climbs toward 10 min and edges close to the hard timeout.Change
CI
Add a new "Precompile benchmark revisions" step that runs before both
timed benchmark steps, with its own 15-min timeout:
The two timed steps now fall through to a near-no-op incremental build, so
their timeouts only need to cover hyperfine itself.
integrated-benchmarkbinaryAdd a
--build-onlyflag that runsWorkEnv::build()(clone → fetch →checkout →
cargo build --release --bin=pacquetfor each revision) andexits — no
init(), no proxy-cache priming, no hyperfine.In this mode the binary also skips spawning verdaccio / connecting to a
running registry — the precompile step doesn't have one up yet, and
cargo builddoesn't need a registry. Theensure_programchecks forbash/hyperfine/pnpmstay unconditional: CI installs them allbefore the precompile step runs, so the checks are sub-second no-ops in
practice and keep fail-fast behavior if the environment is misconfigured.
WorkEnv::buildispuband called directly frommain.rs.CLI ergonomics
--scenarioand--build-onlyare mutually exclusive: exactly one mustbe given. Expressed purely through clap field-level attributes — no
runtime validation:
--scenariorequired--scenariocannot be used with--build-only--scenario=…only--build-onlyonlyWorkEnv.scenariobecomesOption<BenchmarkScenario>;init()andbenchmark()unwrap it (only reached on the non---build-onlypath,where clap guarantees
--scenariois set).Files touched
tasks/integrated-benchmark/src/cli_args.rs— new--build-onlyflag;--scenariobecomesOption<…>withrequired_unless_present+conflicts_with.tasks/integrated-benchmark/src/main.rs— gate verdaccio / virtual-registry setup on!build_only; dispatch toenv.build()orenv.run().tasks/integrated-benchmark/src/work_env.rs—build()made public;scenariofield becomes optional;init()andbenchmark()unwrap it at the boundary..github/workflows/integrated-benchmark.yml— newPrecompile benchmark revisionsstep before the two timed steps.Out of scope / not done