[diskann-inmem] Prepare code for quantization and beyond - #1352
[diskann-inmem] Prepare code for quantization and beyond#1352Mark Hildebrand (hildebrandmw) wants to merge 38 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors diskann-inmem’s in-memory provider architecture to prepare for future quantization support by separating slot lifecycle management (Store) from storage mechanics (Plugin), and moving Store ownership into the Layer abstraction.
Changes:
- Introduces a
store::pluginmodule and reworks the in-memoryStoreinto a generic driver overPlugin, enabling multiple storage backends under the same EBR lifecycle. - Reworks
Provider/layersintegration so layers constructSearchAccessor/PruneAccessordirectly (including new pruning buffering/opaque-key plumbing), and updates call sites (bench + integration). - Adds/updates concurrency stress tests and CI doc builds to cover the reorganized APIs and internal documentation.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-inmem/src/tag.rs | Adds a typed AtomicTag::SIZE constant used by plugins/layout computations. |
| diskann-inmem/src/store/plugin.rs | Adds the Plugin/Slot traits and lifecycle documentation for store-driven EBR transitions. |
| diskann-inmem/src/store/mod.rs | New generic Store<P> driving plugin lifecycle + tags/freelist/registry/neighbors; adds tests. |
| diskann-inmem/src/store/invasive.rs | Ports the prior “invasive store” into a Plugin implementation with reader/slot types. |
| diskann-inmem/src/store/checked.rs | Adds a Plugin implementation that aggressively validates lifecycle invariants (test/integration). |
| diskann-inmem/src/store.rs | Removes the old monolithic uniform Store implementation (superseded by store/mod.rs + plugins). |
| diskann-inmem/src/provider.rs | Reworks provider to delegate storage/accessor construction to layers and updates search/prune paths. |
| diskann-inmem/src/prefetch.rs | Adds structured, checkable prefetch abstractions and tests. |
| diskann-inmem/src/num.rs | Adds typed integer wrappers (Capacity, MaxDegree, IdLimit) and new Bytes utilities. |
| diskann-inmem/src/neighbors.rs | Updates neighbors graph storage to use IdLimit/MaxDegree typed wrappers. |
| diskann-inmem/src/lib.rs | Exposes the new store module publicly and wires in prefetch. |
| diskann-inmem/src/layers/mod.rs | Redesigns layer traits around construction + accessors + insert/prune hooks; adds internal prune/expand traits. |
| diskann-inmem/src/integration/store/mod.rs | Adds shared macro boilerplate for integration-test wrappers over different store plugins. |
| diskann-inmem/src/integration/store/invasive.rs | Integration-test wrapper for the invasive plugin-backed store. |
| diskann-inmem/src/integration/store/checked.rs | Integration-test wrapper for the checked plugin-backed store. |
| diskann-inmem/src/integration/store.rs | Removes the old single-store integration wrapper (replaced with per-plugin wrappers). |
| diskann-inmem/src/integration/counters.rs | Updates integration counter snapshot docs to match the new public surface. |
| diskann-inmem/src/ids.rs | Switches ID mapping to typed Capacity and updates bounds logic/tests accordingly. |
| diskann-inmem/src/freelist.rs | Minor doc tweak while keeping freelist mechanics intact. |
| diskann-inmem/src/counters.rs | Makes LocalCounters public (still unconstructable externally) to support public APIs. |
| diskann-inmem/integration/support/datatype.rs | Switches f16 conversions to diskann_wide casting utilities for consistency. |
| diskann-inmem/integration/store/mod.rs | Replaces the old single store stress benchmark with shared infrastructure + per-plugin benchmarks. |
| diskann-inmem/integration/store/invasive.rs | Adds invasive-store stress benchmark implementation using the new wrappers. |
| diskann-inmem/integration/store/checked.rs | Adds checked-store stress benchmark implementation using the new wrappers. |
| diskann-inmem/integration/store.rs | Removes the old monolithic store stress benchmark implementation. |
| diskann-inmem/integration/main.rs | Updates integration runner to register the new store benchmarks via store::register. |
| diskann-inmem/integration/jsons/store-stress.json | Updates benchmark job definitions for separate invasive/checked store stress tests. |
| diskann-inmem/integration/jsons/store-stress-test.json | Updates smaller test job definitions for separate invasive/checked store stress tests. |
| diskann-inmem/integration/jsons/integration-baseline.json | Updates baseline counter expectations reflecting the new implementation paths. |
| diskann-inmem/integration/index/runner.rs | Updates index integration runner to construct providers via new Full::config + typed params. |
| diskann-inmem/Cargo.toml | Adds hashbrown dependency used by new prune buffering logic. |
| diskann-benchmark/src/index/inmem2.rs | Updates benchmark provider construction to new Full::config API; adds u8 benchmark. |
| Cargo.lock | Records the new hashbrown dependency for diskann-inmem. |
| .github/workflows/ci.yml | Tightens doc builds with RUSTDOCFLAGS=-D rustdoc::all, adds inmem private docs, and includes integration-test feature. |
Suppressed comments (1)
diskann-inmem/src/integration/store/mod.rs:20
- Spelling in doc comment: "wraper" should be "wrapper".
/// A test store wraper.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1352 +/- ##
==========================================
+ Coverage 91.54% 91.58% +0.04%
==========================================
Files 521 523 +2
Lines 100347 101158 +811
==========================================
+ Hits 91863 92650 +787
- Misses 8484 8508 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Prepare
diskann-inmemfor quantization and beyond.Note that this is infrastructure work to get the code ready. Quantization is not yet integrated.
Goals
The requirements to support quantization are annoyingly orthogonal:
Collections like PQ, scalar, and spherical quantization should be able to run in "quant-only" mode and "quant + full-precision" mode, where the full-precision store is used for reranking. Internally, I would like these to be the same type to cut down on unnecessary monomorphization. This means we need support for at least two collections managed by the same epoch protected
Store(one quant, the other full-precision). In this case, the full-precision store can do without the invasive tags used for the primary store, but also needs to be optional and ideally support any off32,f16,u8,i8and beyond.For testing purposes, we probably want to retain the ability for PQ to do hybrid pruning (part full-precision, part quantized). This completely breaks the current model used by inmem of managing raw
&[u8]slices. While we could technically make it work, if we needed to do something like multi-vector operations, a&[u8]is just not the right approach anyways.We also want to be able to support multi-vectors and other kinds of non-uniform data in an epoch guarded
Store, which gets rid of the uniform assumption of the current invasive store.Supporting all of these required a pretty drastic reorganization of
diskann-inmem.Architecture
This PR is all about moving things up and down. The architecture went from this:
where the provider had a separate
LayerandStoreand combined the two to make aSearchAccessorandPruneAccessorto thisFrom top to bottom:
The
Providernow contains aLayerinstead of aLayerand aStore. Instead, theStorehas been moved directly into theLayer. Instead of theProviderbeing responsible for building aSearchAccessorfrom pieces exposed by the oldLayer, theProvidercompletely delegatesSearchAccessorandPruneAccessorconstruction to itsLayer.The rest of the
Provider's job is interfacing thediskann::graph::glueAPI to the simplifiedLayerAPI.Layernow gains theStore. TheLayerfamily of traits is extended to include logical operations like insert and retire. As mentions above, it is also responsible for buildingSearchAccessors andPruneAccessors.Storereceives minor changes - its internalBufferwhere it used to manage the invasive data store directly has now been moved to aPlugintrait. In this architecture, theStoreis just responsible for driving thePlugintrait's lifecycle API and is completely uninvolved with the mechanics of raw data reading and writing.Pluginis a new trait for a slot-based store whose slots are driven byStore. The old invasive store is an example of such aPlugin.In addition to this hierarchical layering, a
Layeris allowed (and indeed, expected) to bypass its immediateStoreto the underlyingPlugindirectly to build the various accessors. For theInvasivestore, this works because concurrency tags are embedded directly in thePlugin- readers do not need anything to do with the parent store.Why This Mostly (Probably) Works
The
Plugintrait provides an extension point for managing different types of data. For the quantized case, we reuseInvasivefor the quantized data and a simplerBuffer-based one for the full-precision reranking data. Then we can createPluginconsisting of both layers. The short-cut from aLayerto itsPluginmeans a quantized provider can create it'sSearchAccessor/PruneAccessors with knowledge of both. We could even have anInvasivestore for both the quantized data and the full-precision data and reuse the existing full-precision infrastructure to support both quantized and full-precision searches over the same `Layer.Additionally,
Pluginmakes no requirements on the kind of data store. This allows us to store un-even sized allocations in aPlugin. It's theLayer's job to make sense of everything.Finally, the
Layerknowing the details of itsPlugins means we can (with some creativity) still support hybrid pruning.Suggested Reviewing Order
This is a large PR, but I tried very hard to keep things structured. The reviewing order outlined here is a suggested bottom-up order. Understanding how the lower levels work is important for understanding how the higher ones come together.
num.rs: A quick warm-up. This PR introduces some strongly typed integers with specific semantics.prefetch.rs: Another warm-up. I wasn't satisfied with the safety/flexibility of prefetching in the current in-mem provider. This PR exacerbated the situation, so I introduced a bit more structure on prefetchers.store/plugin.rs: This defines thePlugintrait and it's expected lifecycle. I captured the nuances in healthy module level and trait level documentation. This is probably the most nuanced change in this PR.store/checked.rs: An implementation ofPluginthat aggressively checks that the invariants required for thePluginAPI are upheld byStore. Again, there is healthy module-level documentation describing the logic.store/invasive.rs: The old invasive data store moved to implement thePlugintrait. This largely preserves what was already in the oldStoreand is conceptually much simpler thanstore/checked.rs.store/mod.rs: ModifyingStoreto work against aPlugingeneric instead of directly managing the invasive store. Note that there are some changes to initialization.Store::newnow takes three distinct arguments:Layout: Description for capacity, number of frozen points, and maximum degree.Config: Configuration state dedicated directly to management of the internal concurrency data structures.PluginConfig: Configuration of the internal plugin. TheXConfigtraits are used in this PR to perform deferred initialization of large data structure.Detour: With the introduction of a modularized
Store, changes were made to the following integration-test related files to enable concurrent stress-test of differentStoreimplemntations.src/integration/store/*: Shared boiler plate and implementations for exposing differentStoreto the integration test framework. This PR exposes wrappers for theInvasiveandCheckedstores in theinvasiveandcheckedmodules respectively.integration/store/*: Integration test exposure for the different stores as different jobs in the integration test suite.For these, the overall structure of the exposed stores is extremely similar. Most of churn in these files is moving things around so keep the amount of repeated code to a minimum.
layer/mod.rs: ReworkedLayer/Set/Search/Inserttraits for the new architecture.Layer: Gains a few life-cycle related items.Set: Is now responsible for also obtaining an internal slot for the inserted element.Search/Insert: Reworked to returnSearchAccessorandPruneAccessors directly. This is mainly to allow us to keep the internalStore/Plugindetails hidden from the public interface.In addition:
ExpandBeam: Moves from its old location inprovider.rs. Otherwise, is mostly unchanged.Prune: A new trait for pruning. Prune implementation now internally "buffer" items in the prune set. This effectively makes the type of the elements being pruned hidden, allowing for hybrid pruning in the future.layer/full.rs: This is where everything comes together. TheConfigis used to group together full-precision related constructor arguments andFullnow gains aStore. Importantly, now thatFullknows the full details of its store, we can more aggressively optimizeExpandBeamwith fewer bounds and length checks. Note that start point initialization is now managed byFull's constructor.The implementation of
ExpandBeamis taken pretty much directly from the oldprovider.rscode. Additionally, the tests have gotten more robust with Miri having more coverage of theExpandBeamimplementation and correctness tests for the various distance specializations.I highly recommend looking at the public docs
and the private docs
to get a feeling for the public API and internal documentation.
AI Disclosure: An agent was used review changes and implementation details, help brainstorm, and make focused edits to documentation.