Jegao/mutiple filtered test - #1319
Conversation
Adds a production-style, query-time filter-match path for multihop search and the benchmark/report artifacts from the multiple-filter evaluation. diskann-label-filter: - New live_filter module: InlineAttributeIndex (builder) -> FrozenAttributeIndex exposing a QueryLabelProvider whose is_match evaluates an AND/OR (+equality) predicate live, per visited node, against the node's roaring attribute-id set (lock-free read, integer-encoded terminals, no FFI, no global posting list). NOT/relational operators are rejected; evaluation errors are treated as non-match. Includes unit tests. diskann-benchmark: - New search type 'topk-multihop-live-filter' (plugin + SearchPhase wiring) that builds the attribute index once from the label file and evaluates the filter live during search, so reported latency includes the real per-node filter cost. - filters.rs helpers: build_inline_attribute_index / make_live_providers. diskann-benchmark-core / diskann-benchmark-runner: - Add p99.9 latency percentile to the search output (percentiles, knn Summary, result DTO + console table). Adds diskann-benchmark/multiple_filter_test_report.md documenting the 9-case multihop vs beta vs live-filter comparison (recall + latency incl. p90/p99/p99.9). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds InlineAttributeIndexCsr/FrozenAttributeIndexCsr, an alternative to the roaring InlineAttributeIndex that stores each node's sorted attribute-ids in a flat CSR layout (offsets: Vec<u32> + values: Vec<u32>). is_match reads one contiguous row and answers each equality terminal with a binary_search, avoiding the HashMap probe and RoaringTreemap heap pointer-chases of the roaring path. Reuses AttributeEncoder/EncodedFilterExpr so predicate parsing, semantics, and errors are identical. Exported from lib.rs; includes parity unit tests asserting CSR == roaring per node. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19cdd8e8-cc28-4b87-af2b-97a3f1ac9488
New criterion bench (live_filter_bench) comparing the per-node is_match cost of three representations of the same corpus/predicate: roaring treemap (current), flat CSR, and per-attribute posting bitmaps. All three are asserted to agree on every probe before timing. Tunable via LF_BENCH_N / LF_BENCH_PROBES. Run: cargo bench -p diskann-label-filter --bench main -- live_filter. Also repairs evaluator_bench, which used the removed \\\ operator and panicked at setup, blocking the shared bench binary; swapped for equivalent exact-array \\\ clauses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19cdd8e8-cc28-4b87-af2b-97a3f1ac9488
New multihop live-filter search type backed by the flat-CSR attribute index instead of the roaring HashMap<u32, RoaringTreemap>. Identical traversal and recall to topk-multihop-live-filter; only the per-node is_match differs. Wires the SearchPhase variant (graph_index.rs), the plugin (plugins.rs), registration + Plugin impl (benchmarks.rs), and build_inline_attribute_index_csr / make_live_providers_csr (filters.rs). End-to-end on the 10M index (k=150, L=150, 1 thread): 3.4x (S1) - 4.4x (S4) lower mean/p99 latency at equal recall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19cdd8e8-cc28-4b87-af2b-97a3f1ac9488
New section 8.1 documenting the is_match microbenchmark (roaring 14-20x slower than flat CSR) and the end-to-end A/B of topk-multihop-live-filter vs -csr (3.4-4.4x lower latency at equal recall), plus CSR artifacts in section 9. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19cdd8e8-cc28-4b87-af2b-97a3f1ac9488
bin_cast: widen a DiskANN .bin file's element type (u8/i8/f16 -> f32). tsv_to_diskann_bin: convert a label-tagged TSV of embeddings into DiskANN bin + JSONL label/predicate files for compute_groundtruth and the filtered-search benchmark. Both are standalone CLIs used to prepare the multiple-filter test data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19cdd8e8-cc28-4b87-af2b-97a3f1ac9488
Add posting-list materialization, selectivity-adaptive CSR/bitmap selection, and bit-sliced lookup providers with parity and boundary tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expose bitmap, adaptive, and bit-sliced multihop search modes. Recreate lazy providers for every repetition and search-L run so materialization remains part of measured query latency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the posting bitmap crossover and the adaptive and bit-sliced results, including latency, selectivity, and memory tradeoffs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (67.01%) 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 #1319 +/- ##
==========================================
- Coverage 91.26% 90.53% -0.74%
==========================================
Files 517 521 +4
Lines 98511 101946 +3435
==========================================
+ Hits 89910 92298 +2388
- Misses 8601 9648 +1047
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
If this is intended to merge to main rather than being speculative/experimental changes, let me know and I can start a preliminary review. |
Keep concrete QueryLabelProvider types through filtered search and encoded multihop benchmarks while preserving trait-object defaults. Document the exact-parity S1-S9 performance results. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Persist frequent labels as dense bit slices and the tail as contiguous sorted postings. Add provider/language TSV build and probe tools, representation-aware query ordering, validation coverage, and the 100M-row trial results. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Add UINT8 hybrid-filter benchmark wiring plus tools to prepare deduplicated provider-and-language queries, map variable-length truth IDs, and validate truth/filter consistency. Record the 100M recall and latency sweep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Add the 100M storage and K=150/L=150 latency comparison for the static 10K dense threshold, plus the query-aware promotion recommendation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Persist per-query latency arrays in benchmark JSON and document the 100M indexed BruteForceKNN hybrid comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Record the dense-subset correctness checks, SIMD clause-count cost, and planned exact-scan fallback below 1% selectivity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e92a7a2-b7bb-4374-8a2d-b6b84bb3a71b
Keep the first reviewable implementation focused on dense Bitslice storage and flat DNF/CNF queries. Remove comparison-only AST/bitmap paths and hybrid implementation while retaining deferred hybrid design findings in the benchmark reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep only the encoded dense Bitslice-DNF benchmark added by this work. Remove the live, CSR, bitmap, auto, recursive Bitslice, and inline comparison modes together with their unused live-filter implementation and microbenchmark, while retaining historical results in the reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the standalone BIN casting and TSV conversion utilities out of the multiple-filter change because they are test-data preparation tools rather than part of the reviewed implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split label-index building, format handling, query serving, errors, and tests into focused modules without changing the public API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new encoded-label benchmark query translation can generate invalid label tokens (e.g., containing &/|), which can alter DNF semantics and should be explicitly rejected/validated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends DiskANN’s filtered-search benchmarking stack with an encoded dense “bitslice” label-index and a flat DNF query representation, and updates the multihop filtered-search path/benchmarks to exercise it with lower per-node overhead (including enabling static dispatch for concrete label providers).
Changes:
- Add the new
diskann-label-indexcrate for persisted dense bitslice label storage plus DNF/CNF query compilation/evaluation. - Add a new benchmark search phase/plugin for multihop search using the encoded bitslice DNF path, including fresh-per-rep multihop runners for lazy per-query provider state.
- Extend benchmark reporting to include p99.9 (p999) latency metrics and reduce some allocation in multihop expansion.
File summaries
| File | Description |
|---|---|
| diskann/src/graph/search/multihop_filter_search.rs | Avoids allocating an intermediate Vec of two-hop IDs; preserves hop accounting. |
| diskann/src/graph/ext/labeled.rs | Makes Filtered/FilteredAccessor generic over the label provider type for static dispatch. |
| diskann-label-index/src/lib.rs | New crate entrypoint and public re-exports for the encoded label index. |
| diskann-label-index/src/index.rs | Dense bitslice index loader and compiled DNF/CNF query evaluator. |
| diskann-label-index/src/format.rs | Wire-format constants plus validation and read/write helpers. |
| diskann-label-index/src/error.rs | Error type for encoding/loading/querying label indexes. |
| diskann-label-index/src/builder.rs | JSONL encoder that builds and persists the dense bitslice label index. |
| diskann-label-index/src/tests.rs | New unit tests covering round-trips, query semantics, and invalid inputs. |
| diskann-label-index/Cargo.toml | Adds the new crate to the workspace with serde_json/thiserror (+ tempfile for tests). |
| diskann-label-filter/benches/benchmarks/evaluator_bench.rs | Updates benchmark filters to avoid removed $in usage. |
| diskann-benchmark/src/utils/filters.rs | Adds lazy encoded-query provider + DNF flattening for encoded bitslice benchmarks. |
| diskann-benchmark/src/inputs/graph_index.rs | Adds a new topk-multihop-encoded-bitslice-dnf search phase kind. |
| diskann-benchmark/src/index/search/plugins.rs | Registers a new plugin kind for the encoded bitslice DNF multihop benchmark mode. |
| diskann-benchmark/src/index/search/knn.rs | Adds run_fresh_multihop to rebuild the runner each repetition/search-L. |
| diskann-benchmark/src/index/result.rs | Adds p90 + p99.9 columns/fields to displayed/search result summaries. |
| diskann-benchmark/src/index/benchmarks.rs | Wires up the new encoded bitslice DNF multihop benchmark execution path. |
| diskann-benchmark/multiple_filter_test_report.md | Adds the multi-filter search performance report artifact. |
| diskann-benchmark/multiple_filter_optimization_summary.md | Adds the detailed optimization summary artifact for the approach. |
| diskann-benchmark/Cargo.toml | Removes roaring dependency and adds diskann-label-index. |
| diskann-benchmark-runner/src/utils/percentiles.rs | Extends percentile computation to include p99.9 (p999). |
| diskann-benchmark-core/src/search/graph/multihop.rs | Makes benchmark-core MultiHop generic over label provider type for static dispatch. |
| diskann-benchmark-core/src/search/graph/knn.rs | Adds p999_latencies to the aggregated KNN summary output. |
| Cargo.toml | Adds diskann-label-index to the workspace members and workspace dependencies. |
| Cargo.lock | Updates lockfile for the new crate/dependency graph adjustments. |
Review details
- Files reviewed: 23/24 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| fn eq_value_to_label(field: &str, value: &Value) -> anyhow::Result<String> { | ||
| match value { | ||
| Value::Bool(true) => Ok(field.to_string()), | ||
| Value::Bool(false) | Value::Number(_) | Value::String(_) => { | ||
| Ok(format!("{field}={}", value_repr(value))) | ||
| } | ||
| Value::Null => Err(anyhow::anyhow!( | ||
| "encoded benchmark equality predicates do not support null for field '{field}'" | ||
| )), | ||
| Value::Array(_) => Err(anyhow::anyhow!( | ||
| "encoded label-index equality predicates do not support array values for field '{field}'" | ||
| )), | ||
| Value::Object(_) => Err(anyhow::anyhow!( | ||
| "encoded label-index equality predicates require scalar or array values; field '{field}' used an object" | ||
| )), | ||
| } | ||
| } |
| /// The 99.9th percentile latency for individual queries. | ||
| /// | ||
| /// This contains one entry per repetition in the batch. | ||
| pub p999_latencies: Vec<MicroSeconds>, | ||
|
|
| fn compare_to_label(field: &str, op: &CompareOp) -> anyhow::Result<String> { | ||
| match op { | ||
| CompareOp::Eq(value) => eq_value_to_label(field, value), | ||
| CompareOp::Ne(_) | ||
| | CompareOp::Lt(_) | ||
| | CompareOp::Lte(_) | ||
| | CompareOp::Gt(_) | ||
| | CompareOp::Gte(_) => Err(anyhow::anyhow!( | ||
| "encoded label-index queries only support equality/set-membership predicates; field '{field}' used unsupported operator {op}" | ||
| )), | ||
| } | ||
| } |
|
Thanks for the PR. Can you please clarify the intention? Is this intended for merging, meant as a reference, or something else? |
Reference Issues/PRs
What does this implement/fix? Briefly explain your changes.
Any other comments?