[benchamrk] Remove DispatchRule#1064
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the obsolete diskann_benchmark_runner::dispatcher module and replaces most type-matching usage with a simpler AsDataType trait, while relocating MatchScore/FailureScore into diskann_benchmark_runner::benchmark and updating downstream crates accordingly. It continues the cleanup started in PR #865 to make benchmark matching and diagnostics simpler and more caller-driven.
Changes:
- Remove
dispatcher/DispatchRuleinfrastructure and migrateMatchScore/FailureScoreintobenchmark. - Introduce
AsDataType(+Describe) for boolean type matching and human-readable mismatch descriptions. - Update
diskann-benchmarkanddiskann-benchmark-simdto useAsDataType(and a new localAsArchin SIMD) instead of dispatch rules.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-benchmark/src/utils/mod.rs | Adds match_data_type helper; updates imports to new benchmark::{MatchScore, FailureScore} path. |
| diskann-benchmark/src/backend/index/spherical.rs | Replaces DispatchRule/Type<T> matching with AsDataType::is_match checks. |
| diskann-benchmark/src/backend/index/scalar.rs | Migrates type matching + description formatting to AsDataType constants/descriptions. |
| diskann-benchmark/src/backend/index/product.rs | Uses utils::match_data_type and AsDataType::describe for matching/diagnostics. |
| diskann-benchmark/src/backend/index/benchmarks.rs | Updates full-precision benchmarks to new data type matching + description logic. |
| diskann-benchmark/src/backend/filters/benchmark.rs | Updates imports to use benchmark::{MatchScore, FailureScore}. |
| diskann-benchmark/src/backend/exhaustive/spherical.rs | Updates imports to use benchmark::{MatchScore, FailureScore}. |
| diskann-benchmark/src/backend/exhaustive/product.rs | Updates imports to use benchmark::{MatchScore, FailureScore}. |
| diskann-benchmark/src/backend/exhaustive/minmax.rs | Updates imports to use benchmark::{MatchScore, FailureScore}. |
| diskann-benchmark/src/backend/disk_index/benchmarks.rs | Replaces Type<T>: DispatchRule<DataType> constraints with T: AsDataType and new matching/description logic. |
| diskann-benchmark-simd/src/lib.rs | Replaces DispatchRule for data types + arch dispatch with AsDataType and new local AsArch. |
| diskann-benchmark-runner/src/utils/datatype.rs | Removes Type<T>/DispatchRule matching and introduces AsDataType + Describe. |
| diskann-benchmark-runner/src/test/typed.rs | Updates test benchmarks to match on AsDataType and use new description formatting. |
| diskann-benchmark-runner/src/test/dim.rs | Updates imports to new benchmark::{MatchScore, FailureScore} path. |
| diskann-benchmark-runner/src/registry.rs | Updates imports to new benchmark::{MatchScore, FailureScore} path. |
| diskann-benchmark-runner/src/lib.rs | Stops exporting the removed dispatcher module (left as a commented line). |
| diskann-benchmark-runner/src/dispatcher/mod.rs | Deleted (obsolete module removed). |
| diskann-benchmark-runner/src/dispatcher/api.rs | Deleted (obsolete module removed). |
| diskann-benchmark-runner/src/benchmark.rs | Adds MatchScore/FailureScore types directly to benchmark module. |
| diskann-benchmark-runner/src/any.rs | Removes DispatchRule-based matching/conversion helpers from Any. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub struct Describe(DescribeInner); | ||
|
|
||
| impl Describe { | ||
| /// Return `true` is the data type match was successful. |
| pub mod any; | ||
| pub mod app; | ||
| pub mod dispatcher; | ||
| // pub mod dispatcher; |
| @@ -579,7 +555,7 @@ where | |||
| _: diskann_benchmark_runner::Checkpoint<'_>, | |||
| mut output: &mut dyn diskann_benchmark_runner::Output, | |||
| ) -> anyhow::Result<Self::Output> { | |||
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1064 +/- ##
==========================================
- Coverage 89.51% 89.51% -0.01%
==========================================
Files 461 459 -2
Lines 85920 85642 -278
==========================================
- Hits 76914 76659 -255
+ Misses 9006 8983 -23
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Remove the module
diskann_benchmark_runner::dispatcherand all its associated items except forMatchScoreandFailureScore, which get moved todiskann_benchmark_runner::benchmark.The current users of
DispatchRuleare almost universally focused on matching againstDataType. This PR introduces a much simplerAsDataTypetrait as a replacement, removing the need for theTypeauxiliary struct altogether.PR #865 changed how benchmark matching worked and rendered
DispatchRuleobsolete, this continues that clean up in an effort to make the benchmark API friendlier.Suggested Review Order
diskann_benchmark_runner:src/benchmark.rs: New home forMatchScore/FailureScore.src/utils/datatype.rs: NewAsDataTypetrait for matching against theDataTypeenum. Importantly, matching only returnstrue/false, not aMatchScoreorFailureScore. This moves the ranking of matches to the caller rather than baking it in internally.src/any.rs: RemoveDispatchRulerelated inherent methods ofAnyas these are not longer useful. The type-erasure infrastructure inbenchmark.rshas been responsible for proper downcasting and reporting since Cleanupdiskann-benchmark-runnerand friends. #865.diskann_benchmark:src/utils/mod.rs: Since type-based matching is now the caller's responsibility, a smallmatch_data_typehelper is added to replace the current uses ofType<T>: DispatchRule<DataType>.AsDataType.diskann_benchmark_simd: Mainly updating toAsDataType. Architecture dispatch gets a similar treatment toAsDataType, which is simpler now thatAsArchis a local trait allowing us to skip theIdentitynew-type wrapper.