Repository navigation
Commit Cargo.lock so benchmarks aren't subject to dependency drift - #265
Merged
Merged
Conversation
The bench job resolved dependencies fresh on every run, so a benchmark comparison spanned whatever dependency releases happened between the base run and the head run. On #264 that showed up as three regressions caused by num-bigint moving 0.4.6 -> 0.4.8, not by the diff. Committing the root lockfile fixes the dependency graph for benchmarks. The lockfile is a fresh resolve, so it pins num-bigint 0.4.8, the version `cargo add jiter` resolves today; benchmarks re-baseline once against it rather than measuring a version no user gets. test-latest-deps runs `cargo update` before `cargo test`, so a breaking dependency release still fails CI. The lint job now runs `cargo metadata --locked`, which fails when Cargo.toml changes without refreshing the lockfile. crates/jiter-python/Cargo.lock stays ignored; it is a maturin artifact and jiter-python is a workspace member.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 15.66%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| 👁 | massive_ints_array_jiter_iter |
564.1 µs | 685 µs | -17.65% |
| 👁 | massive_ints_array_jiter_value |
743 µs | 849.6 µs | -12.55% |
| 👁 | python_parse_massive_ints_array |
1.1 ms | 1.4 ms | -16.71% |
Comparing commit-cargo-lock (b4d5802) with main (785cf75)
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
`cargo metadata --locked` ran after `cargo doc`, and after pre-commit's cargo fmt/clippy/check hooks, any of which regenerates a stale lockfile first, so the check passed regardless. Placing it before all of them is possible but not worth the ordering constraint. test-latest-deps still covers dependency breakage.
The `**/Cargo.lock` plus `!/Cargo.lock` pair only existed to carve out the root lockfile. Dropping both is equivalent: the lockfiles under profiling/, scratch/ and scratch-project/ are already covered by the directory rules above.
Both produce a byte-identical lockfile here, but deleting it is what happens for anyone adding jiter to their project, so it says what the job is for without the reader having to know cargo update's semantics.
The nightly test job failed in `cargo llvm-cov` with "failed to collect object files ... target/llvm-cov-target/debug". rustc 1.100.0-nightly (2026-08-17) uses Cargo's new build-dir layout, which cargo-llvm-cov 0.8.5 does not know about, so it looks for object files where there are none. 0.9.0 handles the new layout. install-action resolves `cargo-llvm-cov` from a manifest baked into its release: v2.71.3 (April) pins latest to 0.8.5, v2.86.3 pins it to 0.9.0. Reproduced on rustc 1.100.0-nightly (8fa1c96cf 2026-08-17): 0.8.5 fails with that error, 0.9.0 writes a valid codecov.json. This break is not from committing Cargo.lock; it also fails on main and on #264.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The bench job resolves dependencies fresh on every run, so a CodSpeed comparison spans whatever dependency releases happened between the base run and the head run. On #264 that produced three regressions caused by
num-bigintmoving 0.4.6 -> 0.4.8 between a June base run and an August head run, rather than by anything in the diff.Committing the root
Cargo.lockfixes the dependency graph, so a benchmark comparison only reflects the diff.Expect one re-baseline on this PR. The committed lockfile is a fresh resolve, so it pins
num-bigint0.4.8. The working tree's old lockfile pinned 0.4.6, but lockfiles don't apply to dependents, so 0.4.8 is whatcargo add jiterresolves today; benchmarking 0.4.6 would measure a version no user gets. CodSpeed should showmassive_ints_array_jiter_iterandmassive_ints_array_jiter_valueregressing once, then stay stable.That regression is real and upstream:
BigDigits::pushallocates capacity exactly 2 when aBigUintspills to the heap, where pre-0.4.7 it got 4 fromVec::push. Fix proposed in rust-num/num-bigint#355; when it lands we bump and get the time back.test-latest-depsdeletesCargo.lockbeforecargo test, so dependencies resolve fresh the way they do for anyone adding jiter to their project, and a breaking dependency release still fails CI. It's in thecheckgate.crates/jiter-python/Cargo.lockstays ignored: it's a maturin build artifact, and jiter-python is a workspace member covered by the root lockfile.