[C-API] Add getter and setter for index threadpool size#305
[C-API] Add getter and setter for index threadpool size#305
Conversation
There was a problem hiding this comment.
Pull request overview
Adds C API support to query and modify the search thread pool size for an SVS index after construction, by persisting thread pool configuration alongside the index implementation.
Changes:
- Added
svs_index_get_num_threads/svs_index_set_num_threadsto the public C API. - Extended C-binding index wrappers to retain a
ThreadPoolBuilderand rebuild the underlying thread pool on resize. - Enhanced
ThreadPoolBuilderwith thread-count querying and resize support.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| bindings/c/src/threadpool.hpp | Adds threadpool kind/user-threadpool tracking plus get_threads_num() and resize() support. |
| bindings/c/src/svs_c.cpp | Implements the new C API getter/setter wrappers. |
| bindings/c/src/index_builder.hpp | Passes ThreadPoolBuilder into built/loaded index wrappers. |
| bindings/c/src/index.hpp | Stores ThreadPoolBuilder inside index wrappers; adds virtual get/set thread count hooks. |
| bindings/c/include/svs/c_api/svs_c.h | Declares and documents the new C API functions. |
bindings/c/src/threadpool.hpp
Outdated
| if (kind == SVS_THREADPOOL_KIND_SINGLE_THREAD) { | ||
| throw std::logic_error("Cannot resize a single-threaded threadpool."); | ||
| } | ||
| if (kind == SVS_THREADPOOL_KIND_CUSTOM) { | ||
| throw std::logic_error("Cannot resize a custom threadpool."); |
There was a problem hiding this comment.
ThreadPoolBuilder::resize() throws std::logic_error for unsupported cases (single-thread/custom). Through wrap_exceptions, this gets reported as SVS_ERROR_RUNTIME, which makes this look like a runtime failure rather than an invalid API request. Consider throwing std::invalid_argument (or a project-specific exception mapped to SVS_ERROR_INVALID_ARGUMENT/SVS_ERROR_NOT_IMPLEMENTED) and also reject new_num_threads == 0 here to prevent constructing an invalid pool from internal callers.
| if (kind == SVS_THREADPOOL_KIND_SINGLE_THREAD) { | |
| throw std::logic_error("Cannot resize a single-threaded threadpool."); | |
| } | |
| if (kind == SVS_THREADPOOL_KIND_CUSTOM) { | |
| throw std::logic_error("Cannot resize a custom threadpool."); | |
| if (new_num_threads == 0) { | |
| throw std::invalid_argument("Thread pool size must be at least 1."); | |
| } | |
| if (kind == SVS_THREADPOOL_KIND_SINGLE_THREAD) { | |
| throw std::invalid_argument("Cannot resize a single-threaded threadpool."); | |
| } | |
| if (kind == SVS_THREADPOOL_KIND_CUSTOM) { | |
| throw std::invalid_argument("Cannot resize a custom threadpool."); |
| EXPECT_ARG_NOT_NULL(index); | ||
| EXPECT_ARG_NOT_NULL(out_num_threads); | ||
| *out_num_threads = index->impl->get_num_threads(); | ||
| return true; |
There was a problem hiding this comment.
This getter uses index->impl without validating it is non-null. Several other C API entry points explicitly check index->impl and return SVS_ERROR_INVALID_ARGUMENT for an invalid handle; consider adding the same guard here to avoid a null dereference and to keep error behavior consistent.
| EXPECT_ARG_NOT_NULL(index); | ||
| EXPECT_ARG_GT_THAN(num_threads, 0); | ||
| index->impl->set_num_threads(num_threads); | ||
| return true; |
There was a problem hiding this comment.
This setter calls index->impl->set_num_threads(...) without verifying index->impl is non-null. To match the existing handle validation pattern (e.g., svs_index_search), add an explicit Invalid index handle check before dereferencing index->impl so misuse fails gracefully instead of crashing.
| /// @brief Set number of threads for search in the index's thread pool | ||
| /// @param index The index handle | ||
| /// @param num_threads The number of threads to set | ||
| /// @param out_err An optional error handle to capture errors | ||
| /// @return true on success, false on failure | ||
| SVS_API bool svs_index_set_num_threads( | ||
| svs_index_h index, size_t num_threads, svs_error_h out_err /*=NULL*/ | ||
| ); |
There was a problem hiding this comment.
The new API docs don’t describe failure modes (e.g., attempting to set threads on an index built with SVS_THREADPOOL_KIND_CUSTOM or SVS_THREADPOOL_KIND_SINGLE_THREAD currently throws). Please document what happens in those cases (error return + expected error code/message), so C API users know when svs_index_set_num_threads is supported.
rfsaliev
force-pushed
the
rfsaliev/c-api-index-num-threads
branch
from
fc0d9fa to
07a2493
Compare
This pull request adds support for dynamically getting and setting the number of threads used by the search thread pool in the SVS index via the C API. It introduces new API functions, extends internal index structures to manage thread pool configuration, and updates index construction to support these features.
API Enhancements:
svs_index_get_num_threadsandsvs_index_set_num_threadsto allow users to query and modify the number of threads used for search operations in an index. [1] [2]Index and Thread Pool Management:
Index,DynamicIndex,IndexVamana, andDynamicIndexVamanaclasses to store aThreadPoolBuilder, enabling dynamic thread pool resizing and thread count retrieval. Implementedget_num_threadsandset_num_threadsmethods for these classes. [1] [2] [3] [4] [5]ThreadPoolBuilderclass to support getting the current number of threads, resizing the thread pool (with error handling for unsupported cases), and tracking the thread pool kind and user-provided thread pools. [1] [2]Index Construction Updates:
ThreadPoolBuilderto index implementations, ensuring the thread pool configuration can be managed after construction. [1] [2] [3] [4]Dependency Inclusion:
threadpool.hppheader inindex.hppto support the new thread pool management features.