[CPP Runtime] Refactor storage tags and add static Vamana index support#297
[CPP Runtime] Refactor storage tags and add static Vamana index support#297
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the C++ runtime storage/tag dispatch to be allocator-aware and introduces a concrete “static” VamanaIndex runtime API/implementation (plus LeanVec-specialized builders), updating existing IVF/Flat/Dynamic-Vamana code to the new storage plumbing and extending the C++ runtime test suite accordingly.
Changes:
- Refactor runtime storage dispatch from
StorageKindTagtoStorageType<Kind, Alloc>and pass allocators into storage factories. - Add static
VamanaIndeximplementation (build/load/save/search/range_search) and LeanVec build specializations. - Update IVF/Flat/Dynamic-Vamana implementations + add runtime tests for static Vamana variants.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| include/svs/core/data/simple.h | Adds is_blocked_v trait to detect blocked allocator wrappers. |
| bindings/cpp/src/svs_runtime_utils.h | Reworks storage type mapping/factory APIs to be allocator-aware and tagless. |
| bindings/cpp/src/vamana_index_impl.h | New static Vamana implementation (including LeanVec specialization). |
| bindings/cpp/src/vamana_index.cpp | Implements the new VamanaIndex/VamanaIndexLeanVec runtime interfaces. |
| bindings/cpp/include/svs/runtime/vamana_index.h | Expands VamanaIndex public API for static Vamana and LeanVec builders. |
| bindings/cpp/include/svs/runtime/dynamic_vamana_index.h | Adapts DynamicVamana to the updated VamanaIndex base interface. |
| bindings/cpp/src/dynamic_vamana_index_impl.h | Updates storage dispatch/build path for allocator-based storage factory. |
| bindings/cpp/src/dynamic_vamana_index_leanvec_impl.h | Updates LeanVec tag dispatch to StorageType<Kind, Alloc>. |
| bindings/cpp/src/ivf_index_impl.h | Refactors IVF storage dispatch to the new StorageType<Kind, Alloc> approach. |
| bindings/cpp/src/dynamic_ivf_index_impl.h | Refactors Dynamic-IVF storage dispatch similarly. |
| bindings/cpp/src/flat_index_impl.h | Updates Flat index storage type/tag usage to StorageType<Kind, Alloc>. |
| bindings/cpp/tests/runtime_test.cpp | Adds static Vamana read/write/search/range-search tests and LeanVec training-path test. |
| bindings/cpp/CMakeLists.txt | Gates link_mkl_static() behind SVS_EXPERIMENTAL_LINK_STATIC_MKL. |
rfsaliev
force-pushed
the
rfsaliev/cpp-runtime-static-vamana
branch
from
da6d148 to
1a773d7
Compare
| // Selective search with IDSelector | ||
| auto old_sp = get_impl()->get_search_parameters(); | ||
| auto sp_restore = svs::lib::make_scope_guard([&]() noexcept { | ||
| get_impl()->set_search_parameters(old_sp); | ||
| }); | ||
| get_impl()->set_search_parameters(sp); | ||
|
|
There was a problem hiding this comment.
In the filtered-search path, this temporarily mutates the underlying index’s search parameters (set_search_parameters) inside a const method. This is not thread-safe if the same index instance is searched concurrently (different calls can race and restore the wrong parameters). Prefer an API that passes search parameters directly to the iterator/search, or protect the mutation with synchronization (or clearly document that concurrent searches are unsupported when using filters/params).
| // Return type defined to simple to allow substitution in templates. | ||
| template <> struct StorageFactory<UnsupportedStorageType> { | ||
| using StorageType = SimpleDatasetType<float>; | ||
| template <typename Alloc> struct StorageFactory<UnsupportedStorageType<Alloc>> { |
There was a problem hiding this comment.
Is possible to unify StorageFactory with IVFStorageFactory?
There was a problem hiding this comment.
Yes, it is possible, but I would like to do such refactoring in a separate PR - this one is big enough already :)
@ibhati, your comments?
There was a problem hiding this comment.
Yes, it is definitely possible. But I agree, let's do it in a separate PR.
rfsaliev
force-pushed
the
rfsaliev/cpp-runtime-static-vamana
branch
from
1a773d7 to
9062bf5
Compare
Enable static Vamana index functionality support in CPP Runtime API.
This PR includes following changes:
StorageKindTag<Kind>is removed and replaced withStorageType<Kind, Alloc>in usagesStoragefactory::init()block_sizeargument is replaced withallocator- to be constructed in index implementation code (with appropriate block size)VamanaIndexinterface implementation.