Repository navigation
[compat] Remove callgrind benchmarks - #1513
Open
Mark Hildebrand (hildebrandmw) wants to merge 1 commit into
Open
Mark Hildebrand (hildebrandmw) wants to merge 1 commit into
Mark Hildebrand (hildebrandmw) wants to merge 1 commit into
Conversation
Mark Hildebrand (hildebrandmw)
requested review from
a team
and
a balanced review from Copilot
October 26, 2026 16:35
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The benchmark deletions and dependency cleanup are consistent, preserve Criterion targets, and leave no identified blocking issues.
No open issues 🎉
What changed in this PR
Removes Callgrind-based benchmarks from DiskANN to eliminate the dependency chain causing Rust future-compatibility warnings, while preserving existing Criterion benchmarks.
Changes:
- Deletes Callgrind benchmark suites and entry points from
diskann-vectoranddiskann-disk. - Removes their benchmark targets and
iai-callgrinddependency declarations. - Prunes unused Callgrind-related packages from the lockfile.
| File | Description |
|---|---|
diskann-vector/Cargo.toml |
Removes the Callgrind dependency and benchmark target. |
diskann-vector/benches/benchmarks_iai/mod.rs |
Deletes Callgrind module declarations. |
diskann-vector/benches/benchmarks_iai/l2_iai.rs |
Deletes squared-L2 Callgrind benchmarks. |
diskann-vector/benches/benchmarks_iai/cosine_normalized_iai.rs |
Deletes normalized-cosine Callgrind benchmarks. |
diskann-vector/benches/benchmarks_iai/cosine_iai.rs |
Deletes cosine Callgrind benchmarks. |
diskann-vector/benches/benchmarks_iai/contains_bench_iai.rs |
Deletes SIMD containment Callgrind benchmarks. |
diskann-vector/benches/bench_main_vector_iai.rs |
Deletes the vector Callgrind entry point. |
diskann-disk/Cargo.toml |
Removes the Callgrind dependency and benchmark target. |
diskann-disk/benches/benchmarks_iai/mod.rs |
Deletes Callgrind module declarations. |
diskann-disk/benches/benchmarks_iai/kmeans_bench_iai.rs |
Deletes clustering and vector-norm Callgrind benchmarks. |
diskann-disk/benches/benchmarks_iai/aligned_file_reader_bench_iai.rs |
Deletes the aligned-reader Callgrind benchmark. |
diskann-disk/benches/bench_main_iai.rs |
Deletes the disk Callgrind entry point. |
Cargo.toml |
Removes the workspace Callgrind dependency. |
Cargo.lock |
Removes Callgrind packages and unused transitive dependencies. |
Review effort: Balanced
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1513 +/- ##
=======================================
Coverage 92.16% 92.16%
=======================================
Files 592 592
Lines 123582 123582
=======================================
+ Hits 113899 113900 +1
+ Misses 9683 9682 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Aditya Krishnan (arkrishn94)
approved these changes
Oct 26, 2026
Alex Razumov (arrayka)
left a comment
Contributor
There was a problem hiding this comment.
Please let me take a look at the migration path today. I find the IAI approach useful for catching performance regressions in performance-critical code in an unstable CI environment. Unfortunately, we haven't enabled iai benchmarks in CI yet due to lack of prioritization :(
Wei Wu (wuw92)
approved these changes
Oct 27, 2026
Alex Razumov (arrayka)
added a commit
that referenced
this pull request
Oct 29, 2026
#### Reference Issues/PRs <!-- Example: Fixes #1234. See also #3456. Please use keywords (e.g., Fixes) to create link to the issues or pull requests you resolved, so that they will automatically be closed when your pull request is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests --> Alternative proposal to #1513: retain the benchmarks by migrating to the maintained successor rather than removing them. ### Why migrate Callgrind benchmarks from iai-callgrind to Gungraun? `iai-callgrind` pulls in `proc-macro-error2 v2.0.1`, which triggers a Rust future-incompatibility warning: ``` cargo clippy --locked --workspace --all-targets --no-deps --config 'build.rustflags=["-Dwarnings"]' -- -Dwarnings warning: the following packages contain code that will be rejected by a future version of Rust: proc-macro-error2 v2.0.1 ``` ### Why not delete the Iai-Callgrind benchmarks? Callgrind benchmarks detect performance regressions using instruction counts and cache metrics that are less sensitive to CI noise than wall-clock timings. Migrating to Gungraun preserves them for local comparisons and future CI integration while removing the unmaintained dependency. ### What Migrate to its successor, Gungraun, to remove that dependency chain while preserving the benchmarks: - Replace `iai-callgrind` with `gungraun 0.20.0`, update the lockfile, and rename benchmark files, targets, modules, and identifiers. Keep benchmark workloads, metrics, and 5% regression limits unchanged. ### Before/After comparison: | Benchmark command | Result | | --- | --- | | Before: `cargo bench -p diskann-vector --bench bench_main_vector_iai` | Passed: 17 benchmarks, 0 regressions; 8.68166s | | After: `cargo bench -p diskann-vector --bench bench_main_vector_gungraun` | Passed: 17 benchmarks, 0 regressions, 0 filtered; 9.70543s | | Before: `cargo bench -p diskann-disk --bench bench_main_iai` | Passed: 3 benchmarks, 0 regressions; 95.8849s | | After: `cargo bench -p diskann-disk --bench bench_main_gungraun` | Passed: 3 benchmarks, 0 regressions, 0 filtered; 96.6773s | ## What **dev** dependencies does the transition add to diskann-disk and diskann-vector? Only **dev** dependencies have changed: ``` Added (11): bincode-next 3.1.1 either-or-both 0.3.1 pastey 0.2.3 rapidhash 4.5.1 unty-next 0.1.2 gungraun 0.20.0 gungraun-common 0.1.0 gungraun-macros 0.10.0 gungraun-runner 0.20.0 proc-macro-error-attr3 3.1.1 proc-macro-error3 3.1.1 Removed (5): iai-callgrind 0.16.1 iai-callgrind-macros 0.6.1 iai-callgrind-runner 0.16.1 proc-macro-error-attr2 2.0.0 proc-macro-error2 2.0.1 ``` The following are median **elapsed Cargo unit times**, including compilation and associated build-script units: | Dependency transition | Before | After | |---|---:|---:| | `iai-callgrind` → `gungraun` | 2.62 s | 3.15 s | | `iai-callgrind-macros` → `gungraun-macros` | 1.38 s | 2.38 s | | `iai-callgrind-runner` → `gungraun-runner` | 2.48 s | 2.41 s | | `proc-macro-error-attr2` → `proc-macro-error-attr3` | 0.70 s | 0.69 s | | `proc-macro-error2` → `proc-macro-error3` | 0.21 s | 0.22 s | | `bincode-next` | — | 2.29 s | | `either-or-both` | — | 0.43 s | | `gungraun-common` | — | 0.81 s | | `pastey` | — | 0.84 s | | `rapidhash` | — | 0.33 s | | `unty-next` | — | 0.04 s | ### Did the transition increase compilation times? - **`cargo build`:** No additional dependencies or generated library code. - **`cargo test`:** Little change in the measured runs, though only one sample per configuration was collected. Stripped test executable sizes were unchanged for vector and increased by 0.043% for disk. - **Benchmark compilation:** Across five clean builds per configuration, median compilation time increased by approximately **11% for `diskann-vector`** and **3% for `diskann-disk`**, using the specific Callgrind/Gungraun benchmark targets.
This branch has not been deployed
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
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.
This PR removes
iai-callgrindbased benchmarks.Why
We're receiving the following future incompatibility warning now:
The offending crate proc-macro-error-2 is unmaintained. Additionally,
iai-callgrindsimilarly appears to be unmaintained. It's repository now links to what appears to be a successor.Point being: we cannot continue to use
iai-callgrind.Alternatives
I've generally been dubious of the utility of these benchmarks and so have not looked at all into what it would take to port to the successor. If someone is getting value from these and is willing to maintain these benchmarks, please open an alternate PR.