Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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:
|
Uh oh!
There was an error while loading. Please reload this page.
## 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.