Support u64 vertex ids in the bftree provider by JordanMaples · Pull Request #1216 · microsoft/DiskANN

JordanMaples · 2026-06-30T21:28:51Z

What

Make the bf-tree provider generic over its vertex-id type so it can index past
the u32::MAX (~4.3B) ceiling, while keeping u32 as the default (no memory or
storage cost for existing datasets). Also add defensive hardening, wide-id test
coverage, and a benchmark-layer path to validate the u64 id space against
real-world datasets.

New id.rs — BfTreeId trait with u32 / u64 impls, from_index /
try_from_index, and validate_id_capacity.
provider.rs — BfTreeProvider<T, Q = QuantVectorProvider, I = u32>.
The vertex id type threads through construction, save, and load. The persisted
SavedParams records id_width; load validates it against the requested type
so a u32 index can't be silently reopened as u64 (or vice versa).
neighbors.rs / vectors.rs — neighbor cells and starting-point seeding
are generic over I, sized via size_of::().

Why

Larger-than-memory, billion-scale datasets need vertex ids wider than u32.
Defaulting I = u32 keeps the common case unchanged; opting into u64 is a
single type parameter at the call site.

Hardening

Defensive coverage for the wide-id path:

Capacity guard rejecting id counts that overflow the chosen width.
Save/load rejects an id_width mismatch instead of misreading data.
Tests for high-bit (> u32::MAX) ids and their low-bit collisions, plus a
u64 save/load round-trip.

Wide-id tests + 32-bit compile guard

The earlier u64-typed tests only used values in 0..N, which still fit in
u32 and would pass even under silent 32-bit truncation. Added tests that
actually exceed u32::MAX:

test_quantized_index_search_u64_high_ids — end-to-end quantized insert +
search with ids above u32::MAX, using a sparse max_points so no
billion-scale allocation is needed.
test_u64_id_recall_parity_on_transposed_dataset — builds a random dataset
twice (dense u32 identity vs an order-preserving u64 offset above
u32::MAX) and asserts identical per-query neighbor sets plus a recall floor
against brute-force ground truth.

Added BfTreeId::INDEX_CONVERSION_LOSSLESS, overridden for u64 to assert a
64-bit usize and referenced from the monomorphized validate_id_capacity.
This turns as_index truncation into a compile error only for u64 providers
on 32-bit targets, leaving the default u32 path (even on 32-bit) unaffected.

Real-world `u64` validation (benchmark layer)

Real datasets ship as a dense 0..N block whose ids always fit in u32, so
they never exercise the u64 paths, and reaching an id > u32::MAX
positionally would need billions of rows. Added an opt-in, validation-only
benchmark path that transposes a dataset's dense u32 row ids into the
u64-only range (base > u32::MAX) without touching production code:

diskann-benchmark-core: an Offset ToId<u64> mapper (base + i, checked
add), documented as validation-only.
diskann-benchmark: a single_or_multi_insert_offset helper, a
U64IdValidation { offset } JSON config on BfTreeFullPrecisionBuild
(#[serde(default)], non-breaking), a widened bftree_parameters_offset, and
a dedicated run_u64_id_validation orchestration that builds a
BfTreeProvider<T, NoStore, u64>, inserts with the offset, and widens
ground-truth ids via Matrix::map to keep recall aligned. This reuses the
already-generic provider/search/recall stacks rather than relaxing the shared
u32 build pipeline.
Example config + an end-to-end integration test asserting the transposed
u64 run reproduces the identity-u32 recall baseline.

Validated on the 100K Wikipedia/Cohere dataset (offset 2^32, so every vertex
id lives above u32::MAX): recall@100 0.9536 (u32) vs 0.9537 (u64) — parity
confirmed (the sub-1e-4 delta is nondeterministic 8-thread multi-insert batch
ordering, not an id-path difference).

Deferred: the streaming benchmark is not yet transposable — its shared
executor is u32-pinned (the TagSlotManager slot allocator, the
ManagedStream trait, the Managed translation layer, and BfTreeStream all
hardcode u32), so a transposed stream run needs a broader genericization of
that shared infrastructure. Tracked as follow-up.

Notes

Each commit builds and lints clean in isolation (clean bisect).
Open design question: bf-tree's I is an internal id. The inmem2
direction uses InternalId = u32 (hardcoded) + generic external id. Worth
aligning on whether bftree needs wide internal ids or should follow the same
internal-u32 / external-generic split before merge.

Copilot

Pull request overview

This PR generalizes the diskann-bftree provider to support u64 vertex IDs (while keeping u32 as the default), enabling bf-tree indexes to exceed the u32::MAX vertex ceiling. It threads a new BfTreeId abstraction through the provider, neighbor storage, vector starting-point logic, and save/load metadata, and adds tests that exercise the wide-id path end-to-end.

Changes:

Introduces BfTreeId (u32/u64) and adds capacity validation to prevent silent truncation when minting IDs from indices.
Makes BfTreeProvider and related accessors/strategies generic over vertex-id type I, including neighbor record sizing via size_of::().
Persists id_width in SavedParams and validates width/capacity on load; adds tests for u64 high-bit IDs and u64 save/load round-trips.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
diskann-bftree/src/id.rs	Adds `BfTreeId` trait + capacity validation helper for index-derived IDs.
diskann-bftree/src/lib.rs	Exposes the new `id` module and re-exports `BfTreeId`.
diskann-bftree/src/vectors.rs	Makes starting-point ID generation generic over `BfTreeId`.
diskann-bftree/src/neighbors.rs	Sizes bf-tree neighbor records based on `I` width; adds `u64` ID collision/round-trip test.
diskann-bftree/src/provider.rs	Makes `BfTreeProvider` generic over `I`; adds save/load `id_width` validation and `u64` path tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-06-30T21:44:07Z

Codecov Report

❌ Patch coverage is 93.48172% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.50%. Comparing base (45d8359) to head (01ea329).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
diskann-bftree/src/provider.rs	94.74%	26 Missing ⚠️
diskann-bftree/src/id.rs	75.00%	13 Missing ⚠️
diskann-bftree/src/vectors.rs	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1216      +/-   ##
==========================================
+ Coverage   90.48%   90.50%   +0.02%     
==========================================
  Files         497      498       +1     
  Lines       95489    96065     +576     
==========================================
+ Hits        86399    86945     +546     
- Misses       9090     9120      +30

Flag	Coverage Δ
miri	`90.50% <93.48%> (+0.02%)`	⬆️
unittests	`90.17% <93.48%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
diskann-benchmark-core/src/build/ids.rs	`97.54% <100.00%> (+0.57%)`	⬆️
diskann-benchmark/src/index/build.rs	`86.98% <100.00%> (+1.27%)`	⬆️
diskann-benchmark/src/main.rs	`91.64% <ø> (ø)`
diskann-bftree/src/lib.rs	`59.09% <ø> (ø)`
diskann-bftree/src/neighbors.rs	`97.08% <100.00%> (+0.13%)`	⬆️
diskann-bftree/src/vectors.rs	`90.05% <50.00%> (-0.48%)`	⬇️
diskann-bftree/src/id.rs	`75.00% <75.00%> (ø)`
diskann-bftree/src/provider.rs	`91.75% <94.74%> (+1.32%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

harsha-simhadri · 2026-06-30T22:01:13Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

JordanMaples · 2026-07-01T16:57:42Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

@harsha-simhadri yes, in u64 mode it doubles the adjacency-list storage (neighbor ids are stored inline), but that's explicit opt-in and there are no u64 consumers yet. I can tackle delta compression as a follow-up. Neighbor order doesn't affect search correctness, so we can sort ids and delta+varint them, which recovers essentially all of the u64 overhead.

harsha-simhadri · 2026-07-01T17:44:20Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

@harsha-simhadri yes, in u64 mode it doubles the adjacency-list storage (neighbor ids are stored inline), but that's explicit opt-in and there are no u64 consumers yet. I can tackle delta compression as a follow-up. Neighbor order doesn't affect search correctness, so we can sort ids and delta+varint them, which recovers essentially all of the u64 overhead.

thanks, could you create an issue for future follow up. thanks

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

harsha-simhadri · 2026-07-13T20:11:54Z

Coud you please link the follow up issues on delta encoding for u64 in bf-tree
Share the testing done for u64 id type, and the results.

JordanMaples · 2026-07-13T20:52:07Z

@harsha-simhadri

Coud you please link the follow up issues on delta encoding for u64 in bf-tree

The follow up issue is already linked above your comment.
#1234

JordanMaples · 2026-07-13T21:03:12Z

@harsha-simhadri
As far as tests for u64 go, I added 3 unit tests in this PR but haven't run against a real-word dataset.

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The provider hardcoded `u32` vertex ids, capping any bf-tree-backed index at ~4.29B vectors. That ceiling is at odds with the crate's purpose of supporting larger-than-memory, billion-scale-and-beyond datasets. Introduce a `BfTreeId` trait (`VectorId + IntoUsize` plus index-construction) implemented for `u32` and `u64`, and thread a new id type parameter `I` through `BfTreeProvider<T, Q, I = u32>` and all of its accessors, strategies, and save/load impls. The default `I = u32` keeps every existing caller (benchmark harness, tests) compiling and behaving identically, while `u64` is now available opt-in for indexes that exceed the 32-bit range. `iter()`/`IntoIterator` now map `0..total` through `I::from_index` instead of relying on `Range<u32>` (a generic `Range` would require the unstable `Step` trait). `VectorProvider::starting_points` is likewise generic over the id type. The on-disk neighbor format follows the id width and is self- consistent on load (the width is not persisted, mirroring the metric). A new `test_quantized_index_search_u64_ids` builds and searches a `BfTreeProvider<_, _, u64>` index end-to-end to prove the u64 path is functional, not merely compilable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Reinforce the newly-reachable u64 id path surfaced by review: - neighbors: size record key/value buffers by size_of::() instead of hardcoded size_of::<u32>(), so u64 providers reserve correct record sizes. - id: add validate_id_capacity to reject vertex counts that overflow the id type, and guard new_empty with it. - provider: persist id_width in SavedParams (back-compat default of 4) and validate it on load, so a u64 index can't be silently loaded as u32. - tests: add u64 high-bit id coverage (ids/neighbors > u32::MAX, low-bit collision check) plus a u64 save/load round-trip, id-width mismatch rejection, and capacity-guard rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add two tests exercising bf-tree vertex ids beyond u32::MAX: - test_quantized_index_search_u64_high_ids: end-to-end quantized insert + search with ids above u32::MAX, using a sparse max_points so no billion-scale allocation is needed. - test_u64_id_recall_parity_on_transposed_dataset: builds a random dataset twice (dense u32 identity vs order-preserving u64 offset above u32::MAX) and asserts identical neighbor sets per query plus a recall floor against brute-force ground truth. Existing u64 tests only used the u64 type with values in 0..N, which still fit in u32 and would pass even under silent 32-bit truncation. Also add a BfTreeId::INDEX_CONVERSION_LOSSLESS associated const, overridden for u64 to assert a 64-bit usize and referenced from the monomorphized validate_id_capacity. This turns the as_index truncation into a compile error only for u64 providers on 32-bit targets, leaving the default u32 path (even on 32-bit) unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3795cf4-fd2c-46db-ac06-fc4a360392ba

Add an opt-in, validation-only path to the bf-tree full-precision benchmark that transposes a real dataset's dense 0..N u32 row ids into the u64-only range (base > u32::MAX), exercising the provider's u64 vertex-id code paths without touching production code. - diskann-benchmark-core: `Offset` ToId<u64> mapper (base + i, checked) with tests, documented as validation-only. - diskann-benchmark: `single_or_multi_insert_offset` insert helper, a `U64IdValidation { offset }` JSON config on BfTreeFullPrecisionBuild (serde default, non-breaking), a widened `bftree_parameters_offset`, and a dedicated `run_u64_id_validation` orchestration that builds a BfTreeProvider<T, NoStore, u64>, inserts with the offset, and widens ground-truth ids via Matrix::map to keep recall aligned. - Example config + end-to-end integration test asserting the transposed u64 run reproduces the identity-u32 recall baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3795cf4-fd2c-46db-ac06-fc4a360392ba

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Address hyper-critical review + Copilot PR feedback: - Add `test_u64_aliased_ids_distinct_vectors` covering the full-precision vector store's `as_index` key path. Existing u64 tests only use id values in `0..N` (collision-free low 32 bits), so a truncating key would pass them; this test inserts distinct vectors at `low` and `low | (1<<32)` and asserts each reads back its own vector, catching low-32-bit key aliasing. - Soften the over-claiming doc comment on `test_quantized_index_search_u64_high_ids` to reflect what it actually covers, pointing at the dedicated aliasing tests for key integrity. - DRY `single_or_multi_insert`/`single_or_multi_insert_offset` (Copilot #3): extract private `single_or_multi_insert_with_ids<..., M: ToId>`; both wrappers delegate with `Identity`/`Offset` mappers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3795cf4-fd2c-46db-ac06-fc4a360392ba

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

+        input: &BfTreeFullPrecisionBuild,
+        validation: U64IdValidation,
+        checkpoint: Checkpoint<'_>,
+        mut output: &mut dyn Output,


JordanMaples requested review from a team and Copilot June 30, 2026 21:28

Copilot started reviewing on behalf of JordanMaples June 30, 2026 21:39 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread diskann-bftree/src/provider.rs Outdated

Comment thread diskann-bftree/src/provider.rs Outdated

hildebrandmw reviewed Jul 1, 2026

View reviewed changes

Comment thread diskann-bftree/src/provider.rs Outdated

Comment thread diskann-bftree/src/id.rs Outdated

JordanMaples mentioned this pull request Jul 7, 2026

Delta Compression for U64 based ids #1234

Open

JordanMaples enabled auto-merge (squash) July 15, 2026 23:11

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 3583acb to 5d4d540 Compare July 15, 2026 23:18

JordanMaples and others added 5 commits July 27, 2026 10:21

Copilot AI review requested due to automatic review settings July 27, 2026 19:00

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 5d4d540 to b07f493 Compare July 27, 2026 19:00

Copilot started reviewing on behalf of JordanMaples July 27, 2026 19:01 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread diskann-benchmark/src/index/build.rs

Copilot AI review requested due to automatic review settings July 27, 2026 21:00

Copilot started reviewing on behalf of JordanMaples July 27, 2026 21:00 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread diskann-benchmark/src/index/bftree/full_precision.rs

input: &BfTreeFullPrecisionBuild,

validation: U64IdValidation,

checkpoint: Checkpoint<'_>,

mut output: &mut dyn Output,

Uh oh!

Conversation

JordanMaples commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Hardening

Wide-id tests + 32-bit compile guard

Real-world u64 validation (benchmark layer)

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

harsha-simhadri commented Jun 30, 2026

Uh oh!

JordanMaples commented Jul 1, 2026

Uh oh!

harsha-simhadri commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

harsha-simhadri commented Jul 13, 2026

Uh oh!

JordanMaples commented Jul 13, 2026

Uh oh!

JordanMaples commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JordanMaples commented Jun 30, 2026 •

edited

Loading

Real-world `u64` validation (benchmark layer)

codecov-commenter commented Jun 30, 2026 •

edited

Loading