Repository navigation
Unified index format V1 - #1221
Merged
Merged
Conversation
| 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
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>
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>
Jerry Gao (Sanhaoji2)
force-pushed
the
jegao/UnifyIndexFormat
branch
from
July 9, 2026 07:21
f01b748 to
395f534
Compare
| #include <cstdint> | ||
| #include <vector> | ||
|
|
||
| #include "windows_customizations.h" |
Contributor
Author
There was a problem hiding this comment.
why we need to include windows_customizations.h here?
…search unified_index_memory::search_impl pre-checked _query_scratch.size()==0 and threw "scratch pool empty (was load() called?)". Under concurrent load this is a normal transient state -- every scratch object is checked out by other query threads -- so the guard made concurrent queries beyond the pool size fail immediately with an ANNException instead of waiting. Remove the guard. ScratchStoreManager's constructor already blocks (pop() + wait_for_push_notify() loop) until a scratch frees up, matching the legacy Index<T>::search paths which construct the manager with no such pre-check. The pool is always populated by load() (init_scratch_pool allocates >= 1 entry, num threads clamped to 1), so the "was load() called?" case cannot occur via the factory API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5c45633f-9544-4335-8769-dddf934bcb0e
The unified builder constructed IndexWriteParameters as IndexWriteParametersBuilder(ctx.L, ctx.R), i.e. search_list_size = L and filter_list_size = 0 (default). For a filtered build that inverts the intended behaviour: search_for_point_and_prune runs the FILTERED greedy search with filter_list_size (= 0, degenerate) and then, because search_list_size = L > 0, ALSO runs an unfiltered greedy search and merges its candidates. The graph is therefore built from unfiltered candidate exploration, producing a denser, differently-connected graph than the legacy v2 mem/SSD indexes (built via DANNBuilderV2 / ConstructIndexConfigBuilder), which set search_list_size = 0 and filter_list_size = L for filtered builds. Mirror the v2 filtered setup: for a filtered build, pass search_list_size = 0 and filter_list_size = L so graph construction is driven solely by the filtered greedy search. Unfiltered builds are unchanged. Add a regression test (builder_filtered_uses_filtered_only_candidate_pool): with a fixed rand() seed and a single build thread, the builder's filtered graph is byte-identical to a direct Index build with search_list_size = 0, and strictly sparser than one built with search_list_size = L. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5c45633f-9544-4335-8769-dddf934bcb0e
Jerry Gao (Sanhaoji2)
merged commit Jul 24, 2026
ea3f3e2
into
jegao/LabelHotFix
4 of 39 checks passed
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?