Skip to content

Upgrade bf-tree dependency#1183

Open
JordanMaples wants to merge 4 commits into
mainfrom
jordanmaples/bump-bftree
Open

Upgrade bf-tree dependency#1183
JordanMaples wants to merge 4 commits into
mainfrom
jordanmaples/bump-bftree

Conversation

@JordanMaples

@JordanMaples JordanMaples commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Bump bf-tree 0.4 → 0.5

Upgrades the bf-tree dependency from 0.4.9 to 0.5.4 and migrates to the new CPR snapshot API.

Changes

Snapshot API migration

  • snapshot_memory_to_disk / snapshot() → cpr_snapshot(&path)
  • new_from_snapshot_disk_to_memory / new_from_snapshot → BfTree::new_from_cpr_snapshot()
  • save_bftree is no longer async — cpr_snapshot writes directly to the target path
  • load_bftree simplified — 0.5 reconstructs config from the snapshot file header, so no external BfTreeParams config is needed
  • Removed dead BfTreeParams::to_config and copy_snapshot_if_needed

use_snapshot configuration

  • Added use_snapshot: bool to BfTreeProvider, BfTreeProviderParameters, and SavedParams
  • cpr_snapshot panics if called without use_snapshot configured — save_bftree guards against this
  • BfTreeStoreConfig::use_snapshot is required in benchmark JSON input (no #[serde(default)])
  • SavedParams::use_snapshot uses #[serde(default)] for backwards compat with existing saved param files

Round-trip tests

  • graph_index_bftree_save_load_roundtrip — full-precision build → save → load → verify
  • graph_index_bftree_spherical_save_load_roundtrip — spherical quantized build → save → load → verify

Benchmark results (wikipedia-100K, float32, inner product)

Static (build + search):

┌─────────────┬──────────────┬────────────┐
│ Metric      │ 0.4 baseline │ 0.5 bumped │
├─────────────┼──────────────┼────────────┤
│ Recall@100  │ 0.9536       │ 0.9536     │
├─────────────┼──────────────┼────────────┤
│ QPS         │ 457          │ 467        │
├─────────────┼──────────────┼────────────┤
│ p99 latency │ 26.0ms       │ 25.5ms     │
└─────────────┴──────────────┴────────────┘

Streaming (insert/delete/search):

┌─────────────┬──────────────┬───────────────────┐
│ Metric      │ 0.4 baseline │ 0.5 bumped        │
├─────────────┼──────────────┼───────────────────┤
│ Recall@100  │ 0.9692       │ 0.9691            │
├─────────────┼──────────────┼───────────────────┤
│ QPS         │ 625          │ 673 (+7.6%)       │
├─────────────┼──────────────┼───────────────────┤
│ p99 latency │ 19.2ms       │ 17.3ms (-10%)     │
└─────────────┴──────────────┴───────────────────┘

No regression on static workloads. ~7.6% QPS improvement on streaming.

@JordanMaples

Copy link
Copy Markdown
Contributor Author

Closes #1157
Supersedes #1163

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR upgrades the bf-tree dependency to 0.5.x and migrates DiskANN’s bf-tree provider persistence from the old snapshot APIs to the new CPR snapshot APIs, including updating benchmark inputs/examples and adding round-trip save/load coverage.

Changes:

  • Bump bf-tree from 0.4.x to 0.5.x and migrate provider save/load to CPR snapshots.
  • Introduce use_snapshot plumbing across provider parameters, saved params, and benchmark JSON inputs/examples.
  • Add benchmark-driven round-trip save/load tests for bf-tree full-precision and spherical (quantized) indexes.

Reviewed changes

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

Show a summary per file
File Description
diskann-bftree/src/provider.rs CPR snapshot save/load migration; new use_snapshot flag added to provider + serialization.
diskann-bftree/src/neighbors.rs Update neighbor provider snapshot test to use cpr_snapshot.
diskann-benchmark/src/main.rs Add bf-tree benchmark round-trip save/load integration tests.
diskann-benchmark/src/inputs/bftree.rs Add use_snapshot to bf-tree benchmark store config and map into provider params.
diskann-benchmark/src/index/bftree/full_precision.rs Save built bf-tree index when save_path is requested.
diskann-benchmark/src/index/bftree/spherical.rs Save built bf-tree spherical index when save_path is requested.
diskann-benchmark/example/graph-index-bftree.json Add use_snapshot field to example store configs.
diskann-benchmark/example/graph-index-bftree-stream.json Add use_snapshot field to example store configs.
diskann-benchmark/example/graph-index-bftree-spherical.json Add use_snapshot field to example store configs (incl. quant).
Cargo.toml Update dependency requirement to bf-tree = "0.5".
Cargo.lock Lockfile update to bf-tree 0.5.4 and dependency graph adjustments.

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

Comment on lines 268 to 283
Ok(Self {
quant_vectors: quant_precursor.create(params.quant_vector_provider_config)?,
full_vectors: VectorProvider::new_with_config(
params.max_points,
params.dim,
params.num_start_points.get(),
params.vector_provider_config,
)?,
neighbor_provider: NeighborProvider::new_with_config(
params.max_degree,
params.neighbor_list_provider_config,
)?,
metric: params.metric,
graph_params: params.graph_params,
use_snapshot: params.use_snapshot,
})
Comment thread diskann-benchmark/src/inputs/bftree.rs Outdated
Comment on lines +217 to +220
use_snapshot: vector_store_config
.as_ref()
.and_then(|c| c.use_snapshot)
.unwrap_or(false),
@codecov-commenter

codecov-commenter commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.46%. Comparing base (d9ce362) to head (aea760c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
diskann-bftree/src/provider.rs 88.57% 8 Missing ⚠️

❌ Your patch status has failed because the patch coverage (89.33%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1183      +/-   ##
==========================================
- Coverage   89.47%   89.46%   -0.02%     
==========================================
  Files         486      487       +1     
  Lines       92161    92177      +16     
==========================================
+ Hits        82458    82462       +4     
- Misses       9703     9715      +12     
Flag Coverage Δ
miri 89.46% <89.33%> (-0.02%) ⬇️
unittests 89.11% <89.33%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
diskann-benchmark/src/main.rs 91.36% <ø> (ø)
diskann-bftree/src/neighbors.rs 94.03% <100.00%> (+0.08%) ⬆️
diskann-bftree/src/provider.rs 90.72% <88.57%> (-0.25%) ⬇️

... and 19 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JordanMaples and others added 2 commits June 17, 2026 13:25
- Update bf-tree dependency from 0.4.9 to 0.5
- Migrate save/load to cpr_snapshot/new_from_cpr_snapshot
- Add use_snapshot field to BfTreeProvider, BfTreeProviderParameters,
  SavedParams, and BfTreeStoreConfig
- Guard save_bftree against unconfigured snapshot support
- Move record size validation into provider constructors (from PR #1166)
- Remove dead BfTreeParams::to_config and copy_snapshot_if_needed
- Add save_path support and round-trip tests for both full-precision
  and spherical (quantized) bftree indexes
- Remove #[ignore] on spherical round-trip test (0.5 fixes the panic)
- Require use_snapshot in benchmark JSON input configs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove top-level use_snapshot from build structs; derive from store configs
- Add reconcile_use_snapshot() to validate all configs agree
- Make BfTreeStoreConfig::use_snapshot a required bool (not Option)
- Force-set use_snapshot on all bf-tree Configs in provider constructor
- bftree_parameters_from now returns anyhow::Result

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JordanMaples JordanMaples force-pushed the jordanmaples/bump-bftree branch from 06abe40 to 3902c6a Compare June 18, 2026 02:55
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snash4

snash4 commented Jun 19, 2026

Copy link
Copy Markdown

@JordanMaples - I was testing this PR for save_path and found save_path is not working for graph-index-bftree-stream.json benchmark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants