From 345b9d18f0e36f0ceec4f0b2b6c93658657ba71d Mon Sep 17 00:00:00 2001 From: ofiryanai Date: Sun, 19 Apr 2026 18:48:23 +0300 Subject: [PATCH 01/18] MOD-14916 Devirtualize HNSW / brute-force search hot path (#937) * MOD-14916 Devirtualize distance + getElement on HNSW search hot path MOD-14916 / LTK perf investigation. Two virtual dispatches per HNSW candidate added between v2.10.21 and v8.2.6 account for a measurable share of the KNN regression observed in the LTK benchmarks (-38% throughput on Intel). Both are removed here with the minimum possible change. V1 - distance computation: Every calcDistance() call goes through IndexCalculatorInterface's vtable to reach DistanceCalculatorCommon, which then calls the underlying SIMD function pointer. The intermediate vtable hop is pure indirection; the concrete calculator class is fixed for the life of an index. Expose the underlying dist_func via a new pure-virtual getDistFunc() on IndexCalculatorInterface, implemented by DistanceCalculatorCommon. Cache the returned function pointer in VecSimIndexAbstract at construction time and call it directly in calcDistance(), bypassing the virtual dispatch. V2 - vector fetch: HNSWIndex::getDataByInternalId and BruteForceIndex::getDataByInternalId call this->vectors->getElement(id), which is virtual through the RawDataContainer base. DataBlocksContainer is the only concrete implementation, and this->vectors is always a DataBlocksContainer (created and owned by VecSimIndexAbstract's constructor). Use a static_cast to DataBlocksContainer* plus a qualified call to DataBlocksContainer::getElement to skip the vtable lookup. No behavior change; per-candidate distance and neighbor-fetch calls on HNSW / brute force search paths become direct function-pointer / direct-member calls. Headers in index_factories, hnsw_serializer, and brute_force_factory compile cleanly. * MOD-14916 Inline DataBlocksContainer::getElement on HNSW search hot path Follow-up to the previous V1/V2 devirt commit. The static_cast+qualified call in getDataByInternalId removed the vtable lookup but left the larger cost on the table: DataBlocksContainer::getElement was still defined in data_blocks_container.cpp, so every per-candidate neighbor fetch still paid a real out-of-line function call and a bounds-checked blocks.at() lookup. Without LTO the compiler could neither inline the body nor hoist the div/mod in the HNSW hot loop. Move the definition into the header as inline and drop the .at() bounds check to match the v2.10.21 baseline, which used unchecked operator[] and was fully inlined into processCandidate. Also add a getDistFunc() override to DistanceCalculatorDummy in test_components.cpp so BUILD_TESTS still compiles after the pure virtual added in the previous commit. (cherry picked from commit 4ca500a6fbac0fde659717f726e8c23442c37e7c) --- src/VecSim/algorithms/brute_force/brute_force.h | 5 ++++- src/VecSim/algorithms/hnsw/hnsw.h | 4 +++- src/VecSim/containers/data_blocks_container.cpp | 5 ----- src/VecSim/containers/data_blocks_container.h | 6 +++++- src/VecSim/spaces/computer/calculator.h | 5 +++++ src/VecSim/vec_sim_index.h | 16 +++++++++++++--- tests/unit/test_components.cpp | 3 +++ 7 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/VecSim/algorithms/brute_force/brute_force.h b/src/VecSim/algorithms/brute_force/brute_force.h index 3be453024..be19f2f4c 100644 --- a/src/VecSim/algorithms/brute_force/brute_force.h +++ b/src/VecSim/algorithms/brute_force/brute_force.h @@ -43,7 +43,10 @@ class BruteForceIndex : public VecSimIndexAbstract { size_t indexCapacity() const override; std::unique_ptr getVectorsIterator() const; const DataType *getDataByInternalId(idType id) const { - return reinterpret_cast(this->vectors->getElement(id)); + // `vectors` is always a DataBlocksContainer; skip the RawDataContainer vtable. + return reinterpret_cast( + static_cast(this->vectors) + ->DataBlocksContainer::getElement(id)); } VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, VecSimQueryParams *queryParams) const override; diff --git a/src/VecSim/algorithms/hnsw/hnsw.h b/src/VecSim/algorithms/hnsw/hnsw.h index e5994d314..1a3776fa2 100644 --- a/src/VecSim/algorithms/hnsw/hnsw.h +++ b/src/VecSim/algorithms/hnsw/hnsw.h @@ -385,7 +385,9 @@ labelType HNSWIndex::getEntryPointLabel() const { template const char *HNSWIndex::getDataByInternalId(idType internal_id) const { - return this->vectors->getElement(internal_id); + // `vectors` is always a DataBlocksContainer; skip the RawDataContainer vtable on the hot path. + return static_cast(this->vectors) + ->DataBlocksContainer::getElement(internal_id); } template diff --git a/src/VecSim/containers/data_blocks_container.cpp b/src/VecSim/containers/data_blocks_container.cpp index bf63b683a..f6010b7db 100644 --- a/src/VecSim/containers/data_blocks_container.cpp +++ b/src/VecSim/containers/data_blocks_container.cpp @@ -38,11 +38,6 @@ RawDataContainer::Status DataBlocksContainer::addElement(const void *element, si return Status::OK; } -const char *DataBlocksContainer::getElement(size_t id) const { - assert(id < element_count); - return blocks.at(id / this->block_size).getElement(id % this->block_size); -} - RawDataContainer::Status DataBlocksContainer::removeElement(size_t id) { assert(id == element_count - 1); // only the last element can be removed blocks.back().popLastElement(); diff --git a/src/VecSim/containers/data_blocks_container.h b/src/VecSim/containers/data_blocks_container.h index c375590f2..74f66de11 100644 --- a/src/VecSim/containers/data_blocks_container.h +++ b/src/VecSim/containers/data_blocks_container.h @@ -40,7 +40,11 @@ class DataBlocksContainer : public VecsimBaseObject, public RawDataContainer { Status addElement(const void *element, size_t id) override; - const char *getElement(size_t id) const override; + // Inlined so the hot search path (via getDataByInternalId) can fold the indexing arithmetic. + const char *getElement(size_t id) const override { + assert(id < element_count); + return blocks[id / block_size].getElement(id % block_size); + } Status removeElement(size_t id) override; diff --git a/src/VecSim/spaces/computer/calculator.h b/src/VecSim/spaces/computer/calculator.h index a82293700..539325d8e 100644 --- a/src/VecSim/spaces/computer/calculator.h +++ b/src/VecSim/spaces/computer/calculator.h @@ -23,6 +23,9 @@ class IndexCalculatorInterface : public VecsimBaseObject { virtual ~IndexCalculatorInterface() = default; virtual DistType calcDistance(const void *v1, const void *v2, size_t dim) const = 0; + + // Raw distance function; cached by the index to skip the vtable on the hot path. + virtual spaces::dist_func_t getDistFunc() const = 0; }; /** @@ -56,4 +59,6 @@ class DistanceCalculatorCommon DistType calcDistance(const void *v1, const void *v2, size_t dim) const override { return this->dist_func(v1, v2, dim); } + + spaces::dist_func_t getDistFunc() const override { return this->dist_func; } }; diff --git a/src/VecSim/vec_sim_index.h b/src/VecSim/vec_sim_index.h index 88eabea69..e5a5183b7 100644 --- a/src/VecSim/vec_sim_index.h +++ b/src/VecSim/vec_sim_index.h @@ -86,6 +86,7 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { RawDataContainer *vectors; // The raw vectors data container. private: IndexCalculatorInterface *indexCalculator; // Distance calculator. + spaces::dist_func_t cachedDistFunc; // Cached dist func, used on the hot path. PreprocessorsContainerAbstract *preprocessors; // Storage and query preprocessors. size_t inputBlobSize; // The size of input vectors/queries blob in bytes. May differ from dim * @@ -124,8 +125,11 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { metric(params.metric), blockSize(params.blockSize ? params.blockSize : DEFAULT_BLOCK_SIZE), lastMode(EMPTY_MODE), isMulti(params.multi), isDisk(params.isDisk), logCallbackCtx(params.logCtx), - indexCalculator(components.indexCalculator), preprocessors(components.preprocessors), - inputBlobSize(params.inputBlobSize), storedDataSize(params.storedDataSize) { + indexCalculator(components.indexCalculator), + cachedDistFunc(components.indexCalculator ? components.indexCalculator->getDistFunc() + : nullptr), + preprocessors(components.preprocessors), inputBlobSize(params.inputBlobSize), + storedDataSize(params.storedDataSize) { assert(VecSimType_sizeof(vecType)); assert(storedDataSize); assert(inputBlobSize); @@ -146,10 +150,16 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { /** * @brief Calculate the distance between two vectors based on index parameters. * + * Uses the cached dist func to avoid the indexCalculator vtable on the hot path. + * + * @note Precondition: @c cachedDistFunc must be non-null. Subclasses that construct + * this index with a null @c indexCalculator (e.g. SVS, which uses its own + * internal distance kernels) must not call this method. + * * @return the distance between the vectors. */ DistType calcDistance(const void *vector_data1, const void *vector_data2) const { - return indexCalculator->calcDistance(vector_data1, vector_data2, this->dim); + return cachedDistFunc(vector_data1, vector_data2, this->dim); } /** diff --git a/tests/unit/test_components.cpp b/tests/unit/test_components.cpp index bd3c8d642..2f91d2411 100644 --- a/tests/unit/test_components.cpp +++ b/tests/unit/test_components.cpp @@ -33,6 +33,9 @@ class DistanceCalculatorDummy : public DistanceCalculatorInterfacedist_func(7); } + + // Dummy uses a non-standard dist func signature, so the standard slot is unavailable. + spaces::dist_func_t getDistFunc() const override { return nullptr; } }; } // namespace dummyCalcultor From 4f9d216585ea03a2a5edb288a0c770c59d5fe36b Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Tue, 23 Jun 2026 08:57:23 -0700 Subject: [PATCH 02/18] initial --- src/VecSim/algorithms/svs/svs.h | 40 +++---- src/VecSim/algorithms/svs/svs_tiered.h | 140 +++++++++++++++---------- src/VecSim/vec_sim_tiered_index.h | 15 +++ 3 files changed, 116 insertions(+), 79 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 8b03514f0..200fdd51a 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -263,11 +263,6 @@ class SVSIndex : public VecSimIndexAbstract, fl } int deleted_num = 0; - if constexpr (!isMulti) { - // SVS index does not support overriding vectors with the same label - // so we have to delete them first if needed - deleted_num = deleteVectorsImpl(labels, n); - } std::span ids(labels, n); auto processed_blob = this->preprocessForBatchStorage(vectors_data, n); @@ -279,6 +274,11 @@ class SVSIndex : public VecSimIndexAbstract, fl // SVS index instance cannot be empty, so we have to construct it at first rows impl_ = initImpl(points, ids); } else { + if constexpr (!isMulti) { + // SVS index does not support overriding vectors with the same label + // so we have to delete them first if needed + deleted_num = deleteVectorsImpl(labels, n); + } // Add new points to existing SVS index impl_->add_points(points, ids); } @@ -287,7 +287,7 @@ class SVSIndex : public VecSimIndexAbstract, fl } int deleteVectorImpl(const labelType label) { - if (indexLabelCount() == 0 || !impl_->has_id(label)) { + if (indexLabelCount() == 0) { return 0; } @@ -302,20 +302,7 @@ class SVSIndex : public VecSimIndexAbstract, fl return 0; } - // SVS fails if we try to delete non-existing entries - std::vector entries_to_delete; - entries_to_delete.reserve(n); - for (size_t i = 0; i < n; i++) { - if (impl_->has_id(labels[i])) { - entries_to_delete.push_back(labels[i]); - } - } - - if (entries_to_delete.size() == 0) { - return 0; - } - - const auto deleted_num = impl_->delete_entries(entries_to_delete); + const auto deleted_num = impl_->delete_entries(std::span{labels, n}); this->markIndexUpdate(deleted_num); return deleted_num; @@ -553,13 +540,16 @@ class SVSIndex : public VecSimIndexAbstract, fl } double getDistanceFrom_Unsafe(labelType label, const void *vector_data) const override { - if (!impl_ || !impl_->has_id(label)) { - return std::numeric_limits::quiet_NaN(); - }; + if (!impl_) return std::numeric_limits::quiet_NaN(); auto query_datum = std::span{static_cast(vector_data), this->dim}; - auto dist = impl_->get_distance(label, query_datum); - return toVecSimDistance(dist); + try { + auto dist = impl_->get_distance(label, query_datum); + return std::isnan(dist) ? std::numeric_limits::quiet_NaN() + : toVecSimDistance(static_cast(dist)); + } catch (const svs::lib::ANNException &) { + return std::numeric_limits::quiet_NaN(); + } } VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 9bce50f89..947db01a1 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -233,7 +233,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::atomic_flag indexGCScheduled = ATOMIC_FLAG_INIT; // Used to prevent running multiple index update jobs in parallel. // Even if update jobs scheduled sequentially, they can be started in parallel. - mutable std::mutex updateJobMutex; + // mutable std::shared_mutex updateJobMutex; // The reason of following container just to properly destroy jobs which not executed yet SVSMultiThreadJob::JobsRegistry uncompletedJobs; @@ -261,7 +261,6 @@ class TieredSVSIndex : public VecSimTieredIndex { VecSimBatchIterator *flat_iterator; VecSimBatchIterator *svs_iterator; - std::shared_lock svs_lock; // On single value indices, this set holds the IDs of the results that were returned from // the flat buffer. @@ -327,7 +326,6 @@ class TieredSVSIndex : public VecSimTieredIndex { void acquire_svs_iterator() { assert(svs_iterator == nullptr); - this->index->mainIndexGuard.lock_shared(); svs_iterator = index->backendIndex->newBatchIterator( this->flat_iterator->getQueryBlob(), queryParams); } @@ -336,7 +334,6 @@ class TieredSVSIndex : public VecSimTieredIndex { if (svs_iterator != nullptr && svs_iterator != depleted()) { delete svs_iterator; svs_iterator = nullptr; - this->index->mainIndexGuard.unlock_shared(); } } @@ -361,7 +358,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::move(allocator)), index(index), flat_results(this->allocator), svs_results(this->allocator), flat_iterator(index->frontendIndex->newBatchIterator(query_vector, queryParams)), - svs_iterator(nullptr), svs_lock(index->mainIndexGuard, std::defer_lock), + svs_iterator(nullptr), returned_results_set(this->allocator) { if (queryParams) { this->queryParams = @@ -548,7 +545,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto index = static_cast *>(idx); assert(index); // prevent parallel updates - std::lock_guard lock(index->updateJobMutex); + std::lock_guard lock(index->updateJobMutex); // Release the scheduled flag to allow scheduling again index->indexUpdateScheduled.clear(); // Update the SVS index @@ -575,7 +572,6 @@ class TieredSVSIndex : public VecSimTieredIndex { auto index = static_cast *>(idx); assert(index); - std::lock_guard lock{index->mainIndexGuard}; // Release the scheduled flag to allow scheduling again index->indexGCScheduled.clear(); @@ -605,7 +601,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto jobs = SVSMultiThreadJob::createJobs( this->allocator, SVS_BATCH_UPDATE_JOB, updateSVSIndexWrapper, this, total_threads, std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); - this->submitJobs(jobs); + this->submitUpdateJobs(jobs); } void scheduleSVSIndexGC() { @@ -678,29 +674,24 @@ class TieredSVSIndex : public VecSimTieredIndex { executeTracingCallback("UpdateJob::before_add_to_svs"); { // lock backend index for writing and add vectors there - std::shared_lock main_shared_lock(this->mainIndexGuard); auto svs_index = GetSVSIndex(); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); if (this->backendIndex->indexSize() == 0) { - // If backend index is empty, we need to initialize it first. svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - // Upgrade to unique lock to set the new impl - main_shared_lock.unlock(); - std::lock_guard lock(this->mainIndexGuard); + // std::lock_guard lock(this->mainIndexGuard); svs_index->setImpl(std::move(impl)); } else { - // Backend index is initialized - just add the vectors. - main_shared_lock.unlock(); - std::lock_guard lock(this->mainIndexGuard); - // Upgrade to unique lock to add vectors + // std::lock_guard lock(this->mainIndexGuard); + // std::shared_lock lock(this->mainIndexGuard); svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); } } + executeTracingCallback("UpdateJob::after_add_to_svs"); // clean-up frontend index { // lock frontend index for writing and delete moved vectors @@ -732,8 +723,6 @@ class TieredSVSIndex : public VecSimTieredIndex { // delete vectors from backend index that were deleted from the frontend index during // the update process. { - std::lock_guard main_lock(this->mainIndexGuard); - std::sort(deleted_labels_during_update.begin(), deleted_labels_during_update.end()); auto it = std::unique(deleted_labels_during_update.begin(), deleted_labels_during_update.end()); @@ -791,7 +780,6 @@ class TieredSVSIndex : public VecSimTieredIndex { // It is ok to lock everything at once for in-place mode, // but we will have to unlock averything before calling updateSVSIndexWrapper() // so make the minimal needed lock here. - std::shared_lock backend_shared_lock(this->mainIndexGuard); // Backend index initialization data have to be buffered for proper // compression/training. if (this->backendIndex->indexSize() == 0) { @@ -807,18 +795,18 @@ class TieredSVSIndex : public VecSimTieredIndex { // ... move vectors to the backend index. if (frontend_index_size >= this->trainingTriggerThreshold) { // updateSVSIndexWrapper() accures it's own locks - backend_shared_lock.unlock(); + // backend_shared_lock.unlock(); // initialize the SVS index synchonously using current thread only updateSVSIndexWrapper(this, 1); } return ret; } else { // backend index is initialized - we can add the vector directly - backend_shared_lock.unlock(); + // backend_shared_lock.unlock(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); // prevent update job from running in parallel and lock any access to the backend // index - std::scoped_lock lock(this->updateJobMutex, this->mainIndexGuard); + std::lock_guard lock(this->updateJobMutex); // Set available thread count to 1 for single vector write-in-place operation. // This maintains the contract that single vector operations use exactly one thread. // TODO: Replace this setNumThreads(1) call with an assertion once we establish @@ -840,15 +828,7 @@ class TieredSVSIndex : public VecSimTieredIndex { } } // Remove vector from the backend index if it exists in case of non-MULTI. - auto label_exists = [&]() { - std::shared_lock lock(this->mainIndexGuard); - return svs_index->isLabelExists(label); - }(); - - if (label_exists) { - std::lock_guard lock(this->mainIndexGuard); - ret -= this->backendIndex->deleteVector(label); - } + ret -= this->backendIndex->deleteVector(label); } { // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); @@ -869,7 +849,6 @@ class TieredSVSIndex : public VecSimTieredIndex { { // If main index is empty then update_threshold is trainingTriggerThreshold, // overwise it is updateTriggerThreshold. - std::shared_lock lock(this->mainIndexGuard); update_threshold = this->backendIndex->indexSize() == 0 ? this->trainingTriggerThreshold : this->updateTriggerThreshold; } @@ -936,15 +915,7 @@ class TieredSVSIndex : public VecSimTieredIndex { ret = this->deleteAndRecordSwaps_Unsafe(label); } - label_exists = [&]() { - std::shared_lock lock(this->mainIndexGuard); - return svs_index->isLabelExists(label); - }(); - - if (label_exists) { - std::lock_guard lock(this->mainIndexGuard); - ret += this->backendIndex->deleteVector(label); - } + ret += this->backendIndex->deleteVector(label); return ret; } size_t getNumMarkedDeleted() const override { @@ -953,13 +924,11 @@ class TieredSVSIndex : public VecSimTieredIndex { size_t indexSize() const override { std::shared_lock flat_lock(this->flatIndexGuard); - std::shared_lock main_lock(this->mainIndexGuard); return this->frontendIndex->indexSize() + this->backendIndex->indexSize(); } size_t indexCapacity() const override { std::shared_lock flat_lock(this->flatIndexGuard); - std::shared_lock main_lock(this->mainIndexGuard); return this->frontendIndex->indexCapacity() + this->backendIndex->indexCapacity(); } @@ -997,7 +966,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // the entire training duration (which can take 40-85s on slow machines). // If the mutex is held, training is actively running, so we report // indexUpdateScheduled = true (BACKGROUND_INDEXING = 1). - std::unique_lock lock(this->updateJobMutex, std::try_to_lock); + std::unique_lock lock(this->updateJobMutex, std::try_to_lock); if (lock.owns_lock()) { svsTieredInfo.indexUpdateScheduled = this->indexUpdateScheduled.test() == VecSimBool_TRUE; @@ -1052,17 +1021,81 @@ class TieredSVSIndex : public VecSimTieredIndex { VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, VecSimQueryParams *queryParams) const override { - // SVS implements it's own distance computation functions which may cause sligthly different - // distance values than VecSim Flat Index does, so we always have to merge results with set. - return this->template topKQueryImp(queryBlob, k, queryParams); + // SVS handles its own internal locking for concurrent search + modification, + // so we don't need mainIndexGuard for backend queries. + this->flatIndexGuard.lock_shared(); + + if (this->frontendIndex->indexSize() == 0) { + this->flatIndexGuard.unlock_shared(); + + auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + return this->backendIndex->topKQuery(processed_query, k, queryParams); + } else { + auto flat_results = this->frontendIndex->topKQuery(queryBlob, k, queryParams); + this->flatIndexGuard.unlock_shared(); + + if (flat_results->code != VecSim_QueryReply_OK) { + assert(flat_results->results.empty()); + return flat_results; + } + + auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + auto main_results = this->backendIndex->topKQuery(processed_query, k, queryParams); + + if (main_results->code != VecSim_QueryReply_OK) { + VecSimQueryReply_Free(flat_results); + assert(main_results->results.empty()); + return main_results; + } + + return merge_result_lists(main_results, flat_results, k); + } } VecSimQueryReply *rangeQuery(const void *queryBlob, double radius, VecSimQueryParams *queryParams, VecSimQueryReply_Order order) const override { - // SVS implements it's own distance computation functions which may cause sligthly different - // distance values than VecSim Flat Index does, so we always have to merge results with set. - return this->template rangeQueryImp(queryBlob, radius, queryParams, order); + // SVS handles its own internal locking for concurrent search + modification, + // so we don't need mainIndexGuard for backend queries. + this->flatIndexGuard.lock_shared(); + + if (this->frontendIndex->indexSize() == 0) { + this->flatIndexGuard.unlock_shared(); + + auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + auto res = this->backendIndex->rangeQuery(processed_query, radius, queryParams); + sort_results(res, order); + return res; + } else { + auto flat_results = this->frontendIndex->rangeQuery(queryBlob, radius, queryParams); + this->flatIndexGuard.unlock_shared(); + + if (flat_results->code != VecSim_QueryReply_OK) { + return flat_results; + } + + auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + auto main_results = + this->backendIndex->rangeQuery(processed_query, radius, queryParams); + + if (BY_SCORE == order) { + sort_results_by_score_then_id(main_results); + sort_results_by_score_then_id(flat_results); + + auto code = main_results->code; + VecSimQueryReply *ret = merge_result_lists(main_results, flat_results, -1); + ret->code = code; + return ret; + } else { // BY_ID + concat_results(main_results, flat_results); + filter_results_by_id(main_results); + return main_results; + } + } } VecSimBatchIterator *newBatchIterator(const void *queryBlob, @@ -1081,7 +1114,6 @@ class TieredSVSIndex : public VecSimTieredIndex { TIERED_LOG(VecSimCommonStrings::LOG_VERBOSE_STRING, "running synchronous GC for tiered SVS index in write-in-place mode"); // In write-in-place mode, we run GC synchronously. - std::lock_guard lock{this->mainIndexGuard}; if (this->backendIndex->indexSize() == 0) { // No need to run GC on an empty index. return; @@ -1099,11 +1131,11 @@ class TieredSVSIndex : public VecSimTieredIndex { void acquireSharedLocks() override { this->flatIndexGuard.lock_shared(); - this->mainIndexGuard.lock_shared(); + // this->mainIndexGuard.lock_shared(); } void releaseSharedLocks() override { - this->mainIndexGuard.unlock_shared(); + // this->mainIndexGuard.unlock_shared(); this->flatIndexGuard.unlock_shared(); } }; diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index d097b1ae4..4f8b109a9 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -50,6 +50,7 @@ class VecSimTieredIndex : public VecSimIndexInterface { mutable std::shared_mutex flatIndexGuard; mutable std::shared_mutex mainIndexGuard; + mutable std::shared_mutex updateJobMutex; void lockMainIndexGuard() const { mainIndexGuard.lock(); #ifdef BUILD_TESTS @@ -76,6 +77,16 @@ class VecSimTieredIndex : public VecSimIndexInterface { jobs.size()); } + void submitUpdateJobs(vecsim_stl::vector &jobs) { + vecsim_stl::vector callbacks(jobs.size(), this->allocator); + for (size_t i = 0; i < jobs.size(); i++) { + callbacks[i] = jobs[i]->Execute; + } + std::shared_lock lock(this->updateJobMutex); + this->SubmitJobsToQueue(this->jobQueue, this->jobQueueCtx, jobs.data(), callbacks.data(), + jobs.size()); + } + /** * @brief Return the union of unique labels in both index tiers (which are not deleted). * This is a debug-only method for tiered indexes that computes the union of labels @@ -181,6 +192,7 @@ VecSimTieredIndex::topKQueryImp(const void *queryBlob, size_ // Simply query the main index and return the results while holding the lock. auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); + assert(false); this->mainIndexGuard.lock_shared(); auto res = this->backendIndex->topKQuery(processed_query, k, queryParams); this->mainIndexGuard.unlock_shared(); @@ -201,6 +213,7 @@ VecSimTieredIndex::topKQueryImp(const void *queryBlob, size_ auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Lock the main index and query it. + assert(false); this->mainIndexGuard.lock_shared(); auto main_results = this->backendIndex->topKQuery(processed_query, k, queryParams); this->mainIndexGuard.unlock_shared(); @@ -263,6 +276,7 @@ VecSimTieredIndex::rangeQueryImp(const void *queryBlob, doub auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Simply query the main index and return the results while holding the lock. + assert(false); this->mainIndexGuard.lock_shared(); auto res = this->backendIndex->rangeQuery(processed_query, radius, queryParams); this->mainIndexGuard.unlock_shared(); @@ -286,6 +300,7 @@ VecSimTieredIndex::rangeQueryImp(const void *queryBlob, doub auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Lock the main index and query it. + assert(false); this->mainIndexGuard.lock_shared(); auto main_results = this->backendIndex->rangeQuery(processed_query, radius, queryParams); this->mainIndexGuard.unlock_shared(); From b5c1c0831cdbf20476ca6280a87d7cb624332ea1 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Fri, 26 Jun 2026 04:17:42 -0700 Subject: [PATCH 03/18] optimization --- src/VecSim/algorithms/svs/svs.h | 15 +- src/VecSim/algorithms/svs/svs_tiered.h | 244 ++++++++++++++++++++++--- src/VecSim/vec_sim_common.h | 1 + src/VecSim/vec_sim_tiered_index.h | 15 -- 4 files changed, 229 insertions(+), 46 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 200fdd51a..9043671a2 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -37,6 +37,7 @@ struct SVSIndexBase { SVSIndexBase() : num_marked_deleted{0} {}; virtual ~SVSIndexBase() = default; + virtual int addVector(const void *vector_data, labelType label) = 0; virtual int addVectors(const void *vectors_data, const labelType *labels, size_t n) = 0; virtual int deleteVectors(const labelType *labels, size_t n) = 0; virtual bool isLabelExists(labelType label) const = 0; @@ -45,7 +46,9 @@ struct SVSIndexBase virtual void setNumThreads(size_t numThreads) = 0; virtual size_t getThreadPoolCapacity() const = 0; virtual bool isCompressed() const = 0; - size_t getNumMarkedDeleted() const { return num_marked_deleted; } + size_t getNumMarkedDeleted() const { + return num_marked_deleted.load(std::memory_order_relaxed); + } // Abstract handler to manage SVS implementation instance // declared to avoid unsafe unique_ptr usage @@ -62,7 +65,7 @@ struct SVSIndexBase protected: // Index marked deleted vectors counter to initiate reindexing if it exceeds threshold // markIndexUpdate() manages this counter - size_t num_marked_deleted; + std::atomic num_marked_deleted; }; /** Thread Management Strategy: @@ -316,11 +319,11 @@ class SVSIndex : public VecSimIndexAbstract, fl // SVS index instance should not be empty if (indexLabelCount() == 0) { this->impl_.reset(); - num_marked_deleted = 0; + num_marked_deleted.store(0, std::memory_order_relaxed); return; } - num_marked_deleted += n; + num_marked_deleted.fetch_add(n, std::memory_order_relaxed); } bool isTwoLevelLVQ(const VecSimSvsQuantBits &qbits) { @@ -506,7 +509,7 @@ class SVSIndex : public VecSimIndexAbstract, fl // Enforce single-threaded execution for single vector operations to ensure optimal // performance and consistent behavior. Callers must set numThreads=1 before calling this // method. - assert(getNumThreads() == 1 && "Can't use more than one thread to insert a single vector"); + // assert(getNumThreads() == 1 && "Can't use more than one thread to insert a single vector"); return addVectorsImpl(vector_data, &label, 1); } @@ -721,7 +724,7 @@ class SVSIndex : public VecSimIndexAbstract, fl // https://intel.github.io/ScalableVectorSearch/python/dynamic.html#svs.DynamicVamana.compact impl_->compact(); } - num_marked_deleted = 0; + num_marked_deleted.store(0, std::memory_order_relaxed); } #ifdef BUILD_TESTS diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 947db01a1..82fd33b2c 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -13,6 +13,19 @@ #include #include +/** + * Definition of a job that inserts a new vector from flat into SVS Index. + */ +struct SVSInsertJob : public AsyncJob { + labelType label; + idType id; + + SVSInsertJob(std::shared_ptr allocator, labelType label_, idType id_, + JobCallback insertCb, VecSimIndex *index_) + : AsyncJob(allocator, SVS_INSERT_VECTOR_JOB, insertCb, index_), label(label_), id(id_) {} +}; + + /** * @class SVSMultiThreadJob * @brief Represents a multi-threaded asynchronous job for the SVS algorithm. @@ -200,10 +213,11 @@ class SVSMultiThreadJob : public AsyncJob { template class TieredSVSIndex : public VecSimTieredIndex { + using DistType = float; using Self = TieredSVSIndex; - using Base = VecSimTieredIndex; - using flat_index_t = BruteForceIndex; - using backend_index_t = VecSimIndexAbstract; + using Base = VecSimTieredIndex; + using flat_index_t = BruteForceIndex; + using backend_index_t = VecSimIndexAbstract; using svs_index_t = SVSIndexBase; // swaps_journal is used by updateSVSIndex() to track vectors swap operations that were done in @@ -233,11 +247,22 @@ class TieredSVSIndex : public VecSimTieredIndex { std::atomic_flag indexGCScheduled = ATOMIC_FLAG_INIT; // Used to prevent running multiple index update jobs in parallel. // Even if update jobs scheduled sequentially, they can be started in parallel. - // mutable std::shared_mutex updateJobMutex; + mutable std::shared_mutex updateJobMutex; // The reason of following container just to properly destroy jobs which not executed yet SVSMultiThreadJob::JobsRegistry uncompletedJobs; + std::atomic backendReady{false}; + std::atomic backendInitSubmited{false}; + + vecsim_stl::unordered_map> labelToInsertJobs; + // A mapping to hold invalid jobs, so we can dispose them upon index deletion. + vecsim_stl::unordered_map invalidJobs; + idType currInvalidJobId; // A unique arbitrary identifier for accessing invalid jobs + std::mutex invalidJobsLookupGuard; + + size_t flat_buffer_bound; + /// //////////////////////////////////////////////////////////////////////////////////////////////////// // TieredSVS_BatchIterator // @@ -546,10 +571,17 @@ class TieredSVSIndex : public VecSimTieredIndex { assert(index); // prevent parallel updates std::lock_guard lock(index->updateJobMutex); - // Release the scheduled flag to allow scheduling again - index->indexUpdateScheduled.clear(); // Update the SVS index index->updateSVSIndex(availableThreads); + // Release the scheduled flag to allow scheduling again + index->indexUpdateScheduled.clear(); + } + + static void executeInsertJobWrapper(AsyncJob *job) { + auto *insert_job = static_cast(job); + auto *job_index = static_cast *>(insert_job->index); + job_index->executeInsertJob(insert_job); + delete job; } /** @@ -591,6 +623,21 @@ class TieredSVSIndex : public VecSimTieredIndex { #ifdef BUILD_TESTS public: #endif + + void updateInsertJobInternalId(idType prev_id, idType new_id, labelType label) { + // Update the pending job id, due to a swap that was caused after the removal of new_id. + assert(new_id != INVALID_ID && prev_id != INVALID_ID); + auto it = this->labelToInsertJobs.find(label); + if (it != this->labelToInsertJobs.end()) { + // There is a pending job for the label of the swapped last id - update its id. + for (SVSInsertJob *job_it : it->second) { + if (job_it->id == prev_id) { + job_it->id = new_id; + } + } + } + } + void scheduleSVSIndexUpdate() { // do not schedule if scheduled already if (indexUpdateScheduled.test_and_set()) { @@ -601,7 +648,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto jobs = SVSMultiThreadJob::createJobs( this->allocator, SVS_BATCH_UPDATE_JOB, updateSVSIndexWrapper, this, total_threads, std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); - this->submitUpdateJobs(jobs); + this->submitJobs(jobs); } void scheduleSVSIndexGC() { @@ -648,6 +695,73 @@ class TieredSVSIndex : public VecSimTieredIndex { } } + idType setAndSaveInvalidJob(AsyncJob *job) { + this->invalidJobsLookupGuard.lock(); + job->isValid = false; + idType curInvalidId = currInvalidJobId++; + this->invalidJobs.insert({curInvalidId, job}); + this->invalidJobsLookupGuard.unlock(); + return curInvalidId; + } + + void executeInsertJob(SVSInsertJob *job) { + assert(this->backendReady.load(std::memory_order_acquire)); + + // Note that accessing the job fields should occur with flat index guard held (here and later). + this->flatIndexGuard.lock_shared(); + if (!job->isValid) { + this->flatIndexGuard.unlock_shared(); + // Job has been invalidated in the meantime - nothing to execute, and remove it from the + // lookup. + this->invalidJobsLookupGuard.lock(); + this->invalidJobs.erase(job->id); + this->invalidJobsLookupGuard.unlock(); + return; + } + this->flatIndexGuard.unlock_shared(); + + auto svs_index = GetSVSIndex(); + auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); + svs_index->addVector(storage_blob.get(), job->label); + + // Remove the vector and the insert job from the flat buffer. + this->flatIndexGuard.lock(); + // The job might have been invalidated due to overwrite in the meantime. In this case, + // it was already deleted and the job has been evicted. Otherwise, we need to do it now. + if (job->isValid) { + // Remove the job pointer from the labelToInsertJobs mapping. + auto &jobs = labelToInsertJobs.at(job->label); + for (size_t i = 0; i < jobs.size(); i++) { + if (jobs[i]->id == job->id) { + jobs.erase(jobs.begin() + (long)i); + break; + } + } + if (labelToInsertJobs.at(job->label).empty()) { + labelToInsertJobs.erase(job->label); + } + // Remove the vector from the flat buffer. + // The flat buffer stores data in a contiguous + // array, so deleting an element may move the last element into the freed slot to keep + // ids dense. Capture the last id's label beforehand (after deletion it is no longer in + // the lookup) so we can later fix up its insert job. + labelType last_vec_label = + this->frontendIndex->getVectorLabel(this->frontendIndex->indexSize() - 1); + int deleted = this->frontendIndex->deleteVectorById(job->label, job->id); + if (deleted && job->id != this->frontendIndex->indexSize()) { + // If the vector removal caused a swap with the last id, update the relevant insert job. + this->updateInsertJobInternalId(this->frontendIndex->indexSize(), job->id, + last_vec_label); + } + } else { + // Remove the current job from the invalid jobs' lookup, as we are about to delete it now. + this->invalidJobsLookupGuard.lock(); + this->invalidJobs.erase(job->id); + this->invalidJobsLookupGuard.unlock(); + } + this->flatIndexGuard.unlock(); + } + void updateSVSIndex(size_t availableThreads) { std::vector labels_to_move; std::vector vectors_to_move; @@ -658,6 +772,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto flat_index = this->GetFlatIndex(); const auto frontend_index_size = this->frontendIndex->indexSize(); + // fprintf(stderr, "updateSVSIndex: frontend_index_size = %ld\n", frontend_index_size); const size_t dim = flat_index->getDim(); labels_to_move.reserve(frontend_index_size); vectors_to_move.reserve(frontend_index_size * dim); @@ -681,12 +796,9 @@ class TieredSVSIndex : public VecSimTieredIndex { auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - // std::lock_guard lock(this->mainIndexGuard); svs_index->setImpl(std::move(impl)); } else { - // std::lock_guard lock(this->mainIndexGuard); - // std::shared_lock lock(this->mainIndexGuard); - svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); + // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); } @@ -709,6 +821,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // improvement int deleted = 0; idType id = labels_to_move.size(); + // fprintf(stderr, "updateSVSIndexMidle: frontend_index_size = %ld\n", this->frontendIndex->indexSize()); while (id-- > 0) { auto label = labels_to_move[id]; // Delete the vector from the frontend index if not in-place updated. @@ -716,6 +829,7 @@ class TieredSVSIndex : public VecSimTieredIndex { deleted += this->frontendIndex->deleteVectorById(label, id); } } + // fprintf(stderr, "updateSVSIndexEnd: frontend_index_size = %ld\n", this->frontendIndex->indexSize()); assert(deleted == std::count_if(labels_to_move.begin(), labels_to_move.end(), [](labelType label) { return label != SKIP_LABEL; }) && "Deleted vectors count does not match the number of labels to delete"); @@ -731,6 +845,7 @@ class TieredSVSIndex : public VecSimTieredIndex { svs_index->deleteVectors(deleted_labels_during_update.data(), deleted_labels_during_update.size()); } + this->backendReady.store(true, std::memory_order_release); } public: @@ -738,11 +853,14 @@ class TieredSVSIndex : public VecSimTieredIndex { const TieredIndexParams &tiered_index_params, std::shared_ptr allocator) : Base(svs_index, bf_index, tiered_index_params, allocator), - uncompletedJobs(this->allocator) { + uncompletedJobs(this->allocator), + labelToInsertJobs(this->allocator), + invalidJobs(this->allocator), + currInvalidJobId(0) { const auto &tiered_svs_params = tiered_index_params.specificParams.tieredSVSParams; // If flatBufferLimit is not initialized (0), use the default update threshold. - const size_t flat_buffer_bound = tiered_index_params.flatBufferLimit == 0 + flat_buffer_bound = tiered_index_params.flatBufferLimit == 0 ? SVS_VAMANA_DEFAULT_UPDATE_THRESHOLD : tiered_index_params.flatBufferLimit; @@ -830,7 +948,9 @@ class TieredSVSIndex : public VecSimTieredIndex { // Remove vector from the backend index if it exists in case of non-MULTI. ret -= this->backendIndex->deleteVector(label); } - { // Add vector to the frontend index. + + if (!this->backendInitSubmited.load(std::memory_order_acquire)) { + // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); const auto ft_ret = this->frontendIndex->addVector(blob, label); @@ -843,20 +963,94 @@ class TieredSVSIndex : public VecSimTieredIndex { deleted_labels_journal.push_back(label); } ret = std::max(ret + ft_ret, 0); - // Check frontend index size to determine if an update job schedule is needed. - frontend_index_size = this->frontendIndex->indexSize(); - } - { - // If main index is empty then update_threshold is trainingTriggerThreshold, - // overwise it is updateTriggerThreshold. - update_threshold = this->backendIndex->indexSize() == 0 ? this->trainingTriggerThreshold - : this->updateTriggerThreshold; + + if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { + this->backendInitSubmited.store(true, std::memory_order_release); + scheduleSVSIndexUpdate(); + // fprintf(stderr, "Init submited: this->frontendIndex->indexSize() = %ld\n", this->frontendIndex->indexSize()); + } + return ret; } - if (frontend_index_size >= update_threshold) { - scheduleSVSIndexUpdate(); + + // fprintf(stderr, "Waiting for backend ready\t"); + // spin-lock, while backend is initilizing + while (!this->backendReady.load(std::memory_order_acquire)) { + __builtin_ia32_pause(); } + // fprintf(stderr, "done: this->frontendIndex->indexSize() = %ld\n", this->frontendIndex->indexSize()); - return ret; + if (this->frontendIndex->indexSize() >= flat_buffer_bound) { + auto storage_blob = this->frontendIndex->preprocessForStorage(blob); + return ret = svs_index->addVector(storage_blob.get(), label); + } else { + this->flatIndexGuard.lock(); + idType new_flat_id = this->frontendIndex->indexSize(); + if (this->frontendIndex->isLabelExists(label) && !this->frontendIndex->isMultiValue()) { + // Overwrite the vector and invalidate its only pending job (since we are not in MULTI). + auto *old_job = this->labelToInsertJobs.at(label).at(0); + old_job->id = this->setAndSaveInvalidJob(old_job); + this->labelToInsertJobs.erase(label); + ret = 0; + // We are going to update the internal id that currently holds the vector associated with + // the given label. + new_flat_id = + dynamic_cast *>(this->frontendIndex) + ->getIdOfLabel(label); + // If we are adding a new element (rather than updating an exiting one) we may need to + // increase index capacity. + } + // If this label already exists, this will do overwrite. + this->frontendIndex->addVector(blob, label); + + AsyncJob *new_insert_job = new (this->allocator) + SVSInsertJob(this->allocator, label, new_flat_id, executeInsertJobWrapper, this); + // Save a pointer to the job, so that if the vector is overwritten, we'll have an indication. + if (this->labelToInsertJobs.find(label) != this->labelToInsertJobs.end()) { + // There's already a pending insert job for this label, add another one (without overwrite, + // only possible in multi index) + assert(this->backendIndex->isMultiValue()); + this->labelToInsertJobs.at(label).push_back((SVSInsertJob *)new_insert_job); + } else { + vecsim_stl::vector new_jobs_vec(1, (SVSInsertJob *)new_insert_job, + this->allocator); + this->labelToInsertJobs.insert({label, new_jobs_vec}); + } + this->flatIndexGuard.unlock(); + + // Insert job to the queue and signal the workers' updater. + this->submitSingleJob(new_insert_job); + return ret; + // } else { + // // // Add vector to the frontend index. + // // std::lock_guard lock(this->flatIndexGuard); + // // const auto ft_ret = this->frontendIndex->addVector(blob, label); + + // // if (ft_ret == 0) { // Vector was overriden - add 'skiping' swap to the journal. + // // assert(!this->backendIndex->isMultiValue() && + // // "addVector() may return 0 for single value indices only"); + // // for (auto id : this->frontendIndex->getElementIds(label)) { + // // this->swaps_journal.emplace_back(SKIP_LABEL, id, id); + // // } + // // deleted_labels_journal.push_back(label); + // // } + // // ret = std::max(ret + ft_ret, 0); + + // // // Check frontend index size to determine if an update job schedule is needed. + // // frontend_index_size = this->frontendIndex->indexSize(); + // // // if (this->backendInitSubmited.load(std::memory_order_acquire)) { + // // // if (frontend_index_size >= this->updateTriggerThreshold) { + // // // if (this->backendReady.load(std::memory_order_acquire)) { + // // // scheduleSVSIndexUpdate(); + // // // } + // // // } + // // // } else { + // // if (frontend_index_size >= this->trainingTriggerThreshold) { + // // this->backendInitSubmited.store(true, std::memory_order_release); + // // scheduleSVSIndexUpdate(); + // // } + // // // } + // // return ret; + } } int deleteAndRecordSwaps_Unsafe(labelType label) { diff --git a/src/VecSim/vec_sim_common.h b/src/VecSim/vec_sim_common.h index 82aceaa1c..03e7870f3 100644 --- a/src/VecSim/vec_sim_common.h +++ b/src/VecSim/vec_sim_common.h @@ -273,6 +273,7 @@ typedef enum { HNSW_SWAP_JOB, HNSW_DISK_JOB, SVS_BATCH_UPDATE_JOB, + SVS_INSERT_VECTOR_JOB, SVS_GC_JOB, INVALID_JOB // to indicate that finding a JobType >= INVALID_JOB is an error } JobType; diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index 4f8b109a9..d097b1ae4 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -50,7 +50,6 @@ class VecSimTieredIndex : public VecSimIndexInterface { mutable std::shared_mutex flatIndexGuard; mutable std::shared_mutex mainIndexGuard; - mutable std::shared_mutex updateJobMutex; void lockMainIndexGuard() const { mainIndexGuard.lock(); #ifdef BUILD_TESTS @@ -77,16 +76,6 @@ class VecSimTieredIndex : public VecSimIndexInterface { jobs.size()); } - void submitUpdateJobs(vecsim_stl::vector &jobs) { - vecsim_stl::vector callbacks(jobs.size(), this->allocator); - for (size_t i = 0; i < jobs.size(); i++) { - callbacks[i] = jobs[i]->Execute; - } - std::shared_lock lock(this->updateJobMutex); - this->SubmitJobsToQueue(this->jobQueue, this->jobQueueCtx, jobs.data(), callbacks.data(), - jobs.size()); - } - /** * @brief Return the union of unique labels in both index tiers (which are not deleted). * This is a debug-only method for tiered indexes that computes the union of labels @@ -192,7 +181,6 @@ VecSimTieredIndex::topKQueryImp(const void *queryBlob, size_ // Simply query the main index and return the results while holding the lock. auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); - assert(false); this->mainIndexGuard.lock_shared(); auto res = this->backendIndex->topKQuery(processed_query, k, queryParams); this->mainIndexGuard.unlock_shared(); @@ -213,7 +201,6 @@ VecSimTieredIndex::topKQueryImp(const void *queryBlob, size_ auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Lock the main index and query it. - assert(false); this->mainIndexGuard.lock_shared(); auto main_results = this->backendIndex->topKQuery(processed_query, k, queryParams); this->mainIndexGuard.unlock_shared(); @@ -276,7 +263,6 @@ VecSimTieredIndex::rangeQueryImp(const void *queryBlob, doub auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Simply query the main index and return the results while holding the lock. - assert(false); this->mainIndexGuard.lock_shared(); auto res = this->backendIndex->rangeQuery(processed_query, radius, queryParams); this->mainIndexGuard.unlock_shared(); @@ -300,7 +286,6 @@ VecSimTieredIndex::rangeQueryImp(const void *queryBlob, doub auto processed_query_ptr = this->frontendIndex->preprocessQuery(queryBlob); const void *processed_query = processed_query_ptr.get(); // Lock the main index and query it. - assert(false); this->mainIndexGuard.lock_shared(); auto main_results = this->backendIndex->rangeQuery(processed_query, radius, queryParams); this->mainIndexGuard.unlock_shared(); From 417b970a6304df927cd60310a08cabf7227e9c59 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Fri, 26 Jun 2026 06:06:39 -0700 Subject: [PATCH 04/18] fix threadpool --- src/VecSim/algorithms/svs/svs_tiered.h | 34 +++----------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 82fd33b2c..2657679a4 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -797,10 +797,12 @@ class TieredSVSIndex : public VecSimTieredIndex { labels_to_move.size()); svs_index->setImpl(std::move(impl)); + svs_index->setNumThreads(1); } else { - // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); + svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); + svs_index->setNumThreads(1); } } @@ -1020,36 +1022,6 @@ class TieredSVSIndex : public VecSimTieredIndex { // Insert job to the queue and signal the workers' updater. this->submitSingleJob(new_insert_job); return ret; - // } else { - // // // Add vector to the frontend index. - // // std::lock_guard lock(this->flatIndexGuard); - // // const auto ft_ret = this->frontendIndex->addVector(blob, label); - - // // if (ft_ret == 0) { // Vector was overriden - add 'skiping' swap to the journal. - // // assert(!this->backendIndex->isMultiValue() && - // // "addVector() may return 0 for single value indices only"); - // // for (auto id : this->frontendIndex->getElementIds(label)) { - // // this->swaps_journal.emplace_back(SKIP_LABEL, id, id); - // // } - // // deleted_labels_journal.push_back(label); - // // } - // // ret = std::max(ret + ft_ret, 0); - - // // // Check frontend index size to determine if an update job schedule is needed. - // // frontend_index_size = this->frontendIndex->indexSize(); - // // // if (this->backendInitSubmited.load(std::memory_order_acquire)) { - // // // if (frontend_index_size >= this->updateTriggerThreshold) { - // // // if (this->backendReady.load(std::memory_order_acquire)) { - // // // scheduleSVSIndexUpdate(); - // // // } - // // // } - // // // } else { - // // if (frontend_index_size >= this->trainingTriggerThreshold) { - // // this->backendInitSubmited.store(true, std::memory_order_release); - // // scheduleSVSIndexUpdate(); - // // } - // // // } - // // return ret; } } From 2d5d6999a1dbe990a2756546c8ccef7f109d6a7d Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Fri, 26 Jun 2026 08:42:23 -0700 Subject: [PATCH 05/18] minor fix --- src/VecSim/algorithms/svs/svs_tiered.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 2657679a4..48ea51dc8 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -718,10 +718,10 @@ class TieredSVSIndex : public VecSimTieredIndex { this->invalidJobsLookupGuard.unlock(); return; } + auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); this->flatIndexGuard.unlock_shared(); auto svs_index = GetSVSIndex(); - auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); svs_index->addVector(storage_blob.get(), job->label); // Remove the vector and the insert job from the flat buffer. @@ -1005,7 +1005,8 @@ class TieredSVSIndex : public VecSimTieredIndex { this->frontendIndex->addVector(blob, label); AsyncJob *new_insert_job = new (this->allocator) - SVSInsertJob(this->allocator, label, new_flat_id, executeInsertJobWrapper, this); + SVSInsertJob(this->allocator, label, new_flat_id, executeInsertJobWrapper, this); + // Save a pointer to the job, so that if the vector is overwritten, we'll have an indication. if (this->labelToInsertJobs.find(label) != this->labelToInsertJobs.end()) { // There's already a pending insert job for this label, add another one (without overwrite, From 3c3f842ab0dd8716f0b1e0aade43b17eaa3e55a0 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Mon, 29 Jun 2026 08:42:54 -0700 Subject: [PATCH 06/18] minor cleaning --- src/VecSim/algorithms/svs/svs.h | 4 ++-- src/VecSim/algorithms/svs/svs_tiered.h | 28 ++++++++------------------ 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 9043671a2..67e628d6d 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -351,7 +351,7 @@ class SVSIndex : public VecSimIndexAbstract, fl svs_details::getOrDefault(params.leanvec_dim, SVS_VAMANA_DEFAULT_LEANVEC_DIM)}, epsilon{svs_details::getOrDefault(params.epsilon, SVS_VAMANA_DEFAULT_EPSILON)}, is_two_level_lvq{isTwoLevelLVQ(params.quantBits)}, - threadpool_{std::max(size_t{SVS_VAMANA_DEFAULT_NUM_THREADS}, params.num_threads)}, + threadpool_{1}, impl_{nullptr} { logger_ = makeLogger(); } @@ -509,7 +509,7 @@ class SVSIndex : public VecSimIndexAbstract, fl // Enforce single-threaded execution for single vector operations to ensure optimal // performance and consistent behavior. Callers must set numThreads=1 before calling this // method. - // assert(getNumThreads() == 1 && "Can't use more than one thread to insert a single vector"); + assert(getNumThreads() == 1 && "Can't use more than one thread to insert a single vector"); return addVectorsImpl(vector_data, &label, 1); } diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 48ea51dc8..66c8ce530 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -792,17 +792,17 @@ class TieredSVSIndex : public VecSimTieredIndex { auto svs_index = GetSVSIndex(); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); if (this->backendIndex->indexSize() == 0) { - svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); + // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); svs_index->setImpl(std::move(impl)); - svs_index->setNumThreads(1); + // svs_index->setNumThreads(1); } else { - svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); + // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - svs_index->setNumThreads(1); + // svs_index->setNumThreads(1); } } @@ -938,19 +938,6 @@ class TieredSVSIndex : public VecSimTieredIndex { assert(this->getWriteMode() != VecSim_WriteInPlace && "InPlace mode returns early"); // Async mode - add vector to the frontend index and schedule an update job if needed. - if (!this->backendIndex->isMultiValue()) { - { - std::shared_lock lock(this->flatIndexGuard); - // If the label already exists in the frontend index, we should count it - // to prevent the case when existing vector is moved meanwhile by the update job. - if (this->frontendIndex->isLabelExists(label)) { - ret = -1; - } - } - // Remove vector from the backend index if it exists in case of non-MULTI. - ret -= this->backendIndex->deleteVector(label); - } - if (!this->backendInitSubmited.load(std::memory_order_acquire)) { // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); @@ -969,22 +956,23 @@ class TieredSVSIndex : public VecSimTieredIndex { if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { this->backendInitSubmited.store(true, std::memory_order_release); scheduleSVSIndexUpdate(); - // fprintf(stderr, "Init submited: this->frontendIndex->indexSize() = %ld\n", this->frontendIndex->indexSize()); } return ret; } - // fprintf(stderr, "Waiting for backend ready\t"); // spin-lock, while backend is initilizing while (!this->backendReady.load(std::memory_order_acquire)) { __builtin_ia32_pause(); } - // fprintf(stderr, "done: this->frontendIndex->indexSize() = %ld\n", this->frontendIndex->indexSize()); + this->flatIndexGuard.lock_shared(); if (this->frontendIndex->indexSize() >= flat_buffer_bound) { + this->flatIndexGuard.unlock_shared(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); + return ret = svs_index->addVector(storage_blob.get(), label); } else { + this->flatIndexGuard.unlock_shared(); this->flatIndexGuard.lock(); idType new_flat_id = this->frontendIndex->indexSize(); if (this->frontendIndex->isLabelExists(label) && !this->frontendIndex->isMultiValue()) { From 952efbf76ea788885a5e61b34a94cdb6d1004fae Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Wed, 1 Jul 2026 04:47:03 -0700 Subject: [PATCH 07/18] clean up --- src/VecSim/algorithms/svs/svs_tiered.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 66c8ce530..909da8266 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -772,7 +772,6 @@ class TieredSVSIndex : public VecSimTieredIndex { auto flat_index = this->GetFlatIndex(); const auto frontend_index_size = this->frontendIndex->indexSize(); - // fprintf(stderr, "updateSVSIndex: frontend_index_size = %ld\n", frontend_index_size); const size_t dim = flat_index->getDim(); labels_to_move.reserve(frontend_index_size); vectors_to_move.reserve(frontend_index_size * dim); @@ -792,17 +791,13 @@ class TieredSVSIndex : public VecSimTieredIndex { auto svs_index = GetSVSIndex(); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); if (this->backendIndex->indexSize() == 0) { - // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); svs_index->setImpl(std::move(impl)); - // svs_index->setNumThreads(1); } else { - // svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - // svs_index->setNumThreads(1); } } @@ -823,7 +818,6 @@ class TieredSVSIndex : public VecSimTieredIndex { // improvement int deleted = 0; idType id = labels_to_move.size(); - // fprintf(stderr, "updateSVSIndexMidle: frontend_index_size = %ld\n", this->frontendIndex->indexSize()); while (id-- > 0) { auto label = labels_to_move[id]; // Delete the vector from the frontend index if not in-place updated. @@ -831,7 +825,6 @@ class TieredSVSIndex : public VecSimTieredIndex { deleted += this->frontendIndex->deleteVectorById(label, id); } } - // fprintf(stderr, "updateSVSIndexEnd: frontend_index_size = %ld\n", this->frontendIndex->indexSize()); assert(deleted == std::count_if(labels_to_move.begin(), labels_to_move.end(), [](labelType label) { return label != SKIP_LABEL; }) && "Deleted vectors count does not match the number of labels to delete"); @@ -915,14 +908,12 @@ class TieredSVSIndex : public VecSimTieredIndex { // ... move vectors to the backend index. if (frontend_index_size >= this->trainingTriggerThreshold) { // updateSVSIndexWrapper() accures it's own locks - // backend_shared_lock.unlock(); // initialize the SVS index synchonously using current thread only updateSVSIndexWrapper(this, 1); } return ret; } else { // backend index is initialized - we can add the vector directly - // backend_shared_lock.unlock(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); // prevent update job from running in parallel and lock any access to the backend // index @@ -1286,11 +1277,9 @@ class TieredSVSIndex : public VecSimTieredIndex { void acquireSharedLocks() override { this->flatIndexGuard.lock_shared(); - // this->mainIndexGuard.lock_shared(); } void releaseSharedLocks() override { - // this->mainIndexGuard.unlock_shared(); this->flatIndexGuard.unlock_shared(); } }; From 8265a7a15c43c932b080e702a8200de0adf1b2e7 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Wed, 1 Jul 2026 06:25:52 -0700 Subject: [PATCH 08/18] remove dead code --- deps/ScalableVectorSearch | 2 +- src/VecSim/algorithms/svs/svs.h | 13 +++++-------- src/VecSim/algorithms/svs/svs_tiered.h | 14 ++++++-------- 3 files changed, 12 insertions(+), 17 deletions(-) mode change 160000 => 120000 deps/ScalableVectorSearch diff --git a/deps/ScalableVectorSearch b/deps/ScalableVectorSearch deleted file mode 160000 index 02dea9d9c..000000000 --- a/deps/ScalableVectorSearch +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 02dea9d9cd1c73dc03dc0fd91d14a548f1aece70 diff --git a/deps/ScalableVectorSearch b/deps/ScalableVectorSearch new file mode 120000 index 000000000..25f58fd1a --- /dev/null +++ b/deps/ScalableVectorSearch @@ -0,0 +1 @@ +/home/drazdobu/libraries.ai.vector-search.svs \ No newline at end of file diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 67e628d6d..a3f2ca8c5 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -543,16 +543,13 @@ class SVSIndex : public VecSimIndexAbstract, fl } double getDistanceFrom_Unsafe(labelType label, const void *vector_data) const override { - if (!impl_) return std::numeric_limits::quiet_NaN(); + if (!impl_ || !impl_->has_id(label)) { + return std::numeric_limits::quiet_NaN(); + }; auto query_datum = std::span{static_cast(vector_data), this->dim}; - try { - auto dist = impl_->get_distance(label, query_datum); - return std::isnan(dist) ? std::numeric_limits::quiet_NaN() - : toVecSimDistance(static_cast(dist)); - } catch (const svs::lib::ANNException &) { - return std::numeric_limits::quiet_NaN(); - } + auto dist = impl_->get_distance(label, query_datum); + return toVecSimDistance(dist); } VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 909da8266..383734248 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -247,7 +247,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::atomic_flag indexGCScheduled = ATOMIC_FLAG_INIT; // Used to prevent running multiple index update jobs in parallel. // Even if update jobs scheduled sequentially, they can be started in parallel. - mutable std::shared_mutex updateJobMutex; + mutable std::mutex updateJobMutex; // The reason of following container just to properly destroy jobs which not executed yet SVSMultiThreadJob::JobsRegistry uncompletedJobs; @@ -570,11 +570,11 @@ class TieredSVSIndex : public VecSimTieredIndex { auto index = static_cast *>(idx); assert(index); // prevent parallel updates - std::lock_guard lock(index->updateJobMutex); - // Update the SVS index - index->updateSVSIndex(availableThreads); + std::lock_guard lock(index->updateJobMutex); // Release the scheduled flag to allow scheduling again index->indexUpdateScheduled.clear(); + // Update the SVS index + index->updateSVSIndex(availableThreads); } static void executeInsertJobWrapper(AsyncJob *job) { @@ -885,7 +885,6 @@ class TieredSVSIndex : public VecSimTieredIndex { int addVector(const void *blob, labelType label) override { int ret = 0; auto svs_index = GetSVSIndex(); - size_t update_threshold = 0; size_t frontend_index_size = 0; // In-Place mode - add vector syncronously to the backend index. @@ -917,7 +916,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto storage_blob = this->frontendIndex->preprocessForStorage(blob); // prevent update job from running in parallel and lock any access to the backend // index - std::lock_guard lock(this->updateJobMutex); + std::lock_guard lock(this->updateJobMutex); // Set available thread count to 1 for single vector write-in-place operation. // This maintains the contract that single vector operations use exactly one thread. // TODO: Replace this setNumThreads(1) call with an assertion once we establish @@ -1047,7 +1046,6 @@ class TieredSVSIndex : public VecSimTieredIndex { int deleteVector(labelType label) override { int ret = 0; - auto svs_index = GetSVSIndex(); // Backend index deletions to be synchronized with the frontend index, // elsewhere there is the risk of labels duplication in both indices which can lead to wrong // results of topK queries. In such case we should behave as if InPlace mode is always set. @@ -1112,7 +1110,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // the entire training duration (which can take 40-85s on slow machines). // If the mutex is held, training is actively running, so we report // indexUpdateScheduled = true (BACKGROUND_INDEXING = 1). - std::unique_lock lock(this->updateJobMutex, std::try_to_lock); + std::unique_lock lock(this->updateJobMutex, std::try_to_lock); if (lock.owns_lock()) { svsTieredInfo.indexUpdateScheduled = this->indexUpdateScheduled.test() == VecSimBool_TRUE; From 71c92bdc5d46ece42939ccd8f372036222efba1b Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Wed, 8 Jul 2026 00:30:16 -0700 Subject: [PATCH 09/18] fixes; tests pass --- src/VecSim/algorithms/svs/svs.h | 285 ++++++++++++------ .../algorithms/svs/svs_serializer_impl.h | 1 + src/VecSim/algorithms/svs/svs_tiered.h | 264 +++++++++------- tests/unit/test_svs_tiered.cpp | 15 +- 4 files changed, 360 insertions(+), 205 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index a3f2ca8c5..7dcdc1d93 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -39,6 +39,7 @@ struct SVSIndexBase virtual ~SVSIndexBase() = default; virtual int addVector(const void *vector_data, labelType label) = 0; virtual int addVectors(const void *vectors_data, const labelType *labels, size_t n) = 0; + virtual int deleteVector(labelType label) = 0; virtual int deleteVectors(const labelType *labels, size_t n) = 0; virtual bool isLabelExists(labelType label) const = 0; virtual size_t indexStorageSize() const = 0; @@ -46,6 +47,8 @@ struct SVSIndexBase virtual void setNumThreads(size_t numThreads) = 0; virtual size_t getThreadPoolCapacity() const = 0; virtual bool isCompressed() const = 0; + virtual bool ready() const = 0; + size_t getNumMarkedDeleted() const { return num_marked_deleted.load(std::memory_order_relaxed); } @@ -116,6 +119,17 @@ class SVSIndex : public VecSimIndexAbstract, fl svs::logging::logger_ptr logger_; // SVS Index implementation instance std::unique_ptr impl_; + mutable std::shared_mutex pimplGuard_; + + std::atomic impl_ready_{false}; + + void setReady() { + this->impl_ready_.store(true, std::memory_order_release); + } + + void setUnready() { + this->impl_ready_.store(false, std::memory_order_release); + } static double toVecSimDistance(float v) { return svs_details::toVecSimDistance(v); } @@ -243,7 +257,7 @@ class SVSIndex : public VecSimIndexAbstract, fl } void setImpl(std::unique_ptr handler) override { - if (impl_ != nullptr) { + if (ready()) { throw std::logic_error("SVSIndex::setImpl called on non-empty impl_"); } @@ -251,7 +265,11 @@ class SVSIndex : public VecSimIndexAbstract, fl if (!svs_handler) { throw std::logic_error("Failed to cast to SVSImplHandler"); } - this->impl_ = std::move(svs_handler->impl); + { + std::lock_guard lock(this->pimplGuard_); + this->impl_ = std::move(svs_handler->impl); + if (this->impl_) setReady(); + } } // Assuming numThreads was updated to reflect the number of available threads before this @@ -273,16 +291,23 @@ class SVSIndex : public VecSimIndexAbstract, fl // Wrap data into SVS SimpleDataView for SVS API auto points = svs::data::SimpleDataView{typed_vectors_data, n, this->dim}; - if (!impl_) { - // SVS index instance cannot be empty, so we have to construct it at first rows - impl_ = initImpl(points, ids); - } else { - if constexpr (!isMulti) { + if constexpr (!isMulti) { + if (ready()) { // SVS index does not support overriding vectors with the same label // so we have to delete them first if needed deleted_num = deleteVectorsImpl(labels, n); } + } + + if (!ready()) { + // SVS index instance cannot be empty, so we have to construct it at first rows + std::lock_guard lock(this->pimplGuard_); + impl_ = initImpl(points, ids); + assert(this->impl_ != nullptr); + setReady(); + } else { // Add new points to existing SVS index + std::shared_lock lock(this->pimplGuard_); impl_->add_points(points, ids); } @@ -294,7 +319,11 @@ class SVSIndex : public VecSimIndexAbstract, fl return 0; } - const auto deleted_num = impl_->delete_entries(std::span{&label, 1}); + int deleted_num = 0; + { + std::shared_lock lock(this->pimplGuard_); + deleted_num = impl_->delete_entries(std::span{&label, 1}); + } this->markIndexUpdate(deleted_num); return deleted_num; @@ -305,7 +334,11 @@ class SVSIndex : public VecSimIndexAbstract, fl return 0; } - const auto deleted_num = impl_->delete_entries(std::span{labels, n}); + int deleted_num = 0; + { + std::shared_lock lock(this->pimplGuard_); + deleted_num = impl_->delete_entries(std::span{labels, n}); + } this->markIndexUpdate(deleted_num); return deleted_num; @@ -313,12 +346,16 @@ class SVSIndex : public VecSimIndexAbstract, fl // Count deletions and consolidate index if needed void markIndexUpdate(size_t n = 1) { - if (!impl_) + if (!ready()) return; // SVS index instance should not be empty if (indexLabelCount() == 0) { - this->impl_.reset(); + std::lock_guard lock(this->pimplGuard_); + { + setUnready(); + this->impl_.reset(); + } num_marked_deleted.store(0, std::memory_order_relaxed); return; } @@ -351,32 +388,59 @@ class SVSIndex : public VecSimIndexAbstract, fl svs_details::getOrDefault(params.leanvec_dim, SVS_VAMANA_DEFAULT_LEANVEC_DIM)}, epsilon{svs_details::getOrDefault(params.epsilon, SVS_VAMANA_DEFAULT_EPSILON)}, is_two_level_lvq{isTwoLevelLVQ(params.quantBits)}, - threadpool_{1}, + threadpool_{std::max(size_t{SVS_VAMANA_DEFAULT_NUM_THREADS}, params.num_threads)}, impl_{nullptr} { logger_ = makeLogger(); } ~SVSIndex() = default; + bool ready() const override { + return this->impl_ready_.load(std::memory_order_acquire); + } + size_t indexSize() const override { return indexStorageSize(); } - size_t indexStorageSize() const override { return impl_ ? impl_->view_data().size() : 0; } + size_t indexStorageSize() const override { + if (ready()) { + std::shared_lock lock(this->pimplGuard_); + return impl_->view_data().size(); + } else { + return 0; + } + } size_t indexCapacity() const override { - return impl_ ? storage_traits_t::storage_capacity(impl_->view_data()) : 0; + if (ready()) { + std::shared_lock lock(this->pimplGuard_); + return storage_traits_t::storage_capacity(impl_->view_data()); + } else { + return 0; + } } size_t indexLabelCount() const override { if constexpr (isMulti) { - return impl_ ? impl_->labelcount() : 0; + if (ready()) { + std::shared_lock lock(this->pimplGuard_); + return impl_->labelcount(); + } else { + return 0; + } } else { - return impl_ ? impl_->size() : 0; + if (ready()) { + std::shared_lock lock(this->pimplGuard_); + return impl_->size(); + } else { + return 0; + } } } vecsim_stl::set getLabelsSet() const override { vecsim_stl::set labels(this->allocator); - if (impl_) { + if (ready()) { + std::shared_lock lock(this->pimplGuard_); impl_->on_ids([&labels](size_t label) { labels.insert(label); }); } return labels; @@ -528,7 +592,12 @@ class SVSIndex : public VecSimIndexAbstract, fl } bool isLabelExists(labelType label) const override { - return impl_ ? impl_->has_id(label) : false; + if (ready()) { + std::shared_lock lock(this->pimplGuard_); + return impl_->has_id(label); + } else { + return false; + } } size_t getNumThreads() const override { return threadpool_.size(); } @@ -543,13 +612,23 @@ class SVSIndex : public VecSimIndexAbstract, fl } double getDistanceFrom_Unsafe(labelType label, const void *vector_data) const override { - if (!impl_ || !impl_->has_id(label)) { + if (!ready()) { return std::numeric_limits::quiet_NaN(); - }; + } + { + std::shared_lock lock(this->pimplGuard_); + if (!impl_->has_id(label)) { + return std::numeric_limits::quiet_NaN(); + } + } + auto query_datum = std::span{static_cast(vector_data), this->dim}; - auto dist = impl_->get_distance(label, query_datum); - return toVecSimDistance(dist); + { + std::shared_lock lock(this->pimplGuard_); + auto dist = impl_->get_distance(label, query_datum); + return toVecSimDistance(dist); + } } VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, @@ -559,36 +638,38 @@ class SVSIndex : public VecSimIndexAbstract, fl if (k == 0 || this->indexLabelCount() == 0) { return rep; } + { + std::shared_lock lock(this->pimplGuard_); + // limit result size to index size + k = std::min(k, this->indexLabelCount()); + + auto processed_query_ptr = this->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + + auto query = svs::data::ConstSimpleDataView{ + static_cast(processed_query), 1, this->dim}; + auto result = svs::QueryResult{query.size(), k}; + auto sp = svs_details::joinSearchParams(impl_->get_search_parameters(), queryParams, + is_two_level_lvq); + + auto timeoutCtx = queryParams ? queryParams->timeoutCtx : nullptr; + auto cancel = [timeoutCtx]() { return VECSIM_TIMEOUT(timeoutCtx); }; + + impl_->search(result.view(), query, sp, cancel); + if (cancel()) { + rep->code = VecSim_QueryReply_TimedOut; + return rep; + } - // limit result size to index size - k = std::min(k, this->indexLabelCount()); - - auto processed_query_ptr = this->preprocessQuery(queryBlob); - const void *processed_query = processed_query_ptr.get(); - - auto query = svs::data::ConstSimpleDataView{ - static_cast(processed_query), 1, this->dim}; - auto result = svs::QueryResult{query.size(), k}; - auto sp = svs_details::joinSearchParams(impl_->get_search_parameters(), queryParams, - is_two_level_lvq); - - auto timeoutCtx = queryParams ? queryParams->timeoutCtx : nullptr; - auto cancel = [timeoutCtx]() { return VECSIM_TIMEOUT(timeoutCtx); }; - - impl_->search(result.view(), query, sp, cancel); - if (cancel()) { - rep->code = VecSim_QueryReply_TimedOut; - return rep; - } - - assert(result.n_queries() == 1); + assert(result.n_queries() == 1); - const auto n_neighbors = result.n_neighbors(); - rep->results.reserve(n_neighbors); + const auto n_neighbors = result.n_neighbors(); + rep->results.reserve(n_neighbors); - for (size_t i = 0; i < n_neighbors; i++) { - rep->results.push_back( - VecSimQueryResult{result.index(0, i), toVecSimDistance(result.distance(0, i))}); + for (size_t i = 0; i < n_neighbors; i++) { + rep->results.push_back( + VecSimQueryResult{result.index(0, i), toVecSimDistance(result.distance(0, i))}); + } } // Workaround for VecSim merge_results() that expects results to be sorted // by score, then by id from both indices. @@ -604,58 +685,61 @@ class SVSIndex : public VecSimIndexAbstract, fl if (radius == 0 || this->indexLabelCount() == 0) { return rep; } + { + std::shared_lock lock(this->pimplGuard_); + + auto timeoutCtx = queryParams ? queryParams->timeoutCtx : nullptr; + auto cancel = [timeoutCtx]() { return VECSIM_TIMEOUT(timeoutCtx); }; + + // Prepare query blob for SVS + auto processed_query_ptr = this->preprocessQuery(queryBlob); + const void *processed_query = processed_query_ptr.get(); + std::span query{static_cast(processed_query), + this->dim}; + + // Base search parameters for the SVS iterator schedule. + auto sp = svs_details::joinSearchParams(impl_->get_search_parameters(), queryParams, + is_two_level_lvq); + // SVS BatchIterator handles the search in batches + // The batch size is set to the index search window size by default + const size_t batch_size = sp.buffer_config_.get_search_window_size(); + + // Create SVS BatchIterator for range search + // Search result is cached in the iterator and can be accessed by the user + auto svs_it = impl_->make_batch_iterator(query); + svs_it.next(batch_size, cancel); + if (cancel()) { + rep->code = VecSim_QueryReply_TimedOut; + return rep; + } - auto timeoutCtx = queryParams ? queryParams->timeoutCtx : nullptr; - auto cancel = [timeoutCtx]() { return VECSIM_TIMEOUT(timeoutCtx); }; - - // Prepare query blob for SVS - auto processed_query_ptr = this->preprocessQuery(queryBlob); - const void *processed_query = processed_query_ptr.get(); - std::span query{static_cast(processed_query), - this->dim}; - - // Base search parameters for the SVS iterator schedule. - auto sp = svs_details::joinSearchParams(impl_->get_search_parameters(), queryParams, - is_two_level_lvq); - // SVS BatchIterator handles the search in batches - // The batch size is set to the index search window size by default - const size_t batch_size = sp.buffer_config_.get_search_window_size(); - - // Create SVS BatchIterator for range search - // Search result is cached in the iterator and can be accessed by the user - auto svs_it = impl_->make_batch_iterator(query); - svs_it.next(batch_size, cancel); - if (cancel()) { - rep->code = VecSim_QueryReply_TimedOut; - return rep; - } - - // range search using epsilon - const auto epsilon = queryParams && queryParams->svsRuntimeParams.epsilon != 0 - ? queryParams->svsRuntimeParams.epsilon - : this->epsilon; - - const auto range_search_boundaries = radius * (1.0 + std::abs(epsilon)); - bool keep_searching = true; - - // Loop while iterator cache is not empty and search radius + epsilon is not exceeded - while (keep_searching && svs_it.size() > 0) { - // Iterate over the cached search results - for (auto &neighbor : svs_it) { - const auto dist = toVecSimDistance(neighbor.distance()); - if (dist <= radius) { - rep->results.push_back(VecSimQueryResult{neighbor.id(), dist}); - } else if (dist > range_search_boundaries) { - keep_searching = false; + // range search using epsilon + const auto epsilon = queryParams && queryParams->svsRuntimeParams.epsilon != 0 + ? queryParams->svsRuntimeParams.epsilon + : this->epsilon; + + const auto range_search_boundaries = radius * (1.0 + std::abs(epsilon)); + bool keep_searching = true; + + // Loop while iterator cache is not empty and search radius + epsilon is not exceeded + while (keep_searching && svs_it.size() > 0) { + // Iterate over the cached search results + for (auto &neighbor : svs_it) { + const auto dist = toVecSimDistance(neighbor.distance()); + if (dist <= radius) { + rep->results.push_back(VecSimQueryResult{neighbor.id(), dist}); + } else if (dist > range_search_boundaries) { + keep_searching = false; + } } - } - // If search radius + epsilon is not exceeded, request SVS BatchIterator for the next - // batch - if (keep_searching) { - svs_it.next(batch_size, cancel); - if (cancel()) { - rep->code = VecSim_QueryReply_TimedOut; - return rep; + // If search radius + epsilon is not exceeded, request SVS BatchIterator for the next + // batch + if (keep_searching) { + svs_it.next(batch_size, cancel); + if (cancel()) { + rep->code = VecSim_QueryReply_TimedOut; + return rep; + } } } } @@ -713,7 +797,8 @@ class SVSIndex : public VecSimIndexAbstract, fl } void runGC() override { - if (impl_) { + if (ready()) { + std::shared_lock lock(this->pimplGuard_); // There is documentation for consolidate(): // https://intel.github.io/ScalableVectorSearch/python/dynamic.html#svs.DynamicVamana.consolidate impl_->consolidate(); diff --git a/src/VecSim/algorithms/svs/svs_serializer_impl.h b/src/VecSim/algorithms/svs/svs_serializer_impl.h index 2780d3457..6f6757922 100644 --- a/src/VecSim/algorithms/svs/svs_serializer_impl.h +++ b/src/VecSim/algorithms/svs/svs_serializer_impl.h @@ -112,6 +112,7 @@ void SVSIndex distance_f(), std::move(threadpool_handle), false, logger_); impl_ = std::make_unique(std::move(loaded)); } + setReady(); } template { std::atomic_flag indexGCScheduled = ATOMIC_FLAG_INIT; // Used to prevent running multiple index update jobs in parallel. // Even if update jobs scheduled sequentially, they can be started in parallel. - mutable std::mutex updateJobMutex; + mutable std::shared_mutex updateJobMutex; // The reason of following container just to properly destroy jobs which not executed yet SVSMultiThreadJob::JobsRegistry uncompletedJobs; - std::atomic backendReady{false}; std::atomic backendInitSubmited{false}; + std::unordered_set ids_to_init_; vecsim_stl::unordered_map> labelToInsertJobs; // A mapping to hold invalid jobs, so we can dispose them upon index deletion. @@ -570,7 +570,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto index = static_cast *>(idx); assert(index); // prevent parallel updates - std::lock_guard lock(index->updateJobMutex); + std::lock_guard lock(index->updateJobMutex); // Release the scheduled flag to allow scheduling again index->indexUpdateScheduled.clear(); // Update the SVS index @@ -580,6 +580,8 @@ class TieredSVSIndex : public VecSimTieredIndex { static void executeInsertJobWrapper(AsyncJob *job) { auto *insert_job = static_cast(job); auto *job_index = static_cast *>(insert_job->index); + // prevent parallel execution with index initilizing job + std::shared_lock lock(job_index->updateJobMutex); job_index->executeInsertJob(insert_job); delete job; } @@ -705,7 +707,8 @@ class TieredSVSIndex : public VecSimTieredIndex { } void executeInsertJob(SVSInsertJob *job) { - assert(this->backendReady.load(std::memory_order_acquire)); + auto svs_index = GetSVSIndex(); + assert(svs_index->ready()); // Note that accessing the job fields should occur with flat index guard held (here and later). this->flatIndexGuard.lock_shared(); @@ -721,7 +724,6 @@ class TieredSVSIndex : public VecSimTieredIndex { auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); this->flatIndexGuard.unlock_shared(); - auto svs_index = GetSVSIndex(); svs_index->addVector(storage_blob.get(), job->label); // Remove the vector and the insert job from the flat buffer. @@ -763,84 +765,79 @@ class TieredSVSIndex : public VecSimTieredIndex { } void updateSVSIndex(size_t availableThreads) { + std::vector ids_to_move; std::vector labels_to_move; std::vector vectors_to_move; - std::vector deleted_labels_during_update; { // lock frontendIndex from modifications - std::shared_lock flat_lock{this->flatIndexGuard}; + std::lock_guard flat_lock{this->flatIndexGuard}; + auto svs_index = GetSVSIndex(); + bool is_initialization = !svs_index->ready(); auto flat_index = this->GetFlatIndex(); const auto frontend_index_size = this->frontendIndex->indexSize(); + const auto init_batch_size = is_initialization ? ids_to_init_.size() : frontend_index_size; const size_t dim = flat_index->getDim(); - labels_to_move.reserve(frontend_index_size); - vectors_to_move.reserve(frontend_index_size * dim); + ids_to_move.reserve(init_batch_size); + labels_to_move.reserve(init_batch_size); + vectors_to_move.reserve(init_batch_size * dim); + + if (is_initialization) { + for (idType id : ids_to_init_) { + ids_to_move.push_back(id); + labels_to_move.push_back(flat_index->getVectorLabel(id)); + auto data = flat_index->getDataByInternalId(id); + vectors_to_move.insert(vectors_to_move.end(), data, data + dim); + } + ids_to_init_.clear(); + } else { + for (idType id = 0; id < frontend_index_size; id++) { + ids_to_move.push_back(id); + labels_to_move.push_back(flat_index->getVectorLabel(id)); + auto data = flat_index->getDataByInternalId(id); + vectors_to_move.insert(vectors_to_move.end(), data, data + dim); + } + } - for (idType i = 0; i < frontend_index_size; i++) { - labels_to_move.push_back(flat_index->getVectorLabel(i)); - auto data = flat_index->getDataByInternalId(i); - vectors_to_move.insert(vectors_to_move.end(), data, data + dim); + std::sort(ids_to_move.begin(), ids_to_move.end()); + int total_deleted = 0; + for (auto it = ids_to_move.rbegin(); it != ids_to_move.rend(); ++it) { + idType id = *it; + auto label = this->frontendIndex->getVectorLabel(id); + // Delete the vector from the frontend index if not in-place updated. + if (label != SKIP_LABEL) { + labelType last_vec_label = + this->frontendIndex->getVectorLabel(this->frontendIndex->indexSize() - 1); + int deleted = this->frontendIndex->deleteVectorById(label, id); + if (deleted && id != this->frontendIndex->indexSize()) { + // If the vector removal caused a swap with the last id, update the relevant insert job. + this->updateInsertJobInternalId(this->frontendIndex->indexSize(), id, + last_vec_label); + } + total_deleted += deleted; + } } - // reset journal to the current frontend index state - swaps_journal.clear(); - deleted_labels_journal.clear(); + + assert(total_deleted == std::count_if(labels_to_move.begin(), labels_to_move.end(), + [](labelType label) { return label != SKIP_LABEL; }) && + "Deleted vectors count does not match the number of labels to delete"); + } // release frontend index executeTracingCallback("UpdateJob::before_add_to_svs"); { // lock backend index for writing and add vectors there auto svs_index = GetSVSIndex(); + svs_index->setNumThreads(1); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); if (this->backendIndex->indexSize() == 0) { auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - svs_index->setImpl(std::move(impl)); } else { svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); } } - - executeTracingCallback("UpdateJob::after_add_to_svs"); - // clean-up frontend index - { // lock frontend index for writing and delete moved vectors - std::lock_guard lock(this->flatIndexGuard); - - // swap deleted labels journal with the local variable to track deleted labels during - // update - std::swap(deleted_labels_during_update, deleted_labels_journal); - - // Apply swaps from journal to labels_to_move to reflect changes made in meanwhile. - applySwapsToLabelsArray(labels_to_move, this->swaps_journal); - - // delete vectors from the frontend index in reverse order - // it increases the chance of avoiding swaps in the frontend index and performance - // improvement - int deleted = 0; - idType id = labels_to_move.size(); - while (id-- > 0) { - auto label = labels_to_move[id]; - // Delete the vector from the frontend index if not in-place updated. - if (label != SKIP_LABEL) { - deleted += this->frontendIndex->deleteVectorById(label, id); - } - } - assert(deleted == std::count_if(labels_to_move.begin(), labels_to_move.end(), - [](labelType label) { return label != SKIP_LABEL; }) && - "Deleted vectors count does not match the number of labels to delete"); - } - // delete vectors from backend index that were deleted from the frontend index during - // the update process. - { - std::sort(deleted_labels_during_update.begin(), deleted_labels_during_update.end()); - auto it = std::unique(deleted_labels_during_update.begin(), - deleted_labels_during_update.end()); - deleted_labels_during_update.erase(it, deleted_labels_during_update.end()); - auto svs_index = GetSVSIndex(); - svs_index->deleteVectors(deleted_labels_during_update.data(), - deleted_labels_during_update.size()); - } - this->backendReady.store(true, std::memory_order_release); } public: @@ -916,7 +913,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto storage_blob = this->frontendIndex->preprocessForStorage(blob); // prevent update job from running in parallel and lock any access to the backend // index - std::lock_guard lock(this->updateJobMutex); + std::lock_guard lock(this->updateJobMutex); // Set available thread count to 1 for single vector write-in-place operation. // This maintains the contract that single vector operations use exactly one thread. // TODO: Replace this setNumThreads(1) call with an assertion once we establish @@ -931,16 +928,11 @@ class TieredSVSIndex : public VecSimTieredIndex { if (!this->backendInitSubmited.load(std::memory_order_acquire)) { // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); - const auto ft_ret = this->frontendIndex->addVector(blob, label); - - if (ft_ret == 0) { // Vector was overriden - add 'skiping' swap to the journal. - assert(!this->backendIndex->isMultiValue() && - "addVector() may return 0 for single value indices only"); - for (auto id : this->frontendIndex->getElementIds(label)) { - this->swaps_journal.emplace_back(SKIP_LABEL, id, id); - } - deleted_labels_journal.push_back(label); + if (this->frontendIndex->isLabelExists(label)) { + deleteAndUpdateInitIds(label); } + ids_to_init_.insert(this->frontendIndex->indexSize()); + const auto ft_ret = this->frontendIndex->addVector(blob, label); ret = std::max(ret + ft_ret, 0); if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { @@ -950,13 +942,8 @@ class TieredSVSIndex : public VecSimTieredIndex { return ret; } - // spin-lock, while backend is initilizing - while (!this->backendReady.load(std::memory_order_acquire)) { - __builtin_ia32_pause(); - } - this->flatIndexGuard.lock_shared(); - if (this->frontendIndex->indexSize() >= flat_buffer_bound) { + if (svs_index->ready() && (this->frontendIndex->indexSize() >= flat_buffer_bound)) { this->flatIndexGuard.unlock_shared(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); @@ -966,7 +953,18 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.lock(); idType new_flat_id = this->frontendIndex->indexSize(); if (this->frontendIndex->isLabelExists(label) && !this->frontendIndex->isMultiValue()) { + if (this->labelToInsertJobs.count(label) == 0) { + // No pending insert job. + // Just replace vector in the frontend + // If this label already exists, this will do overwrite. + ret = this->frontendIndex->addVector(blob, label); + this->flatIndexGuard.unlock(); + return ret; + } + // Overwrite the vector and invalidate its only pending job (since we are not in MULTI). + // Label exists, but job doesn't. It means the label is used for initialization. + // Just create new job, this job will remove duplicate from the backend. auto *old_job = this->labelToInsertJobs.at(label).at(0); old_job->id = this->setAndSaveInvalidJob(old_job); this->labelToInsertJobs.erase(label); @@ -998,19 +996,26 @@ class TieredSVSIndex : public VecSimTieredIndex { } this->flatIndexGuard.unlock(); + // Here, a worker might ingest the previous vector that was stored under "label" + // (in case of override in non-MULTI index) - so if it's there, we remove it + // we submit the insert job. + if (!this->backendIndex->isMultiValue()) { + if (svs_index->ready()) { + // If we removed the previous vector from both svs and flat in the overwrite process, + // we still return 0 (not -1). + ret = std::max(ret - svs_index->deleteVector(label), 0); + } + } + // Insert job to the queue and signal the workers' updater. this->submitSingleJob(new_insert_job); return ret; } } - int deleteAndRecordSwaps_Unsafe(labelType label) { + void deleteAndUpdateInitIds(labelType label) { auto deleting_ids = this->frontendIndex->getElementIds(label); - // assert if all elements of deleting_ids are unique - assert(std::set(deleting_ids.begin(), deleting_ids.end()).size() == deleting_ids.size() && - "deleting_ids should contain unique ids"); - // Sort deleting_ids by id descending order std::sort(deleting_ids.begin(), deleting_ids.end(), [](const auto &a, const auto &b) { return a > b; }); @@ -1025,43 +1030,94 @@ class TieredSVSIndex : public VecSimTieredIndex { }) && "updated_ids should be a subset of deleting_ids"); - // Record swaps in the journal. - for (auto id : deleting_ids) { - auto it = updated_ids.find(id); - if (it != updated_ids.end()) { - assert(id == it->first && "id in updated_ids should match the id in deleting_ids"); - auto newId = id; - auto oldId = it->second.first; - auto oldLabel = it->second.second; - this->swaps_journal.emplace_back(oldLabel, oldId, newId); - } else { - // No swap, just delete is marked by oldId == newId == deleted id - this->swaps_journal.emplace_back(SKIP_LABEL, id, id); + std::vector new_ids_to_init; + for (auto &it : updated_ids) { + idType prev_id = it.second.first; + idType new_id = it.first; + + if (ids_to_init_.count(prev_id) > 0) { + ids_to_init_.erase(prev_id); + new_ids_to_init.push_back(new_id); + } + labelType updated_vec_label = it.second.second; + this->updateInsertJobInternalId(prev_id, new_id, updated_vec_label); + } + + for (idType new_id : new_ids_to_init) { + ids_to_init_.insert(new_id); + } + } + + int deleteAndRecordSwaps_Unsafe(labelType label) { + auto deleting_ids = this->frontendIndex->getElementIds(label); + if (deleting_ids.size() == 0) return 0; + + // assert if all elements of deleting_ids are unique + assert(std::set(deleting_ids.begin(), deleting_ids.end()).size() == deleting_ids.size() && + "deleting_ids should contain unique ids"); + + // If id is deleted, don't use it for initialization + for (idType id : deleting_ids) { + ids_to_init_.erase(id); + } + + if (this->labelToInsertJobs.count(label) > 0) { + // Invalidate the pending insert job(s) into SVS associated with this label + auto &insert_jobs = this->labelToInsertJobs.at(label); + for (auto *job : insert_jobs) { + job->id = this->setAndSaveInvalidJob(job); } + // Remove the pending insert job(s) from the labelToInsertJobs mapping. + this->labelToInsertJobs.erase(label); } - deleted_labels_journal.push_back(label); + deleteAndUpdateInitIds(label); return deleting_ids.size(); } int deleteVector(labelType label) override { int ret = 0; - // Backend index deletions to be synchronized with the frontend index, - // elsewhere there is the risk of labels duplication in both indices which can lead to wrong - // results of topK queries. In such case we should behave as if InPlace mode is always set. - bool label_exists = [&]() { - std::shared_lock lock(this->flatIndexGuard); - return this->frontendIndex->isLabelExists(label); - }(); - - if (label_exists) { - std::lock_guard lock(this->flatIndexGuard); - ret = this->deleteAndRecordSwaps_Unsafe(label); + + this->flatIndexGuard.lock_shared(); + if (this->frontendIndex->isLabelExists(label)) { + this->flatIndexGuard.unlock_shared(); + this->flatIndexGuard.lock(); + // Check again if the label exists, as it may have been removed while we released the lock. + if (this->frontendIndex->isLabelExists(label)) { + ret = this->deleteAndRecordSwaps_Unsafe(label); + // if (this->labelToInsertJobs.count(label) > 0) { + // // Invalidate the pending insert job(s) into SVS associated with this label + // auto &insert_jobs = this->labelToInsertJobs.at(label); + // for (auto *job : insert_jobs) { + // job->id = this->setAndSaveInvalidJob(job); + // } + // ret += insert_jobs.size(); + // // Remove the pending insert job(s) from the labelToInsertJobs mapping. + // this->labelToInsertJobs.erase(label); + // // Go over the every id that corresponds the label and remove it from the flat buffer. + // // Every delete may cause a swap of the deleted id with the last id, and we return a + // // mapping from id to the original id that resides in this id after the deletion(s) (see + // // an example in this function implementation in MULTI index). + // auto updated_ids = this->frontendIndex->deleteVectorAndGetUpdatedIds(label); + // for (auto &it : updated_ids) { + // idType prev_id = it.second.first; + // labelType updated_vec_label = it.second.second; + // this->updateInsertJobInternalId(prev_id, it.first, updated_vec_label); + // } + // } else { + // // label was deleted during initilization stage + // ret = this->deleteAndRecordSwaps_Unsafe(label); + // } + } + this->flatIndexGuard.unlock(); + } else { + this->flatIndexGuard.unlock_shared(); } ret += this->backendIndex->deleteVector(label); return ret; } + size_t getNumMarkedDeleted() const override { return this->GetSVSIndex()->getNumMarkedDeleted(); } @@ -1110,7 +1166,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // the entire training duration (which can take 40-85s on slow machines). // If the mutex is held, training is actively running, so we report // indexUpdateScheduled = true (BACKGROUND_INDEXING = 1). - std::unique_lock lock(this->updateJobMutex, std::try_to_lock); + std::unique_lock lock(this->updateJobMutex, std::try_to_lock); if (lock.owns_lock()) { svsTieredInfo.indexUpdateScheduled = this->indexUpdateScheduled.test() == VecSimBool_TRUE; diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index 3d9c0bd0d..63f9c6a23 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -613,6 +613,12 @@ TYPED_TEST(SVSTieredIndexTest, background_indexing_check) { std::this_thread::sleep_for(std::chrono::milliseconds(100)); } + // The backgroundIndexing flag tracks the batch update job only; individual async + // SVS_INSERT_VECTOR_JOBs may still be in flight, each transiently holding its vector in + // both the flat and backend indices. Drain the queue so the total size is stable before + // asserting the exact count. + mock_thread_pool.thread_pool_join(); + ASSERT_GT(tiered_index->GetBackendIndex()->indexSize(), training_th + second_batch / update_th); ASSERT_LT(tiered_index->GetFlatIndex()->indexSize(), update_th); ASSERT_EQ(tiered_index->indexSize(), second_batch + training_th); @@ -1125,7 +1131,7 @@ TYPED_TEST(SVSTieredIndexTestBasic, markedDeleted) { ASSERT_EQ(tiered_index->getNumMarkedDeleted(), 0); // Move vectors to the backend - mock_thread_pool.thread_iteration(); + while (mock_thread_pool.jobQ.size() > 0) mock_thread_pool.thread_iteration(); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), n); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); ASSERT_EQ(tiered_index->indexSize(), n); @@ -1222,13 +1228,20 @@ TYPED_TEST(SVSTieredIndexTestBasic, deleteVectorMulti) { ASSERT_EQ(tiered_index->indexSize(), 2); ASSERT_EQ(tiered_index->deleteVector(vec_label), 2); ASSERT_EQ(tiered_index->indexLabelCount(), 0); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 2); + mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 1); mock_thread_pool.thread_iteration(); ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); // Test deleting a label for which both of its vector's is in SVS index. GenerateAndAddVector(tiered_index, dim, vec_label, vec_label); GenerateAndAddVector(tiered_index, dim, vec_label, other_vec_val); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 2); mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 1); + mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); ASSERT_EQ(tiered_index->indexLabelCount(), 1); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 2); From b130e31c684c9647107ddc75fdd24028a34ee019 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Wed, 8 Jul 2026 06:47:45 -0700 Subject: [PATCH 10/18] refactoring --- src/VecSim/algorithms/svs/svs.h | 4 +- src/VecSim/algorithms/svs/svs_tiered.h | 162 ++++++++++--------------- tests/unit/test_svs_fp16.cpp | 2 +- tests/unit/test_svs_tiered.cpp | 4 +- 4 files changed, 67 insertions(+), 105 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 7dcdc1d93..ee71b7470 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -340,7 +340,9 @@ class SVSIndex : public VecSimIndexAbstract, fl deleted_num = impl_->delete_entries(std::span{labels, n}); } - this->markIndexUpdate(deleted_num); + if (deleted_num > 0) + this->markIndexUpdate(deleted_num); + return deleted_num; } diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 8869b2191..8608de84c 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -232,7 +232,7 @@ class TieredSVSIndex : public VecSimTieredIndex { using swap_record = std::tuple; constexpr static size_t SKIP_LABEL = std::numeric_limits::max(); std::vector swaps_journal; - // deleted_labels_journal is used by updateSVSIndex() to track vectors that were deleted from + // deleted_labels_journal is used by initSVSIndex() to track vectors that were deleted from // Flat index during SVS index updating. The journal contains the deleted labels. These labels // are used to delete the same vectors from the SVS index at the end of the update. std::vector deleted_labels_journal; @@ -553,19 +553,19 @@ class TieredSVSIndex : public VecSimTieredIndex { private: /** - * @brief Updates the SVS index in a thread-safe manner. + * @brief Init the SVS index in a thread-safe manner. * * This static wrapper function performs the following actions: * - Acquires a lock on the index's updateJobMutex to prevent concurrent updates. * - Clears the indexUpdateScheduled flag to allow future scheduling. * - Configures the number of threads for the underlying SVS index update operation. - * - Calls the updateSVSIndex method to perform the actual index update. + * - Calls the initSVSIndex method to perform the actual index update. * * @param idx Pointer to the VecSimIndex to be updated. * @param availableThreads The number of threads available for the update operation. Current * thread us used as well, so the minimal value is 1. */ - static void updateSVSIndexWrapper(VecSimIndex *idx, size_t availableThreads) { + static void initSVSIndexWrapper(VecSimIndex *idx, size_t availableThreads) { assert(availableThreads > 0); auto index = static_cast *>(idx); assert(index); @@ -574,7 +574,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // Release the scheduled flag to allow scheduling again index->indexUpdateScheduled.clear(); // Update the SVS index - index->updateSVSIndex(availableThreads); + index->initSVSIndex(availableThreads); } static void executeInsertJobWrapper(AsyncJob *job) { @@ -640,7 +640,7 @@ class TieredSVSIndex : public VecSimTieredIndex { } } - void scheduleSVSIndexUpdate() { + void scheduleSVSIndexInit() { // do not schedule if scheduled already if (indexUpdateScheduled.test_and_set()) { return; @@ -648,7 +648,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto total_threads = this->GetSVSIndex()->getThreadPoolCapacity(); auto jobs = SVSMultiThreadJob::createJobs( - this->allocator, SVS_BATCH_UPDATE_JOB, updateSVSIndexWrapper, this, total_threads, + this->allocator, SVS_BATCH_UPDATE_JOB, initSVSIndexWrapper, this, total_threads, std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); this->submitJobs(jobs); } @@ -764,39 +764,38 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.unlock(); } - void updateSVSIndex(size_t availableThreads) { + void initSVSIndex(size_t availableThreads) { std::vector ids_to_move; std::vector labels_to_move; std::vector vectors_to_move; { // lock frontendIndex from modifications + // The whole initialization is done under flatIndexGuard std::lock_guard flat_lock{this->flatIndexGuard}; - auto svs_index = GetSVSIndex(); - bool is_initialization = !svs_index->ready(); auto flat_index = this->GetFlatIndex(); - const auto frontend_index_size = this->frontendIndex->indexSize(); - const auto init_batch_size = is_initialization ? ids_to_init_.size() : frontend_index_size; + const auto init_batch_size = ids_to_init_.size(); const size_t dim = flat_index->getDim(); ids_to_move.reserve(init_batch_size); labels_to_move.reserve(init_batch_size); vectors_to_move.reserve(init_batch_size * dim); - if (is_initialization) { - for (idType id : ids_to_init_) { - ids_to_move.push_back(id); - labels_to_move.push_back(flat_index->getVectorLabel(id)); - auto data = flat_index->getDataByInternalId(id); - vectors_to_move.insert(vectors_to_move.end(), data, data + dim); - } - ids_to_init_.clear(); - } else { - for (idType id = 0; id < frontend_index_size; id++) { - ids_to_move.push_back(id); - labels_to_move.push_back(flat_index->getVectorLabel(id)); - auto data = flat_index->getDataByInternalId(id); - vectors_to_move.insert(vectors_to_move.end(), data, data + dim); - } + for (idType id : ids_to_init_) { + ids_to_move.push_back(id); + labels_to_move.push_back(flat_index->getVectorLabel(id)); + auto data = flat_index->getDataByInternalId(id); + vectors_to_move.insert(vectors_to_move.end(), data, data + dim); + } + ids_to_init_.clear(); + + executeTracingCallback("UpdateJob::before_add_to_svs"); + { + auto svs_index = GetSVSIndex(); + svs_index->setNumThreads(1); + assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); + auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), + labels_to_move.size()); + svs_index->setImpl(std::move(impl)); } std::sort(ids_to_move.begin(), ids_to_move.end()); @@ -821,23 +820,7 @@ class TieredSVSIndex : public VecSimTieredIndex { assert(total_deleted == std::count_if(labels_to_move.begin(), labels_to_move.end(), [](labelType label) { return label != SKIP_LABEL; }) && "Deleted vectors count does not match the number of labels to delete"); - } // release frontend index - - executeTracingCallback("UpdateJob::before_add_to_svs"); - { // lock backend index for writing and add vectors there - auto svs_index = GetSVSIndex(); - svs_index->setNumThreads(1); - assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); - if (this->backendIndex->indexSize() == 0) { - auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), - labels_to_move.size()); - svs_index->setImpl(std::move(impl)); - } else { - svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(), - labels_to_move.size()); - } - } } public: @@ -887,25 +870,29 @@ class TieredSVSIndex : public VecSimTieredIndex { // In-Place mode - add vector syncronously to the backend index. if (this->getWriteMode() == VecSim_WriteInPlace) { // It is ok to lock everything at once for in-place mode, - // but we will have to unlock averything before calling updateSVSIndexWrapper() + // but we will have to unlock averything before calling initSVSIndexWrapper() // so make the minimal needed lock here. // Backend index initialization data have to be buffered for proper // compression/training. if (this->backendIndex->indexSize() == 0) { // If backend index size is 0, first collect vectors in frontend index // lock in scope to ensure that these will be released before - // updateSVSIndexWrapper() is called. + // initSVSIndexWrapper() is called. { std::lock_guard lock(this->flatIndexGuard); + if (this->frontendIndex->isLabelExists(label)) { + deleteAndUpdateInitIds(label); + } + ids_to_init_.insert(this->frontendIndex->indexSize()); ret = this->frontendIndex->addVector(blob, label); // If frontend size exceeds the update job threshold, ... frontend_index_size = this->frontendIndex->indexSize(); } // ... move vectors to the backend index. if (frontend_index_size >= this->trainingTriggerThreshold) { - // updateSVSIndexWrapper() accures it's own locks + // initSVSIndexWrapper() accures it's own locks // initialize the SVS index synchonously using current thread only - updateSVSIndexWrapper(this, 1); + initSVSIndexWrapper(this, 1); } return ret; } else { @@ -937,7 +924,7 @@ class TieredSVSIndex : public VecSimTieredIndex { if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { this->backendInitSubmited.store(true, std::memory_order_release); - scheduleSVSIndexUpdate(); + scheduleSVSIndexInit(); } return ret; } @@ -1048,33 +1035,6 @@ class TieredSVSIndex : public VecSimTieredIndex { } } - int deleteAndRecordSwaps_Unsafe(labelType label) { - auto deleting_ids = this->frontendIndex->getElementIds(label); - if (deleting_ids.size() == 0) return 0; - - // assert if all elements of deleting_ids are unique - assert(std::set(deleting_ids.begin(), deleting_ids.end()).size() == deleting_ids.size() && - "deleting_ids should contain unique ids"); - - // If id is deleted, don't use it for initialization - for (idType id : deleting_ids) { - ids_to_init_.erase(id); - } - - if (this->labelToInsertJobs.count(label) > 0) { - // Invalidate the pending insert job(s) into SVS associated with this label - auto &insert_jobs = this->labelToInsertJobs.at(label); - for (auto *job : insert_jobs) { - job->id = this->setAndSaveInvalidJob(job); - } - // Remove the pending insert job(s) from the labelToInsertJobs mapping. - this->labelToInsertJobs.erase(label); - } - - deleteAndUpdateInitIds(label); - return deleting_ids.size(); - } - int deleteVector(labelType label) override { int ret = 0; @@ -1084,30 +1044,30 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.lock(); // Check again if the label exists, as it may have been removed while we released the lock. if (this->frontendIndex->isLabelExists(label)) { - ret = this->deleteAndRecordSwaps_Unsafe(label); - // if (this->labelToInsertJobs.count(label) > 0) { - // // Invalidate the pending insert job(s) into SVS associated with this label - // auto &insert_jobs = this->labelToInsertJobs.at(label); - // for (auto *job : insert_jobs) { - // job->id = this->setAndSaveInvalidJob(job); - // } - // ret += insert_jobs.size(); - // // Remove the pending insert job(s) from the labelToInsertJobs mapping. - // this->labelToInsertJobs.erase(label); - // // Go over the every id that corresponds the label and remove it from the flat buffer. - // // Every delete may cause a swap of the deleted id with the last id, and we return a - // // mapping from id to the original id that resides in this id after the deletion(s) (see - // // an example in this function implementation in MULTI index). - // auto updated_ids = this->frontendIndex->deleteVectorAndGetUpdatedIds(label); - // for (auto &it : updated_ids) { - // idType prev_id = it.second.first; - // labelType updated_vec_label = it.second.second; - // this->updateInsertJobInternalId(prev_id, it.first, updated_vec_label); - // } - // } else { - // // label was deleted during initilization stage - // ret = this->deleteAndRecordSwaps_Unsafe(label); - // } + auto deleting_ids = this->frontendIndex->getElementIds(label); + if (deleting_ids.size() == 0) return 0; + + // assert if all elements of deleting_ids are unique + assert(std::set(deleting_ids.begin(), deleting_ids.end()).size() == deleting_ids.size() && + "deleting_ids should contain unique ids"); + + // If id is deleted, don't use it for initialization + for (idType id : deleting_ids) { + ids_to_init_.erase(id); + } + + if (this->labelToInsertJobs.count(label) > 0) { + // Invalidate the pending insert job(s) into SVS associated with this label + auto &insert_jobs = this->labelToInsertJobs.at(label); + for (auto *job : insert_jobs) { + job->id = this->setAndSaveInvalidJob(job); + } + // Remove the pending insert job(s) from the labelToInsertJobs mapping. + this->labelToInsertJobs.erase(label); + } + + deleteAndUpdateInitIds(label); + return deleting_ids.size(); } this->flatIndexGuard.unlock(); } else { @@ -1162,7 +1122,7 @@ class TieredSVSIndex : public VecSimTieredIndex { }; { // Use try_lock to avoid blocking the main thread during long-running - // training operations. updateSVSIndexWrapper holds updateJobMutex for + // training operations. initSVSIndexWrapper holds updateJobMutex for // the entire training duration (which can take 40-85s on slow machines). // If the mutex is held, training is actively running, so we report // indexUpdateScheduled = true (BACKGROUND_INDEXING = 1). @@ -1171,7 +1131,7 @@ class TieredSVSIndex : public VecSimTieredIndex { svsTieredInfo.indexUpdateScheduled = this->indexUpdateScheduled.test() == VecSimBool_TRUE; } else { - // Mutex is held by updateSVSIndexWrapper — training is in progress. + // Mutex is held by initSVSIndexWrapper — training is in progress. svsTieredInfo.indexUpdateScheduled = true; } } diff --git a/tests/unit/test_svs_fp16.cpp b/tests/unit/test_svs_fp16.cpp index 59c11c0a1..ded23bd72 100644 --- a/tests/unit/test_svs_fp16.cpp +++ b/tests/unit/test_svs_fp16.cpp @@ -2337,7 +2337,7 @@ class FP16SVSTieredIndexTest : public FP16SVSTest { } } // Submit the index update job. - tiered_index->scheduleSVSIndexUpdate(); + tiered_index->scheduleSVSIndexInit(); ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size); // Execute the job from the queue and validate that the index was updated properly. diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index 63f9c6a23..2980d0e38 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -489,7 +489,7 @@ TYPED_TEST(SVSTieredIndexTest, CreateIndexInstance) { ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 0); // Submit the index update job. - tiered_index->scheduleSVSIndexUpdate(); + tiered_index->scheduleSVSIndexInit(); ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size); // Execute the job from the queue and validate that the index was updated properly. @@ -1635,7 +1635,7 @@ TYPED_TEST(SVSTieredIndexTest, parallelInsertAdHoc) { tiered_index->submitSingleJob(search_job); } - tiered_index->scheduleSVSIndexUpdate(); + tiered_index->scheduleSVSIndexInit(); mock_thread_pool.thread_pool_join(); EXPECT_EQ(successful_searches, n); From 78b4ef0286899cecd13f248985152031fcdf0de9 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Wed, 15 Jul 2026 09:27:43 -0700 Subject: [PATCH 11/18] fixes for GC --- src/VecSim/algorithms/svs/svs_tiered.h | 46 ++++++++++++++++++-------- src/VecSim/vec_sim_common.h | 1 + 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 8608de84c..2e797e188 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -25,6 +25,15 @@ struct SVSInsertJob : public AsyncJob { : AsyncJob(allocator, SVS_INSERT_VECTOR_JOB, insertCb, index_), label(label_), id(id_) {} }; +/** + * Definition of a job that launches GC on SVS Index. + */ +struct SVSGCJob : public AsyncJob { + SVSGCJob(std::shared_ptr allocator, + JobCallback insertCb, VecSimIndex *index_) + : AsyncJob(allocator, SVS_GC2_JOB, insertCb, index_) {} +}; + /** * @class SVSMultiThreadJob @@ -601,13 +610,17 @@ class TieredSVSIndex : public VecSimTieredIndex { * @note no need to implement extra non-static method, as GC logic is simple enough to be done * here. */ - static void SVSIndexGCWrapper(VecSimIndex *idx, size_t availableThreads) { - assert(availableThreads > 0); - auto index = static_cast *>(idx); - assert(index); + // static void SVSIndexGCWrapper(VecSimIndex *idx) { + static void SVSIndexGCWrapper(AsyncJob *job) { + // assert(availableThreads > 0); + // auto index = static_cast *>(idx); + // assert(index); - // Release the scheduled flag to allow scheduling again - index->indexGCScheduled.clear(); + auto gc_job = static_cast(job); + auto index = static_cast *>(gc_job->index); + + // // Release the scheduled flag to allow scheduling again + // index->indexGCScheduled.clear(); // Do SVS index GC index->backendIndex->log(VecSimCommonStrings::LOG_VERBOSE_STRING, @@ -617,9 +630,13 @@ class TieredSVSIndex : public VecSimTieredIndex { // No need to run GC on an empty index. return; } - svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); + // svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); + svs_index->setNumThreads(1); // VecSimIndexAbstract::runGC() is protected static_cast(index->backendIndex)->runGC(); + + // Release the scheduled flag to allow scheduling again + index->indexGCScheduled.clear(); } #ifdef BUILD_TESTS @@ -659,11 +676,11 @@ class TieredSVSIndex : public VecSimTieredIndex { return; } - auto total_threads = this->GetSVSIndex()->getThreadPoolCapacity(); - auto jobs = SVSMultiThreadJob::createJobs( - this->allocator, SVS_GC_JOB, SVSIndexGCWrapper, this, total_threads, - std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); - this->submitJobs(jobs); + AsyncJob *new_GC_job = new (this->allocator) + SVSGCJob(this->allocator, SVSIndexGCWrapper, this); + + // Insert job to the queue. + this->submitSingleJob(new_GC_job); } private: @@ -724,6 +741,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); this->flatIndexGuard.unlock_shared(); + svs_index->setNumThreads(1); svs_index->addVector(storage_blob.get(), job->label); // Remove the vector and the insert job from the flat buffer. @@ -933,6 +951,7 @@ class TieredSVSIndex : public VecSimTieredIndex { if (svs_index->ready() && (this->frontendIndex->indexSize() >= flat_buffer_bound)) { this->flatIndexGuard.unlock_shared(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); + svs_index->setNumThreads(1); return ret = svs_index->addVector(storage_blob.get(), label); } else { @@ -1041,7 +1060,7 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.lock_shared(); if (this->frontendIndex->isLabelExists(label)) { this->flatIndexGuard.unlock_shared(); - this->flatIndexGuard.lock(); + std::lock_guard flat_lock{this->flatIndexGuard}; // Check again if the label exists, as it may have been removed while we released the lock. if (this->frontendIndex->isLabelExists(label)) { auto deleting_ids = this->frontendIndex->getElementIds(label); @@ -1069,7 +1088,6 @@ class TieredSVSIndex : public VecSimTieredIndex { deleteAndUpdateInitIds(label); return deleting_ids.size(); } - this->flatIndexGuard.unlock(); } else { this->flatIndexGuard.unlock_shared(); } diff --git a/src/VecSim/vec_sim_common.h b/src/VecSim/vec_sim_common.h index 03e7870f3..d800e7cdc 100644 --- a/src/VecSim/vec_sim_common.h +++ b/src/VecSim/vec_sim_common.h @@ -275,6 +275,7 @@ typedef enum { SVS_BATCH_UPDATE_JOB, SVS_INSERT_VECTOR_JOB, SVS_GC_JOB, + SVS_GC2_JOB, INVALID_JOB // to indicate that finding a JobType >= INVALID_JOB is an error } JobType; From fedd22ea70579aa50f9d75b88cce536066f5ea55 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Mon, 20 Jul 2026 09:35:50 -0700 Subject: [PATCH 12/18] copy blob before flat unlock --- src/VecSim/algorithms/svs/svs_tiered.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 2e797e188..4ebca3e09 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -738,11 +738,16 @@ class TieredSVSIndex : public VecSimTieredIndex { this->invalidJobsLookupGuard.unlock(); return; } - auto storage_blob = this->frontendIndex->preprocessForStorage(this->frontendIndex->getDataByInternalId(job->id)); + + // Copy the vector blob out of the flat buffer while holding flatIndexGuard, so we + // can release the flat lock before indexing into the SVS backend + size_t data_size = this->frontendIndex->getStoredDataSize(); + auto blob_copy = this->getAllocator()->allocate_unique(data_size); + memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), data_size); this->flatIndexGuard.unlock_shared(); svs_index->setNumThreads(1); - svs_index->addVector(storage_blob.get(), job->label); + svs_index->addVector(blob_copy.get(), job->label); // Remove the vector and the insert job from the flat buffer. this->flatIndexGuard.lock(); @@ -812,7 +817,7 @@ class TieredSVSIndex : public VecSimTieredIndex { svs_index->setNumThreads(1); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), - labels_to_move.size()); + labels_to_move.size()); svs_index->setImpl(std::move(impl)); } @@ -1315,3 +1320,4 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.unlock_shared(); } }; + \ No newline at end of file From fcf83dd767e08a8b7747ad16d46736576a9a6f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihai=20Capot=C4=83?= Date: Thu, 23 Jul 2026 22:09:56 -0700 Subject: [PATCH 13/18] Remove null characters from end of file --- src/VecSim/algorithms/svs/svs_tiered.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 4ebca3e09..e3eb441b8 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -1320,4 +1320,3 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.unlock_shared(); } }; - \ No newline at end of file From 4035699ee202da005643963d268d94573e8e9f19 Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Tue, 28 Jul 2026 02:28:19 -0700 Subject: [PATCH 14/18] fixes for gc --- src/VecSim/algorithms/svs/svs.h | 60 ++++++++++--- src/VecSim/algorithms/svs/svs_tiered.h | 109 +++++++++++++++++------ src/VecSim/vec_sim_common.h | 3 +- tests/unit/test_svs_fp16.cpp | 8 ++ tests/unit/test_svs_tiered.cpp | 116 ++++++++++++++----------- 5 files changed, 204 insertions(+), 92 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index ee71b7470..e410529f7 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -41,6 +41,7 @@ struct SVSIndexBase virtual int addVectors(const void *vectors_data, const labelType *labels, size_t n) = 0; virtual int deleteVector(labelType label) = 0; virtual int deleteVectors(const labelType *labels, size_t n) = 0; + virtual void consolidate(const std::vector& labels) = 0; virtual bool isLabelExists(labelType label) const = 0; virtual size_t indexStorageSize() const = 0; virtual size_t getNumThreads() const = 0; @@ -299,21 +300,36 @@ class SVSIndex : public VecSimIndexAbstract, fl } } + this->pimplGuard_.lock_shared(); if (!ready()) { + this->pimplGuard_.unlock_shared(); // SVS index instance cannot be empty, so we have to construct it at first rows std::lock_guard lock(this->pimplGuard_); - impl_ = initImpl(points, ids); - assert(this->impl_ != nullptr); - setReady(); + if (!ready()) { + impl_ = initImpl(points, ids); + assert(this->impl_ != nullptr); + setReady(); + } else { + impl_->add_points(points, ids); + } } else { // Add new points to existing SVS index - std::shared_lock lock(this->pimplGuard_); impl_->add_points(points, ids); + this->pimplGuard_.unlock_shared(); } return n - deleted_num; } + void consolidate(const std::vector& labels) { + std::shared_lock lock(this->pimplGuard_); + if (!ready()) + return; + + size_t n_consolidated = impl_->consolidate(labels); + num_marked_deleted.fetch_sub(n_consolidated, std::memory_order_relaxed); + } + int deleteVectorImpl(const labelType label) { if (indexLabelCount() == 0) { return 0; @@ -354,12 +370,14 @@ class SVSIndex : public VecSimIndexAbstract, fl // SVS index instance should not be empty if (indexLabelCount() == 0) { std::lock_guard lock(this->pimplGuard_); - { - setUnready(); - this->impl_.reset(); + if (indexLabelCountUnsafe() == 0) { + { + setUnready(); + this->impl_.reset(); + } + num_marked_deleted.store(0, std::memory_order_relaxed); + return; } - num_marked_deleted.store(0, std::memory_order_relaxed); - return; } num_marked_deleted.fetch_add(n, std::memory_order_relaxed); @@ -404,8 +422,8 @@ class SVSIndex : public VecSimIndexAbstract, fl size_t indexSize() const override { return indexStorageSize(); } size_t indexStorageSize() const override { + std::shared_lock lock(this->pimplGuard_); if (ready()) { - std::shared_lock lock(this->pimplGuard_); return impl_->view_data().size(); } else { return 0; @@ -413,14 +431,30 @@ class SVSIndex : public VecSimIndexAbstract, fl } size_t indexCapacity() const override { + std::shared_lock lock(this->pimplGuard_); if (ready()) { - std::shared_lock lock(this->pimplGuard_); return storage_traits_t::storage_capacity(impl_->view_data()); } else { return 0; } } + size_t indexLabelCountUnsafe() const { + if constexpr (isMulti) { + if (ready()) { + return impl_->labelcount(); + } else { + return 0; + } + } else { + if (ready()) { + return impl_->size(); + } else { + return 0; + } + } + } + size_t indexLabelCount() const override { if constexpr (isMulti) { if (ready()) { @@ -641,7 +675,6 @@ class SVSIndex : public VecSimIndexAbstract, fl return rep; } { - std::shared_lock lock(this->pimplGuard_); // limit result size to index size k = std::min(k, this->indexLabelCount()); @@ -651,6 +684,7 @@ class SVSIndex : public VecSimIndexAbstract, fl auto query = svs::data::ConstSimpleDataView{ static_cast(processed_query), 1, this->dim}; auto result = svs::QueryResult{query.size(), k}; + std::shared_lock lock(this->pimplGuard_); auto sp = svs_details::joinSearchParams(impl_->get_search_parameters(), queryParams, is_two_level_lvq); @@ -803,7 +837,7 @@ class SVSIndex : public VecSimIndexAbstract, fl std::shared_lock lock(this->pimplGuard_); // There is documentation for consolidate(): // https://intel.github.io/ScalableVectorSearch/python/dynamic.html#svs.DynamicVamana.consolidate - impl_->consolidate(); + // impl_->consolidate(); // There is documentation for compact(): // https://intel.github.io/ScalableVectorSearch/python/dynamic.html#svs.DynamicVamana.compact impl_->compact(); diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 4ebca3e09..c94d3b711 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -25,13 +25,24 @@ struct SVSInsertJob : public AsyncJob { : AsyncJob(allocator, SVS_INSERT_VECTOR_JOB, insertCb, index_), label(label_), id(id_) {} }; +/** + * Definition of a job that launches partial consolidation on SVS Index. + */ +struct SVSConsolidateJob : public AsyncJob { + std::vector labels; + + SVSConsolidateJob(std::shared_ptr allocator, const std::vector& labels_, + JobCallback insertCb, VecSimIndex *index_) + : AsyncJob(allocator, SVS_CONSOLIDATE_JOB, insertCb, index_), labels(labels_) {} +}; + /** * Definition of a job that launches GC on SVS Index. */ struct SVSGCJob : public AsyncJob { SVSGCJob(std::shared_ptr allocator, JobCallback insertCb, VecSimIndex *index_) - : AsyncJob(allocator, SVS_GC2_JOB, insertCb, index_) {} + : AsyncJob(allocator, SVS_GC_JOB, insertCb, index_) {} }; @@ -248,6 +259,7 @@ class TieredSVSIndex : public VecSimTieredIndex { size_t trainingTriggerThreshold; size_t updateTriggerThreshold; + size_t consolidateTriggerThreshold; size_t updateJobWaitTime; // Used to prevent scheduling multiple index update jobs at the same time. // As far as the update job does a batch update, job queue should have just 1 job at the moment. @@ -263,6 +275,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::atomic backendInitSubmited{false}; std::unordered_set ids_to_init_; + std::vector labels_to_consolidate_; vecsim_stl::unordered_map> labelToInsertJobs; // A mapping to hold invalid jobs, so we can dispose them upon index deletion. @@ -610,23 +623,18 @@ class TieredSVSIndex : public VecSimTieredIndex { * @note no need to implement extra non-static method, as GC logic is simple enough to be done * here. */ - // static void SVSIndexGCWrapper(VecSimIndex *idx) { static void SVSIndexGCWrapper(AsyncJob *job) { - // assert(availableThreads > 0); - // auto index = static_cast *>(idx); - // assert(index); - auto gc_job = static_cast(job); auto index = static_cast *>(gc_job->index); - // // Release the scheduled flag to allow scheduling again - // index->indexGCScheduled.clear(); - + std::shared_lock lock(index->updateJobMutex); // Do SVS index GC index->backendIndex->log(VecSimCommonStrings::LOG_VERBOSE_STRING, "running asynchronous GC for tiered SVS index"); auto svs_index = index->GetSVSIndex(); if (index->backendIndex->indexSize() == 0) { + index->indexGCScheduled.clear(); + delete job; // No need to run GC on an empty index. return; } @@ -637,6 +645,18 @@ class TieredSVSIndex : public VecSimTieredIndex { // Release the scheduled flag to allow scheduling again index->indexGCScheduled.clear(); + delete job; + } + + static void SVSIndexConsolidateWrapper(AsyncJob *job) { + auto consolidate_job = static_cast(job); + auto index = static_cast *>(consolidate_job->index); + + std::shared_lock lock(index->updateJobMutex); + auto svs_index = index->GetSVSIndex(); + svs_index->setNumThreads(1); + svs_index->consolidate(consolidate_job->labels); + delete job; } #ifdef BUILD_TESTS @@ -683,6 +703,19 @@ class TieredSVSIndex : public VecSimTieredIndex { this->submitSingleJob(new_GC_job); } + void scheduleSVSIndexConsolidate(labelType label) { + labels_to_consolidate_.push_back(label); + + if (labels_to_consolidate_.size() >= consolidateTriggerThreshold) { + AsyncJob *new_consolidate_job = new (this->allocator) + SVSConsolidateJob(this->allocator, labels_to_consolidate_, SVSIndexConsolidateWrapper, this); + + // Insert job to the queue. + this->submitSingleJob(new_consolidate_job); + labels_to_consolidate_.clear(); + } + } + private: static void applySwapsToLabelsArray(std::vector &labels, const std::vector &swaps) { @@ -746,7 +779,7 @@ class TieredSVSIndex : public VecSimTieredIndex { memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), data_size); this->flatIndexGuard.unlock_shared(); - svs_index->setNumThreads(1); + // svs_index->setNumThreads(1); svs_index->addVector(blob_copy.get(), job->label); // Remove the vector and the insert job from the flat buffer. @@ -792,6 +825,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::vector labels_to_move; std::vector vectors_to_move; + executeTracingCallback("UpdateJob::before_add_to_svs"); { // lock frontendIndex from modifications // The whole initialization is done under flatIndexGuard std::lock_guard flat_lock{this->flatIndexGuard}; @@ -811,13 +845,13 @@ class TieredSVSIndex : public VecSimTieredIndex { } ids_to_init_.clear(); - executeTracingCallback("UpdateJob::before_add_to_svs"); { auto svs_index = GetSVSIndex(); - svs_index->setNumThreads(1); + svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); + svs_index->setNumThreads(1); svs_index->setImpl(std::move(impl)); } @@ -844,6 +878,7 @@ class TieredSVSIndex : public VecSimTieredIndex { [](labelType label) { return label != SKIP_LABEL; }) && "Deleted vectors count does not match the number of labels to delete"); } // release frontend index + executeTracingCallback("UpdateJob::after_add_to_svs"); } public: @@ -872,6 +907,8 @@ class TieredSVSIndex : public VecSimTieredIndex { ? SVS_VAMANA_DEFAULT_TRAINING_THRESHOLD : this->updateTriggerThreshold; + this->consolidateTriggerThreshold = SVS_VAMANA_DEFAULT_CONSOLIDATE_THRESHOLD; + this->trainingTriggerThreshold = tiered_svs_params.trainingTriggerThreshold == 0 ? default_training_threshold @@ -897,7 +934,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // so make the minimal needed lock here. // Backend index initialization data have to be buffered for proper // compression/training. - if (this->backendIndex->indexSize() == 0) { + if ((!svs_index->ready()) && (!this->backendInitSubmited.load(std::memory_order_acquire))) { // If backend index size is 0, first collect vectors in frontend index // lock in scope to ensure that these will be released before // initSVSIndexWrapper() is called. @@ -929,16 +966,22 @@ class TieredSVSIndex : public VecSimTieredIndex { // TODO: Replace this setNumThreads(1) call with an assertion once we establish // a contract that write-in-place mode guarantees numThreads == 1. svs_index->setNumThreads(1); - return this->backendIndex->addVector(storage_blob.get(), label); + int deleted = 0; + if (!this->backendIndex->isMultiValue()) { + deleted = svs_index->deleteVector(label); + if (deleted > 0) + svs_index->consolidate({label}); + } + return this->backendIndex->addVector(storage_blob.get(), label) - deleted; } } assert(this->getWriteMode() != VecSim_WriteInPlace && "InPlace mode returns early"); // Async mode - add vector to the frontend index and schedule an update job if needed. - if (!this->backendInitSubmited.load(std::memory_order_acquire)) { + if ((!svs_index->ready()) && (!this->backendInitSubmited.load(std::memory_order_acquire))) { // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); - if (this->frontendIndex->isLabelExists(label)) { + if (!this->frontendIndex->isMultiValue() && this->frontendIndex->isLabelExists(label)) { deleteAndUpdateInitIds(label); } ids_to_init_.insert(this->frontendIndex->indexSize()); @@ -956,7 +999,12 @@ class TieredSVSIndex : public VecSimTieredIndex { if (svs_index->ready() && (this->frontendIndex->indexSize() >= flat_buffer_bound)) { this->flatIndexGuard.unlock_shared(); auto storage_blob = this->frontendIndex->preprocessForStorage(blob); - svs_index->setNumThreads(1); + + if (!this->backendIndex->isMultiValue()) { + auto deleted = svs_index->deleteVector(label); + if (deleted > 0) + scheduleSVSIndexConsolidate(label); + } return ret = svs_index->addVector(storage_blob.get(), label); } else { @@ -989,7 +1037,7 @@ class TieredSVSIndex : public VecSimTieredIndex { // increase index capacity. } // If this label already exists, this will do overwrite. - this->frontendIndex->addVector(blob, label); + ret += this->frontendIndex->addVector(blob, label); AsyncJob *new_insert_job = new (this->allocator) SVSInsertJob(this->allocator, label, new_flat_id, executeInsertJobWrapper, this); @@ -1014,7 +1062,10 @@ class TieredSVSIndex : public VecSimTieredIndex { if (svs_index->ready()) { // If we removed the previous vector from both svs and flat in the overwrite process, // we still return 0 (not -1). - ret = std::max(ret - svs_index->deleteVector(label), 0); + auto deleted = svs_index->deleteVector(label); + if (deleted > 0) + scheduleSVSIndexConsolidate(label); + ret = std::max(ret - deleted, 0); } } @@ -1091,13 +1142,21 @@ class TieredSVSIndex : public VecSimTieredIndex { } deleteAndUpdateInitIds(label); - return deleting_ids.size(); + ret += deleting_ids.size(); } } else { this->flatIndexGuard.unlock_shared(); } - ret += this->backendIndex->deleteVector(label); + auto deleted = this->backendIndex->deleteVector(label); + if (deleted > 0) { + if (this->getWriteMode() == VecSim_WriteInPlace) { + GetSVSIndex()->consolidate({label}); + } else { + scheduleSVSIndexConsolidate(label); + } + } + ret += deleted; return ret; } @@ -1159,10 +1218,11 @@ class TieredSVSIndex : public VecSimTieredIndex { } } info.tieredInfo.specificTieredBackendInfo.svsTieredInfo = svsTieredInfo; + // Background indexing is in progress whenever the flat buffer is non-empty: every vector + // buffered there has either a pending per-vector insert job or a batch-init job that will + // drain it into the backend. info.tieredInfo.backgroundIndexing = - svsTieredInfo.indexUpdateScheduled && info.tieredInfo.frontendCommonInfo.indexSize > 0 - ? VecSimBool_TRUE - : VecSimBool_FALSE; + info.tieredInfo.frontendCommonInfo.indexSize > 0 ? VecSimBool_TRUE : VecSimBool_FALSE; return info; } @@ -1320,4 +1380,3 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.unlock_shared(); } }; - \ No newline at end of file diff --git a/src/VecSim/vec_sim_common.h b/src/VecSim/vec_sim_common.h index d800e7cdc..7d87baea0 100644 --- a/src/VecSim/vec_sim_common.h +++ b/src/VecSim/vec_sim_common.h @@ -50,6 +50,7 @@ extern "C" { #define SVS_VAMANA_DEFAULT_TRAINING_THRESHOLD (10 * DEFAULT_BLOCK_SIZE) // 10 * 1024 vectors // Default batch update threshold for SVS index. #define SVS_VAMANA_DEFAULT_UPDATE_THRESHOLD (1 * DEFAULT_BLOCK_SIZE) // 1 * 1024 vectors +#define SVS_VAMANA_DEFAULT_CONSOLIDATE_THRESHOLD (1 * DEFAULT_BLOCK_SIZE) // 1 * 1024 vectors #define SVS_VAMANA_DEFAULT_SEARCH_WINDOW_SIZE 10 // NOTE: No need to have SVS_VAMANA_DEFAULT_SEARCH_BUFFER_CAPACITY // as the default is determined by the search_window_size @@ -275,7 +276,7 @@ typedef enum { SVS_BATCH_UPDATE_JOB, SVS_INSERT_VECTOR_JOB, SVS_GC_JOB, - SVS_GC2_JOB, + SVS_CONSOLIDATE_JOB, INVALID_JOB // to indicate that finding a JobType >= INVALID_JOB is an error } JobType; diff --git a/tests/unit/test_svs_fp16.cpp b/tests/unit/test_svs_fp16.cpp index ded23bd72..95a26e6c2 100644 --- a/tests/unit/test_svs_fp16.cpp +++ b/tests/unit/test_svs_fp16.cpp @@ -2919,13 +2919,21 @@ TYPED_TEST(FP16SVSTieredIndexTest, deleteVectorMulti) { ASSERT_EQ(tiered_index->deleteVector(vec_label), 2); ASSERT_EQ(tiered_index->indexLabelCount(), 0); + + ASSERT_EQ(mock_thread_pool.jobQ.size(), 2); + mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 1); mock_thread_pool.thread_iteration(); ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); // Test deleting a label for which both of its vector's is in SVS index. this->GenerateAndAddVector(tiered_index, dim, vec_label, vec_label); this->GenerateAndAddVector(tiered_index, dim, vec_label, other_vec_val); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 2); mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 1); + mock_thread_pool.thread_iteration(); + ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); ASSERT_EQ(tiered_index->indexLabelCount(), 1); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 2); diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index 2980d0e38..6ef887f79 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -556,14 +556,16 @@ TYPED_TEST(SVSTieredIndexTest, addVector) { ASSERT_LE(expected_mem, tiered_index->getAllocationSize()); if constexpr (TypeParam::isMulti()) { - // Add another vector under the same label + // Add another vector under the same label. The backend init was already submitted + // by the first vector (thread_pool_size batch-init jobs), so this second vector takes + // the per-vector async path and schedules one additional SVSInsertJob of its own. VecSimIndex_AddVector(tiered_index, vector, vec_label); ASSERT_EQ(tiered_index->indexSize(), 2); ASSERT_EQ(tiered_index->indexLabelCount(), 1); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 0); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 2); - // Validate that there still 1 update jobs set - ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size); + // The batch-init jobs plus the extra per-vector insert job. + ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size + 1); } } @@ -2937,12 +2939,13 @@ TYPED_TEST(SVSTieredIndexTest, writeInPlaceMode) { ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 2); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); - // Overwrite inplace - only in single-value mode + // In write-in-place mode every mutation runs synchronously, including consolidation + // of soft-deleted slots. size_t expected_marked_deleted = 0; + // Overwrite inplace - only in single-value mode if (!TypeParam::isMulti()) { TEST_DATA_T overwritten_vec[] = {1, 1, 1, 1}; tiered_index->addVector(overwritten_vec, vec_label); - expected_marked_deleted++; ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 3); ASSERT_EQ(tiered_index->indexSize(), 3); ASSERT_EQ(tiered_index->indexLabelCount(), 2); @@ -2950,9 +2953,9 @@ TYPED_TEST(SVSTieredIndexTest, writeInPlaceMode) { ASSERT_EQ(tiered_index->getDistanceFrom_Unsafe(vec_label, overwritten_vec), 0); ASSERT_EQ(tiered_index->GetSVSIndex()->getNumMarkedDeleted(), expected_marked_deleted); } - // Validate that the vector is marked as deleted. + // Validate that the vector is deleted and consolidated synchronously (in-place mode), + // so no marked-deleted entry remains. tiered_index->deleteVector(vec_label); - expected_marked_deleted++; ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), TypeParam::isMulti() ? 2 : 3); ASSERT_EQ(tiered_index->indexLabelCount(), 1); @@ -3142,12 +3145,13 @@ TYPED_TEST(SVSTieredIndexTestBasic, runGCAPI) { auto jobs_before_gc = mock_thread_pool.jobQ.size(); // Run the GC API call, expect that we will clean up the SVS index. VecSimTieredIndex_GC(tiered_index); - // Expected that GC jobs were added to the queue. - ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + mock_thread_pool.thread_pool_size); + // Expected that a single GC job was added to the queue. + ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + 1); // Run GC twice. VecSimTieredIndex_GC(tiered_index); - // Expected that no new GC jobs were added to the queue. - ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + mock_thread_pool.thread_pool_size); + // Expected that no new GC job was added to the queue (indexGCScheduled is still set until the + // pending job runs). + ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + 1); // Wait for any pending jobs to complete. As far as SVS GC is done via a job. mock_thread_pool.init_threads(); mock_thread_pool.thread_pool_join(); @@ -3415,7 +3419,7 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalSingle) { // update job paused, we have vectors 0-(n-1) in the index, let's do index modifications // Remove vector label=n-2, it is copied to backend index. - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 2), 2); + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 2), 1); // Update vector label=1. EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, 1, 10), 0); // Add a new vector @@ -3423,9 +3427,9 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalSingle) { // Add another one EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, n + 1, n + 1), 1); // Remove vector label=0, it is copied to backend index. - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 0), 2); + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 0), 1); // Remove the last vector copied to backend index - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 1), 2); + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 1), 1); // Update vector label=2. EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, 2, 20), 0); // Remove vector label=2. @@ -3447,16 +3451,15 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalSingle) { // 0:deleted, 1: 10, 2: deleted, 3:3, ..., n-2:deleted n-1: 10(n-1), n+1: n+1; // total: n-2 vectors and labels ASSERT_EQ(tiered_index->indexLabelCount(), n - 2); - EXPECT_EQ(tiered_index->GetBackendIndex()->indexLabelCount(), n - 5); + // Nothing remains in flat. + EXPECT_EQ(tiered_index->GetBackendIndex()->indexLabelCount(), n - 2); - // We added 3 vectors to the flat index and removed 5 vectors from the backend index. - // Backend index: 0:deleted, 1:deleted, 2:deleted, 3:3, ..., n-2:deleted, n-1:deleted; - // total: n-5 - EXPECT_EQ(tiered_index->GetBackendIndex()->indexSize(), n); - ASSERT_EQ(tiered_index->getNumMarkedDeleted(), 5); - // Frontend index: 1:10, n-1:10(n-1), n+1:n+1 - ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 3); - ASSERT_EQ(tiered_index->indexSize(), n + tiered_index->GetFlatIndex()->indexSize()); + EXPECT_EQ(tiered_index->GetBackendIndex()->indexSize(), n - 2); + ASSERT_EQ(tiered_index->getNumMarkedDeleted(), 0); + // Flat buffer fully drained by the insert jobs. + ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); + ASSERT_EQ(tiered_index->indexSize(), tiered_index->GetBackendIndex()->indexSize() + + tiered_index->GetFlatIndex()->indexSize()); double abs_err = 1e-2; // Allow a larger relative error for quantization. TEST_DATA_T expected_vector[dim]; @@ -3469,7 +3472,7 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalSingle) { GenerateVector(expected_vector, dim, 0); ASSERT_TRUE(std::isnan(tiered_index->getDistanceFrom_Unsafe(n - 2, expected_vector))); - // Vector label=1, with value 10 should be in the flat index. + // Vector label=1 was updated to value 10; its latest value is retrievable from the index. GenerateVector(expected_vector, dim, 10); ASSERT_NEAR(tiered_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); @@ -3548,22 +3551,24 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalMulti) { // update job paused, we have vectors 0-(n-1) in the index, let's do index modifications - // Remove vector label=n-2, it is copied to backend index. - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 2), 2); + // Remove vector label=n-2. At the pause it lives only in the backend + //, so a single vector is removed. + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 2), 1); // Add one more vector label=1. EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, 1, 10), 1); // Add a new vector EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, n, n), 1); // Add another one EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, n + 1, n + 1), 1); - // Remove vector label=0, it is copied to backend index. - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 0), 2); - // Remove the last vector copied to backend index - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 1), 2); + // Remove vector label=0, only in the backend at the pause. + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 0), 1); + // Remove the last vector, only in the backend at the pause. + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n - 1), 1); // Add one more vector label=2. EXPECT_EQ(GenerateAndAddVector(tiered_index, dim, 2, 20), 1); - // Remove vector label=2: for multi: old is copied to backend , old + new are in flat - EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 2), 3); + // Remove vector label=2: for multi both the original (2) and the new (20) copies sit in + // the flat buffer at this point, so 2 vectors are removed. + EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, 2), 2); // Remove vector label=n - in flat only EXPECT_EQ(VecSimIndex_DeleteVector(tiered_index, n), 1); // Add vector (n-1) again @@ -3581,15 +3586,16 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalMulti) { // 0: deleted, 1: (1,10), 2: deleted, 3:3, ..., n-2: deleted n-1: 10(n-1), n+1: n+1; // total: n-2 labels, n-1 vectors ASSERT_EQ(tiered_index->indexLabelCount(), n - 2); - EXPECT_EQ(tiered_index->GetBackendIndex()->indexLabelCount(), n - 4); + // In the async-insert design the per-vector insert jobs drain every surviving vector from + // the flat buffer into the backend, so nothing remains in flat. + EXPECT_EQ(tiered_index->GetBackendIndex()->indexLabelCount(), n - 2); - // We added 3 vectors to the flat index and removed 4 vectors from the backend index. - // Backend index: 0:deleted, 1:1, 2:deleted, 3:3, ..., n-2:deleted, n-1:deleted; total: n-4 - EXPECT_EQ(tiered_index->GetBackendIndex()->indexSize(), n); - ASSERT_EQ(tiered_index->getNumMarkedDeleted(), 4); - // Frontend index: 1:10, n-1:10(n-1), n+1:n+1 - ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 3); - ASSERT_EQ(tiered_index->indexSize(), n + tiered_index->GetFlatIndex()->indexSize()); + EXPECT_EQ(tiered_index->GetBackendIndex()->indexSize(), n - 1); + ASSERT_EQ(tiered_index->getNumMarkedDeleted(), 0); + // Flat buffer fully drained by the insert jobs. + ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); + ASSERT_EQ(tiered_index->indexSize(), tiered_index->GetBackendIndex()->indexSize() + + tiered_index->GetFlatIndex()->indexSize()); double abs_err = 1e-2; // Allow a larger relative error for quantization. TEST_DATA_T expected_vector[dim]; @@ -3602,11 +3608,11 @@ TYPED_TEST(SVSTieredIndexTestBasic, testSwapJournalMulti) { GenerateVector(expected_vector, dim, 0); ASSERT_TRUE(std::isnan(tiered_index->getDistanceFrom_Unsafe(n - 2, expected_vector))); - // There are 2 vectors labeled "1" with values 1 in backend and 10 in flat. - // We expect the minimal distance for the query 10 to be taken from flat index. + // Label "1" is multi-valued and keeps both of its vectors (values 1 and 10). + // The minimal distance for the query 10 should match the second value. GenerateVector(expected_vector, dim, 10); ASSERT_NEAR(tiered_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); - // And the minimal distance for the query 1.0 to be taken from backend + // And the minimal distance for the query 1.0 should match the first value. GenerateVector(expected_vector, dim, 1); ASSERT_NEAR(tiered_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); @@ -3706,8 +3712,9 @@ TYPED_TEST(SVSTieredIndexTestBasic, testDeletedJournalSingle) { mock_thread_pool.thread_pool_join(); - // Verify that vectors labels: {0, 1, 2, n-1} are marked as deleted in the SVS index. - ASSERT_EQ(tiered_index->GetSVSIndex()->getNumMarkedDeleted(), 4); + // The update job is paused before it transfers vectors to the backend, so at the pause the + // backend is still empty. + ASSERT_EQ(tiered_index->GetSVSIndex()->getNumMarkedDeleted(), 0); // Verify that the deleted vectors are not accessible. double abs_err = 1e-2; // Allow a larger relative error for quantization. @@ -3724,9 +3731,10 @@ TYPED_TEST(SVSTieredIndexTestBasic, testDeletedJournalSingle) { ASSERT_TRUE(std::isnan(tiered_index->getDistanceFrom_Unsafe(label, expected_vector))); } - // label 1 - updated to 10 but deleted in the SVS index - ASSERT_TRUE(flat_index->isLabelExists(1)); - ASSERT_FALSE(svs_index->isLabelExists(1)); + // label 1 - updated to 10 during the pause. The insert job drains it from the flat buffer + // into the backend, so it now lives in the SVS index (not flat) with its updated value. + ASSERT_FALSE(flat_index->isLabelExists(1)); + ASSERT_TRUE(svs_index->isLabelExists(1)); GenerateVector(expected_vector, dim, 10); ASSERT_NEAR(tiered_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); } @@ -3808,8 +3816,9 @@ TYPED_TEST(SVSTieredIndexTestBasic, testDeletedJournalMulti) { mock_thread_pool.thread_pool_join(); - // Verify that vectors labels: {0, 2, n-1} are marked as deleted in the SVS index. - ASSERT_EQ(tiered_index->GetSVSIndex()->getNumMarkedDeleted(), 3); + // The update job is paused before it transfers vectors to the backend, so at the pause the + // backend is still empty. + ASSERT_EQ(tiered_index->GetSVSIndex()->getNumMarkedDeleted(), 0); // Verify that the deleted vectors are not accessible. double abs_err = 1e-2; // Allow a larger relative error for quantization. @@ -3826,13 +3835,14 @@ TYPED_TEST(SVSTieredIndexTestBasic, testDeletedJournalMulti) { ASSERT_TRUE(std::isnan(tiered_index->getDistanceFrom_Unsafe(label, expected_vector))); } - // label 1 - multi-value 1 (in SVS) and 10 (in flat) - ASSERT_TRUE(flat_index->isLabelExists(1)); + // label 1 - multi-value with both vectors (1 and 10). The insert jobs drain the flat buffer + // into the backend, so both values now live in the SVS index and none remain in flat. + ASSERT_FALSE(flat_index->isLabelExists(1)); ASSERT_TRUE(svs_index->isLabelExists(1)); GenerateVector(expected_vector, dim, 1); ASSERT_NEAR(backend_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); GenerateVector(expected_vector, dim, 10); - ASSERT_NEAR(flat_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); + ASSERT_NEAR(backend_index->getDistanceFrom_Unsafe(1, expected_vector), 0, abs_err); } TEST(SVSTieredIndexTest, testThreadPool) { From 649460a7a28553129a7c76b5eddde1c0bf99c85c Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Thu, 30 Jul 2026 00:17:00 -0700 Subject: [PATCH 15/18] fix writer starvation for compaction --- src/VecSim/algorithms/svs/svs_tiered.h | 64 ++++++++++++++++++++------ 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index c94d3b711..c72503829 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -623,29 +623,55 @@ class TieredSVSIndex : public VecSimTieredIndex { * @note no need to implement extra non-static method, as GC logic is simple enough to be done * here. */ - static void SVSIndexGCWrapper(AsyncJob *job) { - auto gc_job = static_cast(job); - auto index = static_cast *>(gc_job->index); + // static void SVSIndexGCWrapper(AsyncJob *job) { + // auto gc_job = static_cast(job); + // auto index = static_cast *>(gc_job->index); + + // std::shared_lock lock(index->updateJobMutex); + // // Do SVS index GC + // index->backendIndex->log(VecSimCommonStrings::LOG_VERBOSE_STRING, + // "running asynchronous GC for tiered SVS index"); + // auto svs_index = index->GetSVSIndex(); + // if (index->backendIndex->indexSize() == 0) { + // index->indexGCScheduled.clear(); + // delete job; + // // No need to run GC on an empty index. + // return; + // } + // // svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); + // svs_index->setNumThreads(1); + // // VecSimIndexAbstract::runGC() is protected + // fprintf(stderr, "runGC\n"); + // static_cast(index->backendIndex)->runGC(); + // fprintf(stderr, "GC done\n"); + + // // Release the scheduled flag to allow scheduling again + // index->indexGCScheduled.clear(); + // delete job; + // } + + static void SVSIndexGCWrapper(VecSimIndex *idx, size_t availableThreads) { + assert(availableThreads > 0); + auto index = static_cast *>(idx); + assert(index); - std::shared_lock lock(index->updateJobMutex); // Do SVS index GC index->backendIndex->log(VecSimCommonStrings::LOG_VERBOSE_STRING, "running asynchronous GC for tiered SVS index"); auto svs_index = index->GetSVSIndex(); if (index->backendIndex->indexSize() == 0) { - index->indexGCScheduled.clear(); - delete job; // No need to run GC on an empty index. return; } - // svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); - svs_index->setNumThreads(1); + std::lock_guard lock(index->updateJobMutex); + + svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); // VecSimIndexAbstract::runGC() is protected static_cast(index->backendIndex)->runGC(); + svs_index->setNumThreads(1); // Release the scheduled flag to allow scheduling again index->indexGCScheduled.clear(); - delete job; } static void SVSIndexConsolidateWrapper(AsyncJob *job) { @@ -696,11 +722,17 @@ class TieredSVSIndex : public VecSimTieredIndex { return; } - AsyncJob *new_GC_job = new (this->allocator) - SVSGCJob(this->allocator, SVSIndexGCWrapper, this); + auto total_threads = this->GetSVSIndex()->getThreadPoolCapacity(); + auto jobs = SVSMultiThreadJob::createJobs( + this->allocator, SVS_GC_JOB, SVSIndexGCWrapper, this, total_threads, + std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); + this->submitJobs(jobs); + + // AsyncJob *new_GC_job = new (this->allocator) + // SVSGCJob(this->allocator, SVSIndexGCWrapper, this); - // Insert job to the queue. - this->submitSingleJob(new_GC_job); + // // Insert job to the queue. + // this->submitSingleJob(new_GC_job); } void scheduleSVSIndexConsolidate(labelType label) { @@ -780,7 +812,10 @@ class TieredSVSIndex : public VecSimTieredIndex { this->flatIndexGuard.unlock_shared(); // svs_index->setNumThreads(1); - svs_index->addVector(blob_copy.get(), job->label); + { + std::shared_lock lock(updateJobMutex); + svs_index->addVector(blob_copy.get(), job->label); + } // Remove the vector and the insert job from the flat buffer. this->flatIndexGuard.lock(); @@ -1006,6 +1041,7 @@ class TieredSVSIndex : public VecSimTieredIndex { scheduleSVSIndexConsolidate(label); } + std::shared_lock lock(updateJobMutex); return ret = svs_index->addVector(storage_blob.get(), label); } else { this->flatIndexGuard.unlock_shared(); From 07f2c8fdf3b7ab80d80c8e9488c8a0b75192d37d Mon Sep 17 00:00:00 2001 From: Dmitry Razdoburdin Date: Tue, 11 Aug 2026 03:52:21 -0700 Subject: [PATCH 16/18] adopt sigle label consolidation --- src/VecSim/algorithms/svs/svs.h | 4 +-- src/VecSim/algorithms/svs/svs_tiered.h | 40 +++++++++++--------------- tests/unit/test_svs_fp16.cpp | 9 ++++-- tests/unit/test_svs_tiered.cpp | 18 +++++++----- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index e410529f7..33cfbc334 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -310,11 +310,11 @@ class SVSIndex : public VecSimIndexAbstract, fl assert(this->impl_ != nullptr); setReady(); } else { - impl_->add_points(points, ids); + impl_->add_points(points, ids, /*reuse_empty*/ false); } } else { // Add new points to existing SVS index - impl_->add_points(points, ids); + impl_->add_points(points, ids, /*reuse_empty*/ false); this->pimplGuard_.unlock_shared(); } diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index c72503829..27d2f4d12 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -259,7 +259,6 @@ class TieredSVSIndex : public VecSimTieredIndex { size_t trainingTriggerThreshold; size_t updateTriggerThreshold; - size_t consolidateTriggerThreshold; size_t updateJobWaitTime; // Used to prevent scheduling multiple index update jobs at the same time. // As far as the update job does a batch update, job queue should have just 1 job at the moment. @@ -275,7 +274,6 @@ class TieredSVSIndex : public VecSimTieredIndex { std::atomic backendInitSubmited{false}; std::unordered_set ids_to_init_; - std::vector labels_to_consolidate_; vecsim_stl::unordered_map> labelToInsertJobs; // A mapping to hold invalid jobs, so we can dispose them upon index deletion. @@ -661,6 +659,7 @@ class TieredSVSIndex : public VecSimTieredIndex { auto svs_index = index->GetSVSIndex(); if (index->backendIndex->indexSize() == 0) { // No need to run GC on an empty index. + index->indexGCScheduled.clear(); return; } std::lock_guard lock(index->updateJobMutex); @@ -736,16 +735,11 @@ class TieredSVSIndex : public VecSimTieredIndex { } void scheduleSVSIndexConsolidate(labelType label) { - labels_to_consolidate_.push_back(label); + AsyncJob *new_consolidate_job = new (this->allocator) + SVSConsolidateJob(this->allocator, {label}, SVSIndexConsolidateWrapper, this); - if (labels_to_consolidate_.size() >= consolidateTriggerThreshold) { - AsyncJob *new_consolidate_job = new (this->allocator) - SVSConsolidateJob(this->allocator, labels_to_consolidate_, SVSIndexConsolidateWrapper, this); - - // Insert job to the queue. - this->submitSingleJob(new_consolidate_job); - labels_to_consolidate_.clear(); - } + // Insert job to the queue. + this->submitSingleJob(new_consolidate_job); } private: @@ -942,8 +936,6 @@ class TieredSVSIndex : public VecSimTieredIndex { ? SVS_VAMANA_DEFAULT_TRAINING_THRESHOLD : this->updateTriggerThreshold; - this->consolidateTriggerThreshold = SVS_VAMANA_DEFAULT_CONSOLIDATE_THRESHOLD; - this->trainingTriggerThreshold = tiered_svs_params.trainingTriggerThreshold == 0 ? default_training_threshold @@ -1016,18 +1008,20 @@ class TieredSVSIndex : public VecSimTieredIndex { if ((!svs_index->ready()) && (!this->backendInitSubmited.load(std::memory_order_acquire))) { // Add vector to the frontend index. std::lock_guard lock(this->flatIndexGuard); - if (!this->frontendIndex->isMultiValue() && this->frontendIndex->isLabelExists(label)) { - deleteAndUpdateInitIds(label); - } - ids_to_init_.insert(this->frontendIndex->indexSize()); - const auto ft_ret = this->frontendIndex->addVector(blob, label); - ret = std::max(ret + ft_ret, 0); + if ((!svs_index->ready()) && (!this->backendInitSubmited.load(std::memory_order_acquire))) { + if (!this->frontendIndex->isMultiValue() && this->frontendIndex->isLabelExists(label)) { + deleteAndUpdateInitIds(label); + } + ids_to_init_.insert(this->frontendIndex->indexSize()); + const auto ft_ret = this->frontendIndex->addVector(blob, label); + ret = std::max(ret + ft_ret, 0); - if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { - this->backendInitSubmited.store(true, std::memory_order_release); - scheduleSVSIndexInit(); + if (this->frontendIndex->indexSize() >= this->trainingTriggerThreshold) { + this->backendInitSubmited.store(true, std::memory_order_release); + scheduleSVSIndexInit(); + } + return ret; } - return ret; } this->flatIndexGuard.lock_shared(); diff --git a/tests/unit/test_svs_fp16.cpp b/tests/unit/test_svs_fp16.cpp index 95a26e6c2..dc36615f6 100644 --- a/tests/unit/test_svs_fp16.cpp +++ b/tests/unit/test_svs_fp16.cpp @@ -2857,8 +2857,9 @@ TYPED_TEST(FP16SVSTieredIndexTest, deleteVector) { ASSERT_EQ(tiered_index->indexSize(), 1); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 1); - // Move the vector to SVS by executing the insert job. - mock_thread_pool.thread_iteration(); + // Move the vector to SVS by executing the pending jobs. + while (mock_thread_pool.jobQ.size() > 0) + mock_thread_pool.thread_iteration(); ASSERT_EQ(tiered_index->indexLabelCount(), 1); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 1); // Scalar quantization accuracy is insufficient for this check. @@ -2905,7 +2906,9 @@ TYPED_TEST(FP16SVSTieredIndexTest, deleteVectorMulti) { ASSERT_EQ(tiered_index->deleteVector(vec_label), 2); ASSERT_EQ(tiered_index->indexSize(), 0); ASSERT_EQ(tiered_index->indexLabelCount(), 0); - mock_thread_pool.thread_iteration(); + + while (mock_thread_pool.jobQ.size() > 0) + mock_thread_pool.thread_iteration(); ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); // Test deleting a label for which both of its vector's is in the flat index. diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index 6ef887f79..bbf655667 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -1088,8 +1088,9 @@ TYPED_TEST(SVSTieredIndexTest, deleteVector) { ASSERT_EQ(tiered_index->indexSize(), 1); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 1); - // Move the vector to SVS by executing the insert job. - mock_thread_pool.thread_iteration(); + // Move the vector to SVS by executing the pending jobs. + while (mock_thread_pool.jobQ.size() > 0) + mock_thread_pool.thread_iteration(); ASSERT_EQ(tiered_index->indexLabelCount(), 1); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 1); // Scalar quantization accuracy is insufficient for this check. @@ -1218,6 +1219,8 @@ TYPED_TEST(SVSTieredIndexTestBasic, deleteVectorMulti) { ASSERT_EQ(tiered_index->deleteVector(vec_label), 2); ASSERT_EQ(tiered_index->indexSize(), 0); ASSERT_EQ(tiered_index->indexLabelCount(), 0); + + mock_thread_pool.thread_iteration(); mock_thread_pool.thread_iteration(); ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); @@ -2662,7 +2665,8 @@ TYPED_TEST(SVSTieredIndexTestBasic, overwriteVectorBasic) { ASSERT_EQ(tiered_index->getDistanceFrom_Unsafe(0, overwritten_vec), 0); // Ingest the updated vector to SVS. - mock_thread_pool.thread_iteration(); + while (mock_thread_pool.jobQ.size() > 0) + mock_thread_pool.thread_iteration(); ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 1); ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); ASSERT_EQ(tiered_index->indexLabelCount(), 1); @@ -3146,12 +3150,12 @@ TYPED_TEST(SVSTieredIndexTestBasic, runGCAPI) { // Run the GC API call, expect that we will clean up the SVS index. VecSimTieredIndex_GC(tiered_index); // Expected that a single GC job was added to the queue. - ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + 1); + ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + mock_thread_pool.thread_pool_size); // Run GC twice. VecSimTieredIndex_GC(tiered_index); - // Expected that no new GC job was added to the queue (indexGCScheduled is still set until the - // pending job runs). - ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + 1); + // Expected that no new GC jobs were added to the queue (indexGCScheduled is still set until the + // pending jobs run). + ASSERT_EQ(mock_thread_pool.jobQ.size(), jobs_before_gc + mock_thread_pool.thread_pool_size); // Wait for any pending jobs to complete. As far as SVS GC is done via a job. mock_thread_pool.init_threads(); mock_thread_pool.thread_pool_join(); From db1d10616cd4dfc403da49b6adeee92a7e1a8708 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Fri, 21 Aug 2026 21:27:51 -0700 Subject: [PATCH 17/18] vecsim separate fin-grain index enabling --- src/VecSim/algorithms/svs/svs.h | 6 ++- src/VecSim/algorithms/svs/svs_extensions.h | 22 +++++++--- .../algorithms/svs/svs_serializer_impl.h | 6 +-- src/VecSim/algorithms/svs/svs_utils.h | 40 ++++++++++++++++--- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 33cfbc334..663725d7c 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -93,10 +93,12 @@ class SVSIndex : public VecSimIndexAbstract, fl using graph_builder_t = SVSGraphBuilder; using graph_type = typename graph_builder_t::graph_type; + // The concurrent index mirrors the upstream template signature and member surface, so + // the only change needed here is the namespace. using impl_type = std::conditional_t< isMulti, - svs::index::vamana::MultiMutableVamanaIndex, - svs::index::vamana::MutableVamanaIndex>; + svs::concurrent::MultiMutableVamanaIndex, + svs::concurrent::MutableVamanaIndex>; bool forcePreprocessing; diff --git a/src/VecSim/algorithms/svs/svs_extensions.h b/src/VecSim/algorithms/svs/svs_extensions.h index 3903d2289..1700b30c9 100644 --- a/src/VecSim/algorithms/svs/svs_extensions.h +++ b/src/VecSim/algorithms/svs/svs_extensions.h @@ -10,10 +10,20 @@ #pragma once #include "VecSim/algorithms/svs/svs_utils.h" #include "svs/extensions/vamana/scalar.h" +// Tells SQDataset that the concurrent SegmentedBlocked tag is a blocked allocator, so +// that resize()/compact() are not constrained away. Without it a scalar-quantized +// concurrent index fails to compile the moment the index grows. +#include "svs/concurrent/extensions/scalar.h" #if HAVE_SVS_LVQ #include SVS_LVQ_HEADER #include SVS_LEANVEC_HEADER +// The same treatment for the LVQ and LeanVec datasets, each of which keeps its own private +// copy of the blocked-allocator trait. LeanVec additionally picks the allocator for its inner +// datasets through that trait, and its blocked branch hardcodes svs::data::Blocked -- so +// without this the inner storage would quietly lose the grow-stable guarantee. +#include "svs/concurrent/extensions/lvq.h" +#include "svs/concurrent/extensions/leanvec.h" #endif // HAVE_SVS_LVQ // Scalar Quantization traits for SVS @@ -21,7 +31,7 @@ template struct SVSStorageTraits { using element_type = std::int8_t; using allocator_type = svs_details::SVSAllocator; - using blocked_type = svs::data::Blocked>; + using blocked_type = svs::concurrent::SegmentedBlocked>; using index_storage_type = svs::quantization::scalar::SQDataset; @@ -32,7 +42,7 @@ struct SVSStorageTraits { // SVS block size is a power of two, so we can use it directly auto svs_bs = svs_details::SVSBlockSize(block_size, element_size(dim)); allocator_type data_allocator{std::move(allocator)}; - return svs::make_blocked_allocator_handle({svs_bs}, data_allocator); + return svs_details::make_segmented_blocked_allocator_handle({svs_bs}, data_allocator); } static constexpr VecSimSvsQuantBits get_compression_mode() { return VecSimSvsQuant_Scalar; } @@ -89,7 +99,7 @@ template struct SVSStorageTraits 1)>> { using allocator_type = svs_details::SVSAllocator; - using blocked_type = svs::data::Blocked>; + using blocked_type = svs::concurrent::SegmentedBlocked>; using strategy_type = typename svs_details::LVQSelector::strategy; using index_storage_type = svs::quantization::lvq::LVQDataset @@ -163,7 +173,7 @@ struct SVSStorageTraits struct SVSStorageTraits { using allocator_type = svs_details::SVSAllocator; - using blocked_type = svs::data::Blocked>; + using blocked_type = svs::concurrent::SegmentedBlocked>; using index_storage_type = svs::leanvec::LeanDataset, svs::leanvec::UsingLVQ, svs::Dynamic, svs::Dynamic, blocked_type>; @@ -193,7 +203,7 @@ struct SVSStorageTraits { // SVS block size is a power of two, so we can use it directly auto svs_bs = svs_details::SVSBlockSize(block_size, element_size(dim)); allocator_type data_allocator{std::move(allocator)}; - return svs::make_blocked_allocator_handle({svs_bs}, data_allocator); + return svs_details::make_segmented_blocked_allocator_handle({svs_bs}, data_allocator); } template diff --git a/src/VecSim/algorithms/svs/svs_serializer_impl.h b/src/VecSim/algorithms/svs/svs_serializer_impl.h index 6f6757922..5e551ef81 100644 --- a/src/VecSim/algorithms/svs/svs_serializer_impl.h +++ b/src/VecSim/algorithms/svs/svs_serializer_impl.h @@ -93,17 +93,17 @@ void SVSIndex compareMetadataFile(folder_path + "/metadata"); if constexpr (isMulti) { - auto loaded = svs::index::vamana::auto_multi_dynamic_assemble( + auto loaded = svs::concurrent::auto_multi_dynamic_assemble( folder_path + "/config", SVS_LAZY(graph_builder_t::load(folder_path + "/graph", this->blockSize, this->buildParams, this->getAllocator())), SVS_LAZY(storage_traits_t::load(folder_path + "/data", this->blockSize, this->dim, this->getAllocator())), distance_f(), std::move(threadpool_handle), - svs::index::vamana::MultiMutableVamanaLoad::FROM_MULTI, logger_); + svs::concurrent::MultiMutableVamanaLoad::FROM_MULTI, logger_); impl_ = std::make_unique(std::move(loaded)); } else { - auto loaded = svs::index::vamana::auto_dynamic_assemble( + auto loaded = svs::concurrent::auto_dynamic_assemble( folder_path + "/config", SVS_LAZY(graph_builder_t::load(folder_path + "/graph", this->blockSize, this->buildParams, this->getAllocator())), diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index 2e240358f..3c1de0f14 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -14,6 +14,10 @@ #include "svs/core/distance.h" #include "svs/lib/float16.h" #include "svs/index/vamana/dynamic_index.h" +// Pulls in `svs::index::vamana::concurrent` and the `svs::concurrent` alias. The concurrent +// namespace redeclares only the entities it has to replace; every other name (build/search +// parameters, extension points) still resolves to the enclosing `svs::index::vamana`. +#include "svs/concurrent/concurrent.h" #if HAVE_SVS_LVQ #include "svs/cpuid.h" @@ -198,6 +202,21 @@ inline std::pair isSVSQuantBitsSupported(VecSimSvsQuan // unreachable code, but to avoid compiler warning return std::make_pair(VecSimSvsQuant_NONE, false); } + +// The `svs::concurrent` counterpart of `svs::make_blocked_allocator_handle`. +// +// The upstream helper cannot be reused here: it hardcodes `svs::data::Blocked` in its return +// type, so there is no way to ask it for the grow-stable tag. The type-erasing +// `AllocatorHandle` wrapper is what makes the compressed datasets (which take an allocator +// handle rather than a concrete allocator) work with VecSim's own allocator. +template +svs::concurrent::SegmentedBlocked> +make_segmented_blocked_allocator_handle(const svs::data::BlockingParameters ¶meters, + Alloc alloc) { + using handle_type = svs::AllocatorHandle; + return svs::concurrent::SegmentedBlocked{ + parameters, svs::make_allocator_handle(std::move(alloc))}; +} } // namespace svs_details template ; // Used in creating storage + // `SegmentedBlocked` is a drop-in replacement for `svs::data::Blocked` (it derives from + // it and carries the same blocking parameters) that additionally keeps already-published + // elements at a stable address when the dataset grows, which is what allows a concurrent + // reader to keep a pointer into the data while a writer appends. + using blocked_type = svs::concurrent::SegmentedBlocked; // svs::Dynamic means runtime dimensionality in opposite to compile-time dimensionality - using index_storage_type = svs::data::BlockedData; + using index_storage_type = + svs::concurrent::SegmentedBlockedData; static constexpr bool is_compressed() { return false; } @@ -272,9 +296,13 @@ struct SVSStorageTraits { template struct SVSGraphBuilder { using allocator_type = svs_details::SVSAllocator; - using blocked_type = svs::data::Blocked; - using graph_data_type = svs::data::BlockedData; - using graph_type = svs::graphs::SimpleGraph; + using blocked_type = svs::concurrent::SegmentedBlocked; + using graph_data_type = + svs::concurrent::SegmentedBlockedData; + // The concurrent `SimpleGraph` has the same shape as the upstream one but guards each + // adjacency list with a sequence lock, so a reader can traverse an edge list that a + // writer is concurrently rewriting. + using graph_type = svs::concurrent::graphs::SimpleGraph; static blocked_type make_blocked_allocator(size_t block_size, size_t graph_max_degree, std::shared_ptr allocator) { @@ -304,7 +332,7 @@ struct SVSGraphBuilder { // based on the data types, which we found to perform better through heuristic analysis. auto prefetch_parameters = svs::index::vamana::extensions::estimate_prefetch_parameters(data); - auto builder = svs::index::vamana::VamanaBuilder( + auto builder = svs::concurrent::VamanaBuilder( graph, data, std::move(distance), parameters, threadpool, prefetch_parameters, logger); // Specific to the Vamana algorithm: From a540c0a1227a4514ad26b7ffce2272e8cc41152a Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Wed, 26 Aug 2026 21:06:45 -0700 Subject: [PATCH 18/18] Rename SVSIndexBase thread API to match upstream Upstream renamed the three thread-control methods on SVSIndexBase: getNumThreads -> getParallelism setNumThreads -> setParallelism getThreadPoolCapacity -> getPoolSize Adopt those names here ahead of merging upstream/main. This is a pure rename -- 32 lines across 6 files, mechanically verified by reversing the substitution and diffing against the parent commit. Method bodies deliberately keep this branch's threadpool API (size/resize/capacity), since VecSimSVSThreadPool here is still the per-index owned pool. Upstream reworked it into a process-wide singleton with thread renting, sized via VecSim_UpdateThreadPoolSize(); that is genuine divergence to reconcile in the merge, not something a rename should paper over. The point is to remove this conflict class before merging. The names collided on roughly half the affected lines without producing conflict markers, so git resolved some toward upstream and some toward here, yielding a tree that referenced methods it no longer declared. Co-Authored-By: Claude Opus 5 --- src/VecSim/algorithms/svs/svs.h | 20 ++++++++++---------- src/VecSim/algorithms/svs/svs_tiered.h | 26 +++++++++++++------------- tests/benchmark/bm_utils.h | 4 ++-- tests/benchmark/bm_vecsim_svs.h | 2 +- tests/unit/test_svs_fp16.cpp | 6 +++--- tests/unit/test_svs_tiered.cpp | 6 +++--- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 663725d7c..b2a7699ba 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -44,9 +44,9 @@ struct SVSIndexBase virtual void consolidate(const std::vector& labels) = 0; virtual bool isLabelExists(labelType label) const = 0; virtual size_t indexStorageSize() const = 0; - virtual size_t getNumThreads() const = 0; - virtual void setNumThreads(size_t numThreads) = 0; - virtual size_t getThreadPoolCapacity() const = 0; + virtual size_t getParallelism() const = 0; + virtual void setParallelism(size_t parallelism) = 0; + virtual size_t getPoolSize() const = 0; virtual bool isCompressed() const = 0; virtual bool ready() const = 0; @@ -504,8 +504,8 @@ class SVSIndex : public VecSimIndexAbstract, fl .maxCandidatePoolSize = this->buildParams.max_candidate_pool_size, .pruneTo = this->buildParams.prune_to, .useSearchHistory = this->buildParams.use_full_search_history, - .numThreads = this->getThreadPoolCapacity(), - .lastReservedThreads = this->getNumThreads(), + .numThreads = this->getPoolSize(), + .lastReservedThreads = this->getParallelism(), .numberOfMarkedDeletedNodes = this->num_marked_deleted, .searchWindowSize = this->search_window_size, .searchBufferCapacity = this->search_buffer_capacity, @@ -611,14 +611,14 @@ class SVSIndex : public VecSimIndexAbstract, fl // Enforce single-threaded execution for single vector operations to ensure optimal // performance and consistent behavior. Callers must set numThreads=1 before calling this // method. - assert(getNumThreads() == 1 && "Can't use more than one thread to insert a single vector"); + assert(getParallelism() == 1 && "Can't use more than one thread to insert a single vector"); return addVectorsImpl(vector_data, &label, 1); } int addVectors(const void *vectors_data, const labelType *labels, size_t n) override { // Prevent misuse: single vector operations should use addVector(), not addVectors() with // n=1 This ensures proper thread management and API contract enforcement. - assert(!(n == 1 && getNumThreads() > 1) && + assert(!(n == 1 && getParallelism() > 1) && "Can't use more than one thread to insert a single vector"); return addVectorsImpl(vectors_data, labels, n); } @@ -638,10 +638,10 @@ class SVSIndex : public VecSimIndexAbstract, fl } } - size_t getNumThreads() const override { return threadpool_.size(); } - void setNumThreads(size_t numThreads) override { threadpool_.resize(numThreads); } + size_t getParallelism() const override { return threadpool_.size(); } + void setParallelism(size_t parallelism) override { threadpool_.resize(parallelism); } - size_t getThreadPoolCapacity() const override { return threadpool_.capacity(); } + size_t getPoolSize() const override { return threadpool_.capacity(); } bool isCompressed() const override { return storage_traits_t::is_compressed(); } diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 27d2f4d12..a240fcd09 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -636,8 +636,8 @@ class TieredSVSIndex : public VecSimTieredIndex { // // No need to run GC on an empty index. // return; // } - // // svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); - // svs_index->setNumThreads(1); + // // svs_index->setParallelism(std::min(availableThreads, index->backendIndex->indexSize())); + // svs_index->setParallelism(1); // // VecSimIndexAbstract::runGC() is protected // fprintf(stderr, "runGC\n"); // static_cast(index->backendIndex)->runGC(); @@ -664,10 +664,10 @@ class TieredSVSIndex : public VecSimTieredIndex { } std::lock_guard lock(index->updateJobMutex); - svs_index->setNumThreads(std::min(availableThreads, index->backendIndex->indexSize())); + svs_index->setParallelism(std::min(availableThreads, index->backendIndex->indexSize())); // VecSimIndexAbstract::runGC() is protected static_cast(index->backendIndex)->runGC(); - svs_index->setNumThreads(1); + svs_index->setParallelism(1); // Release the scheduled flag to allow scheduling again index->indexGCScheduled.clear(); @@ -679,7 +679,7 @@ class TieredSVSIndex : public VecSimTieredIndex { std::shared_lock lock(index->updateJobMutex); auto svs_index = index->GetSVSIndex(); - svs_index->setNumThreads(1); + svs_index->setParallelism(1); svs_index->consolidate(consolidate_job->labels); delete job; } @@ -708,7 +708,7 @@ class TieredSVSIndex : public VecSimTieredIndex { return; } - auto total_threads = this->GetSVSIndex()->getThreadPoolCapacity(); + auto total_threads = this->GetSVSIndex()->getPoolSize(); auto jobs = SVSMultiThreadJob::createJobs( this->allocator, SVS_BATCH_UPDATE_JOB, initSVSIndexWrapper, this, total_threads, std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); @@ -721,7 +721,7 @@ class TieredSVSIndex : public VecSimTieredIndex { return; } - auto total_threads = this->GetSVSIndex()->getThreadPoolCapacity(); + auto total_threads = this->GetSVSIndex()->getPoolSize(); auto jobs = SVSMultiThreadJob::createJobs( this->allocator, SVS_GC_JOB, SVSIndexGCWrapper, this, total_threads, std::chrono::microseconds(updateJobWaitTime), &uncompletedJobs); @@ -805,7 +805,7 @@ class TieredSVSIndex : public VecSimTieredIndex { memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), data_size); this->flatIndexGuard.unlock_shared(); - // svs_index->setNumThreads(1); + // svs_index->setParallelism(1); { std::shared_lock lock(updateJobMutex); svs_index->addVector(blob_copy.get(), job->label); @@ -876,11 +876,11 @@ class TieredSVSIndex : public VecSimTieredIndex { { auto svs_index = GetSVSIndex(); - svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size())); + svs_index->setParallelism(std::min(availableThreads, labels_to_move.size())); assert(labels_to_move.size() == vectors_to_move.size() / this->frontendIndex->getDim()); auto impl = svs_index->createImpl(vectors_to_move.data(), labels_to_move.data(), labels_to_move.size()); - svs_index->setNumThreads(1); + svs_index->setParallelism(1); svs_index->setImpl(std::move(impl)); } @@ -990,9 +990,9 @@ class TieredSVSIndex : public VecSimTieredIndex { std::lock_guard lock(this->updateJobMutex); // Set available thread count to 1 for single vector write-in-place operation. // This maintains the contract that single vector operations use exactly one thread. - // TODO: Replace this setNumThreads(1) call with an assertion once we establish + // TODO: Replace this setParallelism(1) call with an assertion once we establish // a contract that write-in-place mode guarantees numThreads == 1. - svs_index->setNumThreads(1); + svs_index->setParallelism(1); int deleted = 0; if (!this->backendIndex->isMultiValue()) { deleted = svs_index->deleteVector(label); @@ -1392,7 +1392,7 @@ class TieredSVSIndex : public VecSimTieredIndex { return; } // Force single thread for write-in-place mode. - this->GetSVSIndex()->setNumThreads(1); + this->GetSVSIndex()->setParallelism(1); // VecSimIndexAbstract::runGC() is protected static_cast(this->backendIndex)->runGC(); return; diff --git a/tests/benchmark/bm_utils.h b/tests/benchmark/bm_utils.h index 88f3e2e5d..e952794ad 100644 --- a/tests/benchmark/bm_utils.h +++ b/tests/benchmark/bm_utils.h @@ -30,9 +30,9 @@ CreateTieredSVSParams(VecSimParams &svs_params, tieredIndexMock &mock_thread_poo template static void verifyNumThreads(TieredSVSIndex *tiered_index, size_t expected_num_threads, size_t expected_capcity, std::string msg = "") { - ASSERT_EQ(tiered_index->GetSVSIndex()->getThreadPoolCapacity(), expected_capcity) + ASSERT_EQ(tiered_index->GetSVSIndex()->getPoolSize(), expected_capcity) << msg << ": thread pool capacity mismatch"; - size_t num_reserved_threads = tiered_index->GetSVSIndex()->getNumThreads(); + size_t num_reserved_threads = tiered_index->GetSVSIndex()->getParallelism(); if (num_reserved_threads < expected_num_threads) { std::cout << msg << ": WARNING: last reserved threads (" << num_reserved_threads << ") is less than expected (" << expected_num_threads << ")." << std::endl; diff --git a/tests/benchmark/bm_vecsim_svs.h b/tests/benchmark/bm_vecsim_svs.h index fe6c92493..c4b167421 100644 --- a/tests/benchmark/bm_vecsim_svs.h +++ b/tests/benchmark/bm_vecsim_svs.h @@ -115,7 +115,7 @@ class BM_VecSimSVS : public BM_VecSimGeneral { tiered_params.primaryIndexParams->algoParams.svsParams.num_threads; size_t num_threads = params_threadpool_size ? params_threadpool_size : mock_thread_pool.thread_pool_size; - tiered_index->GetSVSIndex()->setNumThreads(num_threads); + tiered_index->GetSVSIndex()->setParallelism(num_threads); test_utils::verifyNumThreads(tiered_index, num_threads, num_threads, std::string("CreateTieredSVSIndex")); diff --git a/tests/unit/test_svs_fp16.cpp b/tests/unit/test_svs_fp16.cpp index dc36615f6..f1746fa13 100644 --- a/tests/unit/test_svs_fp16.cpp +++ b/tests/unit/test_svs_fp16.cpp @@ -2242,8 +2242,8 @@ class FP16SVSTieredIndexTest : public FP16SVSTest { } void verifyNumThreads(TieredSVSIndex *tiered_index, size_t expected_num_threads, size_t expected_capcity) { - ASSERT_EQ(tiered_index->GetSVSIndex()->getNumThreads(), expected_num_threads); - ASSERT_EQ(tiered_index->GetSVSIndex()->getThreadPoolCapacity(), expected_capcity); + ASSERT_EQ(tiered_index->GetSVSIndex()->getParallelism(), expected_num_threads); + ASSERT_EQ(tiered_index->GetSVSIndex()->getPoolSize(), expected_capcity); } TieredSVSIndex *CreateTieredSVSIndex(const TieredIndexParams &tiered_params, @@ -2258,7 +2258,7 @@ class FP16SVSTieredIndexTest : public FP16SVSTest { // Set number of available threads to 1 unless specified otherwise, // so we can insert one vector at a time directly to svs. - tiered_index->GetSVSIndex()->setNumThreads(num_available_threads); + tiered_index->GetSVSIndex()->setParallelism(num_available_threads); size_t params_threadpool_size = tiered_params.primaryIndexParams->algoParams.svsParams.num_threads; size_t expected_capacity = diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index bbf655667..e701489d6 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -92,8 +92,8 @@ class SVSTieredIndexTest : public ::testing::Test { void verifyNumThreads(TieredSVSIndex *tiered_index, size_t expected_num_threads, size_t expected_capcity) { - ASSERT_EQ(tiered_index->GetSVSIndex()->getNumThreads(), expected_num_threads); - ASSERT_EQ(tiered_index->GetSVSIndex()->getThreadPoolCapacity(), expected_capcity); + ASSERT_EQ(tiered_index->GetSVSIndex()->getParallelism(), expected_num_threads); + ASSERT_EQ(tiered_index->GetSVSIndex()->getPoolSize(), expected_capcity); } TieredSVSIndex *CreateTieredSVSIndex(const TieredIndexParams &tiered_params, tieredIndexMock &mock_thread_pool, @@ -108,7 +108,7 @@ class SVSTieredIndexTest : public ::testing::Test { // which requires exactly 1 thread. When using tiered index addVector API, // the thread count is managed internally according to the operation and threadpool // capacity, so testing parallelism remains intact. - tiered_index->GetSVSIndex()->setNumThreads(num_available_threads); + tiered_index->GetSVSIndex()->setParallelism(num_available_threads); size_t params_threadpool_size = tiered_params.primaryIndexParams->algoParams.svsParams.num_threads; size_t expected_capacity =