Skip to content

[C API] Add memory accounting (get_memory_usage / get_memory_breakdown / element_size)#354

Open
yuejiaointel wants to merge 1 commit into
dev/c-apifrom
yuejiao/c-api-memory-usage
Open

[C API] Add memory accounting (get_memory_usage / get_memory_breakdown / element_size)#354
yuejiaointel wants to merge 1 commit into
dev/c-apifrom
yuejiao/c-api-memory-usage

Conversation

@yuejiaointel

Copy link
Copy Markdown
Contributor

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.
  • 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

  • C API (bindings/c): the three functions + svs_memory_breakdown_t in svs_c.h; interface virtuals + concrete overrides in src/index.hpp; impls in src/svs_c.cpp.
  • Core / orchestrator: brings in get_memory_breakdown() (MemoryBreakdown struct + capacity-based svs::data::detail::dataset_allocated_bytes helper) on VamanaIndex / MutableVamanaIndex and through the orchestrator, plus an element_size() accessor parallel to dimensions(). 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 on dev/c-api, this reduces to just the C API layer.

Tests

bindings/c/tests/c_api_index.cpp (static) and c_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.

- 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).
@mergify

mergify Bot commented Jul 16, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@yuejiaointel yuejiaointel requested a review from rfsaliev July 16, 2026 06:49

@rfsaliev rfsaliev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(); }

@rfsaliev rfsaliev Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@rfsaliev rfsaliev dismissed their stale review July 16, 2026 10:24

Do not want to block the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants