[C API] Implement custom allocator interface and related functions for memory management

[rfsaliev]

This pull request introduces a flexible custom allocator interface to the SVS C API, enabling users to specify memory allocation strategies—including default, huge page, or user-defined custom allocators—when building or loading indexes. The changes propagate allocator support throughout the C API, C++ runtime, and index builder infrastructure, ensuring that memory management can be tailored for performance or integration needs.

Key changes include:

C API: Allocator Interface and Integration

• Added a new svs_allocator_kind enum and a detailed svs_allocator_interface struct, allowing users to select or implement custom allocators. Macros and typedefs were also introduced for ease of use. [1] [2]
• New API functions: svs_index_builder_set_allocator and svs_index_builder_set_allocator_custom allow setting the allocator kind or providing a custom allocator implementation for index builders.

C++ Runtime: Custom Allocator Support

• Implemented a CustomAllocator class template in C++ that wraps the C API allocator interface, validates its structure, and provides allocation/deallocation logic. A factory function (make_custom_allocator_handle) is provided for safe construction.
• Updated data builder templates (e.g., for LeanVec, LVQ, Simple, and SQ) to accept allocator handles, ensuring allocator propagation through all builder specializations. [1] [2] [3] [4]

Index Building and Loading: Allocator Propagation

• Modified dynamic and static Vamana index build/load dispatchers and implementations to accept and use allocator handles, ensuring all memory allocations can use the specified allocator. This includes proper rebinding for different value types. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Memory Estimation Improvements

• Updated the memory estimation logic for dynamic Vamana indexes to use the correct allocator and blocking parameters, improving accuracy and consistency with actual index construction.

These changes lay the groundwork for advanced memory management strategies in SVS, improving performance tuning and integration flexibility for users needing custom allocation behavior.

[Copilot]

🟡 Changes recommended

The public allocator interface breaks compatibility, while callback validation, error propagation, and lifetime documentation remain unsafe.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds configurable built-in and custom allocators to C API Vamana index construction and loading.

Changes:

• Introduces the C custom allocator interface and builder configuration APIs.
• Propagates allocator handles through static/dynamic graph and data storage.
• Adds allocator accounting tests and improves dynamic memory estimates.

File summaries

File	Description
include/svs/orchestrators/dynamic_vamana.h	Adds allocator-aware dynamic builds.
include/svs/index/vamana/dynamic_index.h	Builds dynamic graphs with supplied allocators.
include/svs/core/graph.h	Makes graph loading allocator-aware.
include/svs/core/allocator.h	Generalizes erased allocator rebinding.
bindings/c/tests/c_api_test_utils.h	Adds a tracking test allocator.
bindings/c/tests/c_api_index.cpp	Tests static-index allocator configuration.
bindings/c/tests/c_api_dynamic_index.cpp	Tests dynamic-index allocator configuration.
bindings/c/src/svs_c.cpp	Implements C allocator setters.
bindings/c/src/index_builder.hpp	Stores and forwards allocator handles.
bindings/c/src/dispatcher_vamana.hpp	Extends static dispatcher declarations.
bindings/c/src/dispatcher_vamana.cpp	Applies allocators to static data and graphs.
bindings/c/src/dispatcher_dynamic_vamana.hpp	Extends dynamic dispatcher declarations.
bindings/c/src/dispatcher_dynamic_vamana.cpp	Applies allocators and updates estimates.
bindings/c/src/data_builder/sq.hpp	Uses allocator handles for SQ data.
bindings/c/src/data_builder/simple.hpp	Uses allocator handles for simple data.
bindings/c/src/data_builder/lvq.hpp	Uses allocator handles for LVQ data.
bindings/c/src/data_builder/leanvec.hpp	Uses allocator handles for LeanVec data.
bindings/c/src/allocator.hpp	Adapts C callbacks to C++ allocators.
bindings/c/include/svs/c/svs_c.h	Exposes the allocator C API.

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 6
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ethanglaser]

The rebind constraint is on the wrong template parameter. The requires clause validates the source type U, but the dispatch uses the target: rebind_to(datatype_v<T>). It should be HasDataType T and lib::in<T>(AllocatorInterface::rebind_types{}).

As written, rebinding to a type that isn't in rebind_types satisfies the constraint, compiles, and then throws ANNEXCEPTION("Type {} is not supported for this operation!") out of lib::match at runtime (include/svs/lib/meta.h:143) — whereas the old rebind_float / rebind_float16 overloads made that a hard compile error. Same issue on the operator= constraint at line 651. This looks like an unintended side effect of the generalization rather than a deliberate trade.

[ethanglaser]

This constructor is unguarded. The existing templated constructor just below enforces std::is_same_v<typename Impl::value_type, T>; this one enforces nothing. So:

AllocatorHandle<float>{std::make_unique<CustomAllocator<std::byte>>(ops, self)};

compiles, and the resulting handle hands out float* for buffers sized in bytes — a 4x heap overflow. make_custom_allocator_handle uses it correctly, but this is a public constructor on a public header, so nothing stops the next caller from getting it wrong. Could it be private with make_custom_allocator_handle as a friend, or could the value type be carried on AllocatorInterface so this can actually be checked?

[ethanglaser]

Two concerns on this one line.

Hugepages are silently dropped for the graph. Before this PR every C API graph allocation used hugepages: static build via Vamana::build's default Allocator = HugepageAllocator<uint32_t> (include/svs/orchestrators/vamana.h:588), static load via GraphLoader{} -> SimpleGraph<uint32_t, HugepageAllocator<uint32_t>>, and dynamic build via the deduction guide to SimpleBlockedGraph<uint32_t>, which is hardcoded to BlockedData<Idx, Dynamic, HugepageAllocator<Idx>> (include/svs/core/graph/graph.h:450). All three now take this handle, which defaults to svs::lib::Allocator<std::byte> — plain ::operator new. Vector data was already lib::Allocator via MaybeBlockedAlloc, so only the graph regresses, but that's the TLB-sensitive part of the search hot path, and it now applies to every default-configured index rather than being opt-in. HugepageAllocator already falls back to 4 KiB pages when hugepages aren't available (force_ = false), so pointing SVS_ALLOCATOR_KIND_DEFAULT at it should be safe. Is the change intentional? If so, could the PR description carry a benchmark number?

Storing a materialized handle loses the kind. ThreadPoolBuilder, in this same directory, stores {kind, num_threads, user_ops_, user_self_} and constructs the pool in build() (bindings/c/src/threadpool.hpp:150). An AllocatorBuilder following that pattern would keep the kind available to everything else that needs it — see my comment on dispatcher_dynamic_vamana.cpp for the concrete consequence — and would leave room for a svs_index_builder_get_allocator later.

[ethanglaser]

This is the fallout from IndexBuilder storing a live handle rather than a builder: the estimator has no way to know which allocator was configured, so it has to hardcode one. The result is that svs_index_builder_estimate_memory_dynamic is wrong for SVS_ALLOCATOR_KIND_HUGE_PAGE (2 MiB rounding is unaccounted for) and wrong for any custom allocator. dispatch_vamana_memory_estimate (~line 159 of dispatcher_vamana.cpp, unchanged here) is allocator-blind entirely — it still uses SimpleDataBuilder<index_type>{}.

Note that if the motivation for defaulting to lib::Allocator was to make the estimate line up with within_1pct(tracker.live_bytes, memory_usage) in the new tests, that's fixing the estimator's inaccuracy by degrading the runtime. Threading the kind through instead fixes both.

[ethanglaser]

SVS_ALLOCATOR_KIND_CUSTOM is declared in the public enum but unreachable through this function — it lands in default: and reports "Invalid allocator kind", which is actively misleading since it's a documented enumerator. Either give it a targeted message pointing at svs_index_builder_set_allocator_custom, or drop the enumerator from the header.

Related: calling svs_index_builder_set_allocator(builder, DEFAULT, ...) after svs_index_builder_set_allocator_custom silently discards the custom allocator, with no way for the caller to observe which one is currently active.

[ethanglaser]

This rejects struct_size smaller than the current svs_allocator_ops_t, which defeats the purpose of shipping a struct_size field — an older, smaller caller struct is precisely the case that field exists to support. svs_threadpool_interface_ops and svs_id_filter_interface_ops don't check it at all. Three interfaces with three different conventions is worse than any one of them; worth settling on a single rule and documenting it next to the field declaration in svs_c.h.

[ethanglaser]

Non-blocking, but worth a follow-up at least.

wrap_exceptions has no catch (const std::bad_alloc&), and std::bad_alloc derives from std::exception rather than std::runtime_error — so it bypasses the std::runtime_error clause below and lands in the catch-all as SVS_ERROR_UNKNOWN. After this PR that means the same condition reports three different codes depending on the configured allocator:

Allocator	Failure path	Reported code
SVS_ALLOCATOR_KIND_CUSTOM	ops_.allocate returns nullptr -> out_of_memory	SVS_ERROR_OUT_OF_MEMORY
SVS_ALLOCATOR_KIND_DEFAULT	lib::Allocator -> ::operator new throws std::bad_alloc	SVS_ERROR_UNKNOWN
SVS_ALLOCATOR_KIND_HUGE_PAGE	mmap fails -> ANNEXCEPTION (include/svs/core/allocator.h:147)	SVS_ERROR_GENERIC

Note that SVS_ERROR_OUT_OF_MEMORY = 3 has been in the public enum (svs_c.h:53) since before this PR with nothing ever producing it — this makes it reachable for the first time, but only on the path a default-configured caller won't take. Anyone branching on SVS_ERROR_OUT_OF_MEMORY to implement a retry-with-smaller-index policy still won't see it from the default allocator.

Adding catch (const std::bad_alloc& ex) -> SVS_ERROR_OUT_OF_MEMORY, placed above the std::exception clause, would make the code mean what its name says. Deriving out_of_memory from std::bad_alloc instead also works, but it's more awkward than it sounds: std::bad_alloc has no string constructor and its what() is fixed, so you'd have to store the message and override what() yourself. Keeping std::runtime_error as the base and just adding the missing catch clause is the smaller change.

[ethanglaser]

The comment says "use big blocksize to avoid many HugePage allocations," but hugepage_mmap sets MAP_POPULATE on Linux (include/svs/core/allocator.h:136), so 1 << 30 means a ~1 GiB data block and a ~1 GiB graph block get eagerly faulted in — roughly 2 GiB resident, to build an index over 256 vectors of dimension 32. That's a lot to put on CI runners for a smoke test. A modest blocksize gets the same coverage.

[ethanglaser]

Changing from 100, 100 to 32, 50, plus the 4-thread pool added at line 400, changes the configuration for every pre-existing section in this test case — including the memory-accounting ones, which now measure something different than they did before. Why is the change necessary?

[ethanglaser]

svs_index_get_memory_usage returns breakdown.total() = graph_bytes + data_bytes + metadata_bytes (include/svs/index/vamana/index.h:188), but the tracker only ever sees data and graph — so this tolerance is silently absorbing metadata_bytes, which is exactly what the comment above says is excluded. Comparing against svs_index_get_memory_breakdown's data_bytes + graph_bytes instead, as the dynamic test does at bindings/c/tests/c_api_dynamic_index.cpp:854, would let this be an exact equality and would document its own intent.

That said, the dynamic test already excludes metadata and still uses within_1pct (line 840) — with fixed DIMENSION / GRAPH_DEGREE / BLOCK_SIZE the block counts there are deterministic, so what residual is that one absorbing?