[C API] Add memory accounting (get_memory_usage / get_memory_breakdown)#354
Open
yuejiaointel wants to merge 4 commits into
Open
[C API] Add memory accounting (get_memory_usage / get_memory_breakdown)#354yuejiaointel wants to merge 4 commits into
yuejiaointel wants to merge 4 commits into
Conversation
- Core: Add dataset_allocated_bytes to svs/core/data.h - Core: Add MemoryBreakdown struct to VamanaIndex/MutableVamanaIndex - Core: Add element_size() and get_memory_breakdown() to core indexes - Orchestrator: Add element_size() and get_memory_breakdown() to VamanaInterface/Vamana/DynamicVamana - C API: Add svs_memory_breakdown_t struct - C API: Add svs_index_element_size(), svs_index_get_memory_usage(), svs_index_get_memory_breakdown() - C API: Add Index interface methods + implementations in IndexVamana/DynamicIndexVamana - Tests: Add comprehensive tests for static and dynamic indexes with null-arg handling All tests pass (7/8 test cases, 618/619 assertions; 1 pre-existing failure unrelated).
yuejiaointel
requested review from
ahuber21,
ethanglaser and
ibhati
as code owners
July 16, 2026 06:43
|
Tick the box to add this pull request to the merge queue (same as
|
rfsaliev
previously requested changes
Jul 16, 2026
rfsaliev
left a comment
rfsaliev
left a comment
Member
There was a problem hiding this comment.
It seems like element_size() to be reviewed
| size_t dimensions() const { return data_.dimensions(); } | ||
|
|
||
| /// @brief Return the element size (bytes per vector) for the indexed data. | ||
| size_t element_size() const { return data_.element_size(); } |
Member
There was a problem hiding this comment.
IMHO, <Index>::element_size() should return at least data_.element_size() + graph_.element_size()
@eshenayo, can you please add your comment? Thank you.
P.S. It would be good to account metadata_.element_size() - but I am not sure if it is possible due to metadata complexity (e.g. id_translator is the map with the complex internal structure)
Contributor
Author
There was a problem hiding this comment.
Metadata isn't per-vector (id-translator is a shared map), so it can't go in element_size(). And data + graph + metadata is exactly what get_memory_breakdown() already returns. Proposal: keep element_size() at the dataset level, use get_memory_breakdown() on the index.
element_size() now returns per-vector data + graph adjacency row per review; metadata remains in get_memory_breakdown.
…t_memory_breakdown/get_memory_usage
element_size is a dataset-level concept (bytes per stored vector). At the
index level, the memory footprint is the sum of data + graph + metadata
components, which do not cleanly divide per vector due to shared overhead.
The index already exposes complete memory accounting via:
- get_memory_breakdown() returns {graph_bytes, data_bytes, metadata_bytes}
- get_memory_usage() returns breakdown.total()
Removed element_size from:
- Core index (VamanaIndex, MutableVamanaIndex)
- Orchestrator (VamanaInterface, VamanaImpl, Vamana, DynamicVamana)
- C API interface (Index base class, IndexVamana, DynamicIndexVamana)
- C API public surface (svs_index_element_size)
- Tests (c_api_index.cpp, c_api_dynamic_index.cpp)
Dataset-level element_size() (SimpleData, LVQDataset, etc.) is unchanged.
yuejiaointel
changed the title
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
2 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.
Summary
Exposes memory accounting in the C API for the Valkey-search integration:
svs_index_get_memory_usage(index, size_t* out_bytes, err)— total allocated bytes.svs_index_get_memory_breakdown(index, svs_memory_breakdown_t* out, err)—{graph_bytes, data_bytes, metadata_bytes}component split.-(keep at data level)svs_index_element_size(index, size_t* out_bytes, err)— bytes per stored vector.All follow the existing C API conventions (out-param +
svs_error_h,wrap_exceptions), matching the Phase-A design in the memory-accounting contract (intel-innersource #333).Layers
bindings/c): the three functions +svs_memory_breakdown_tinsvs_c.h; interface virtuals + concrete overrides insrc/index.hpp; impls insrc/svs_c.cpp.get_memory_breakdown()(MemoryBreakdownstruct + capacity-basedsvs::data::detail::dataset_allocated_byteshelper) onVamanaIndex/MutableVamanaIndexand through the orchestrator, plus anelement_size()accessor parallel todimensions(). This mirrors the approved public PR [Vamana] Add get_memory_usage() to report allocated bytes #345 so the C API can build and test standalone; once [Vamana] Add get_memory_usage() to report allocated bytes #345 lands ondev/c-api, this reduces to just the C API layer.Tests
bindings/c/tests/c_api_index.cpp(static) andc_api_dynamic_index.cpp(dynamic): usage > 0, breakdown total == usage,graph_bytes/data_bytes> 0 (metadata > 0 for dynamic),element_size == sizeof(float) * dimensions, and null-arg handling. Both test cases pass (84 / 166 assertions).Related: builds on #345; memory-accounting contract in intel-innersource #333 / #326.