Add performance diagnostic to diskann-benchmark#744
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a compile-time performance diagnostic to diskann-benchmark by introducing an orderable “capability level” abstraction in diskann-wide, then using that to warn when the benchmark binary appears to be compiled for an insufficient micro-architecture target (notably x86-64 instead of x86-64-v3+), plus an informational warning on aarch64.
Changes:
- Add
arch::LevelandArchitecture::level()to enable ordering micro-architecture capability without instantiating anArchitecture. - Implement
level()forScalar,x86_64::V3, andx86_64::V4, plus add ordering tests. - Extend
diskann-benchmarkCLI with--quietand emit target-related performance warnings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-wide/src/helpers.rs | Removes dead commented-out cfg_attr block. |
| diskann-wide/src/arch/x86_64/v3/mod.rs | Adds ordering support and exposes V3 capability via level(). |
| diskann-wide/src/arch/x86_64/v4/mod.rs | Adds ordering support and exposes V4 capability via level(). |
| diskann-wide/src/arch/x86_64/mod.rs | Introduces internal level ordering and tests for the ordering. |
| diskann-wide/src/arch/mod.rs | Adds Level type + Architecture::level() API and related docs. |
| diskann-wide/src/arch/emulated/mod.rs | Implements level() for Scalar. |
| diskann-benchmark/src/main.rs | Adds top-level Cli wrapper with --quiet and emits compile-target warnings; updates tests accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (78.84%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #744 +/- ##
==========================================
- Coverage 89.01% 88.99% -0.03%
==========================================
Files 428 428
Lines 78294 78368 +74
==========================================
+ Hits 69692 69742 +50
- Misses 8602 8626 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
## What's Changed ### API Breaking Changes * Remove the `experimental_avx512` feature. by @hildebrandmw in #732 * Use VirtualStorageProvider::new_overlay(test_data_root()) in tests by @Copilot in #726 * save and load max_record_size and leaf_page_size for bftrees by @backurs in #724 * [multi-vector] Verify `Standard` won't overflow in its constructor. by @hildebrandmw in #757 * VirtualStorageProvider: Make new() private, add new_physical by @Copilot in #764 * [minmax] Refactor full query by @arkrishn94 in #770 * Bump diskann-quantization to edition 2024. by @hildebrandmw in #772 ### Additions * [multi-vector] Enable cloning of `Mat` and friends. by @hildebrandmw in #759 * adding bftreepaths in mod.rs by @backurs in #775 * [quantization] Add `as_raw_ptr`. by @hildebrandmw in #774 ### Bug Fixes * Fix `diskann` compilation without default-features and add CI tests. by @hildebrandmw in #722 ### Docs and Comments * Updating the benchmark README to use diskann-benchmark by @bryantower in #709 * Fix doc comment: Windows line endings are \r\n not \n\r by @Copilot in #717 * Fix spelling errors in streaming API documentation by @Copilot in #715 * Add performance diagnostic to `diskann-benchmark` by @hildebrandmw in #744 * Add agents.md onboarding guide for coding agents by @Copilot in #765 * [doc] Fix lots of little typos in `diskann-wide` by @hildebrandmw in #771 ### Performance * [diskann-wide] Optimize `load_simd_first` for 8-bit and 16-bit element types. by @hildebrandmw in #747 ### Dependencies * Bump bytes from 1.11.0 to 1.11.1 by @dependabot[bot] in #723 * [diskann] Add note on the selection of `PruneKind` in `graph::config::Builder`. by @hildebrandmw in #734 * [diskann-providers] Remove the LRU dependency and make `vfs` and `serde_json` optional. by @hildebrandmw in #733 ### Infrastructure * Add initial QEMU tests for `diskann-wide`. by @hildebrandmw in #719 * [CI] Skip coverage for Dependabot. by @hildebrandmw in #725 * Add miri test coverage to CI workflow by @Copilot in #729 * [CI] Add minimal ARM checks by @hildebrandmw in #745 * Enable CodeQL security analysis by @Copilot in #754 ## New Contributors * @backurs made their first contribution in #724 * @arkrishn94 made their first contribution in #770 **Full Changelog**: 0.45.0...0.46.0
Add a warning to
diskann-benchmarkif it looks like the binary was compiled for thex86-64target CPU instead ofx86-64-v3,x86-64-v4, or some other at-least AVX2 compatible CPU.Also add a warning for
aarch64noting that full SIMD support is not available yet.With
diskann-benchmarkbeing available now oncrates.io, I just know that this will be compiled and run with the defaultx86-64target CPU, when the preferred target is strongly at leastx86-64-v3.Implementation
Our architecture detection/selection is performed by
diskann-wide. As such, we can detect if the whole application has been compiled for anx86-64-v3compatible target by checking if the compile time architecturediskann_wide::arch::Currentis at leastdiskann_wide::arch::x86_64::V3. To do this comparison, a light-weight, opaquediskann_wide::arch::Levelis introduced to allowArchitectures to be ordered without instantiating the associated architecture via theArchitecture::levelassociated function.Checks for compatibility can then be done using
I think it's reasonable to add this to
diskann-widesincediskann-benchmarkis not the only crate that may need this functionality anddiskann-wideis really the source of truth when it comes to micro-architecture specific functionality.