Unified index format V1#1221
Open
Sanhaoji2 wants to merge 6 commits into
Open
Conversation
Add a single-file unified index container (docs/unified_index_format.md) that can be built once and loaded as either an in-memory or SSD-served index: - New class hierarchy: unified_index (interface) + unified_index_base<T> with unified_index_memory<T> / unified_index_ssd<T>, backed by unified_node_store and unified_label_data (bitmask + integer encodings). - UnifiedIndexWriter/Reader (unified_index_io) and unified_index_builder for end-to-end build with optional PQ. - Index::save_unified emits the container; get_table_stats() exposed on the unified index (mirrors Index/PQFlashIndex). - Build the unit tests against a new static diskann_s lib (DISKANN_STATIC_LIB) so internal symbols need not be exported from the DLL. - Tests: node-store/label/factory/memory/ssd/builder suites plus legacy<->unified parity (memory, SSD, filtered) and get_table_stats coverage. - Fix: populate _label_map during filtered build so save_unified after build emits a valid label dictionary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cave-g-f
reviewed
Jul 3, 2026
| throw ANNException("unified_node_store_base: offset table size mismatch", -1, __FUNCSIG__, __FILE__, | ||
| __LINE__); | ||
| } | ||
| _coord_bytes = _header.aligned_dim * sizeof(T); |
There was a problem hiding this comment.
keep consistency with write corrd_bytes
cave-g-f
reviewed
Jul 3, 2026
| static_cast<uint64_t>(_start)); | ||
|
|
||
| writer.begin_graph_region(); | ||
| std::vector<T> vec(dim); |
There was a problem hiding this comment.
dim ? aligned_dim? keep consistency write/read
UnifiedIndexWriter::write_node wrote coords as dim*sizeof(T), but the node store decodes them as aligned_dim*sizeof(T) (and the distance functions read the full aligned width). When dim is not a multiple of 8 (e.g. 66 -> 72) the offset math desynced: wrong derived degree, neighbor pointer landing inside the coord region, and distances reading neighbor bytes as coords. - write_node now zero-pads coords from dim to aligned_dim so writer and reader agree on the per-node stride (keeps the aligned-SIMD distance path). - docs/unified_index_format.md: coords documented as aligned_dim*sizeof(T) (padded from dim); degree-derivation formula updated. - Add parity_unaligned_dim_66 regression test (dim=66): memory + SSD unified search now match legacy exactly (50/50); pre-fix it was 39/50 and 38/50. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add DISKANN_DLLEXPORT to the unified_index_builder class so its single public build() entry point can be called across the DLL boundary (e.g. from the AdsSnr PublicDiskANNBuilderWrapper). Other unified_* classes remain unexported (internal to the DLL / static lib). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add DISKANN_DLLEXPORT to make_unified_index_memory and make_unified_index_ssd so the unified index can be loaded/searched across the DLL boundary (e.g. from the AdsSnr PublicDiskANNWrapper serving side). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simple_bitmask was 'class DISKANN_DLLEXPORT', but consumers that compile label_bitmask.cpp directly (e.g. AdsSnr's ANNTestTool, which defines neither _WINDLL nor DISKANN_STATIC_LIB) then define its members in a dllimport context, triggering C4273 'inconsistent dll linkage' (a build error under CloudBuild warning-as-error/auto-baseline). Make all simple_bitmask methods inline (header-only) and remove the class-level DISKANN_DLLEXPORT; label_bitmask.cpp now has no out-of-line definitions. This resolves the odr-use from ColorInfoVector's inline constructor (color_info.h) locally in every consumer -- DLL, static lib, and direct-compile -- with no import/export, eliminating C4273. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Supersedes the header-only inlining (304457a) with the minimal change: remove DISKANN_DLLEXPORT from simple_bitmask and restore label_bitmask.cpp to its original out-of-line definitions. This is sufficient to fix the C4273 'inconsistent dll linkage' in projects that compile label_bitmask.cpp directly (e.g. ANNTestTool); every module that odr-uses simple_bitmask also compiles that .cpp, so no export is needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a single-file unified index container (docs/unified_index_format.md) that can be built once and loaded as either an in-memory or SSD-served index:
Reference Issues/PRs
What does this implement/fix? Briefly explain your changes.
Any other comments?