From e3d30ccf6b0fd73bbb90b9fe290fae51ce611b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B0=B4=E8=8A=9D?= Date: Mon, 8 Jun 2026 10:54:35 +0800 Subject: [PATCH 1/3] perf(hgraph): skip mutex acquisition on immutable index search paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an HGraph index is marked immutable via SetImmutable(), no writer (Add/Build/Remove/Tune) can modify shared state. The search paths previously acquired global_mutex_ and force_remove_mutex_ unconditionally, causing unnecessary atomic CAS cache-line contention across all search threads on read-only indexes. This change skips both locks when immutable_ is true, eliminating the contention on high-concurrency search workloads. Additionally fixes two pre-existing correctness issues: - immutable_ was a plain bool read without synchronization from search threads while written under exclusive lock by SetImmutable(). Changed to std::atomic with acquire/release semantics for ARM safety. - SetImmutable() now acquires add_mutex_ before global_mutex_ to prevent a TOCTOU race where Tune() could pass CHECK_IMMUTABLE_INDEX, then SetImmutable() completes, then search skips the lock while Tune() modifies basic_flatten_codes_. Signed-off-by: 水芝 Assisted-by: Claude:claude-opus-4-6 --- src/algorithm/hgraph/hgraph.cpp | 5 +++-- src/algorithm/hgraph/hgraph_search.cpp | 27 +++++++++++++++++--------- src/algorithm/inner_index_interface.h | 2 +- src/algorithm/sindi/sindi.cpp | 2 +- src/index/index_impl.h | 8 ++++---- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/algorithm/hgraph/hgraph.cpp b/src/algorithm/hgraph/hgraph.cpp index ca37b3702..c9a90f927 100644 --- a/src/algorithm/hgraph/hgraph.cpp +++ b/src/algorithm/hgraph/hgraph.cpp @@ -433,14 +433,15 @@ HGraph::GetVectorByInnerId(InnerIdType inner_id, float* data) const { void HGraph::SetImmutable() { - if (this->immutable_) { + if (this->immutable_.load(std::memory_order_acquire)) { return; } + std::scoped_lock add_lock(this->add_mutex_); std::scoped_lock wlock(this->global_mutex_); this->neighbors_mutex_.reset(); this->neighbors_mutex_ = std::make_shared(); this->searcher_->SetMutexArray(this->neighbors_mutex_); - this->immutable_ = true; + this->immutable_.store(true, std::memory_order_release); } void diff --git a/src/algorithm/hgraph/hgraph_search.cpp b/src/algorithm/hgraph/hgraph_search.cpp index 9d23c374a..019a53d1a 100644 --- a/src/algorithm/hgraph/hgraph_search.cpp +++ b/src/algorithm/hgraph/hgraph_search.cpp @@ -82,10 +82,13 @@ HGraph::KnnSearch(const DatasetPtr& query, fmt::format("ef_search({}) must in range[1, {}]", params.ef_search, ef_search_threshold)); std::shared_lock force_remove_rlock; - if (this->support_force_remove()) { - force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + std::shared_lock shared_lock; + if (!this->immutable_.load(std::memory_order_acquire)) { + if (this->support_force_remove()) { + force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + } + shared_lock = std::shared_lock(this->global_mutex_); } - std::shared_lock shared_lock(this->global_mutex_); k = std::min(k, GetNumElements()); FilterPtr ft = this->create_search_filter(filter, params.use_extra_info_filter); @@ -357,10 +360,13 @@ HGraph::RangeSearch(const DatasetPtr& query, this->validate_range_args(query, radius, limited_size); std::shared_lock force_remove_rlock; - if (this->support_force_remove()) { - force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + std::shared_lock shared_lock; + if (!this->immutable_.load(std::memory_order_acquire)) { + if (this->support_force_remove()) { + force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + } + shared_lock = std::shared_lock(this->global_mutex_); } - std::shared_lock shared_lock(this->global_mutex_); InnerSearchParam search_param; search_param.ep = this->entry_point_id_; @@ -455,10 +461,13 @@ HGraph::SearchWithRequest(const SearchRequest& request) const { fmt::format("ef_search({}) must in range[1, {}]", params.ef_search, ef_search_threshold)); std::shared_lock force_remove_rlock; - if (this->support_force_remove()) { - force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + std::shared_lock shared_lock; + if (!this->immutable_.load(std::memory_order_acquire)) { + if (this->support_force_remove()) { + force_remove_rlock = std::shared_lock(this->force_remove_mutex_); + } + shared_lock = std::shared_lock(this->global_mutex_); } - std::shared_lock shared_lock(this->global_mutex_); k = std::min(k, GetNumElements()); // Setup reasoning context if expected labels are provided. diff --git a/src/algorithm/inner_index_interface.h b/src/algorithm/inner_index_interface.h index 74901004f..a74783e45 100644 --- a/src/algorithm/inner_index_interface.h +++ b/src/algorithm/inner_index_interface.h @@ -567,7 +567,7 @@ class InnerIndexInterface { IndexFeatureListUPtr index_feature_list_{nullptr}; const InnerIndexParameterPtr create_param_ptr_{nullptr}; - bool immutable_{false}; + std::atomic immutable_{false}; protected: std::atomic current_memory_usage_{0}; diff --git a/src/algorithm/sindi/sindi.cpp b/src/algorithm/sindi/sindi.cpp index 32ff8f0de..e1dea85ad 100644 --- a/src/algorithm/sindi/sindi.cpp +++ b/src/algorithm/sindi/sindi.cpp @@ -709,7 +709,7 @@ SINDI::CalDistanceById(const DatasetPtr& query, void SINDI::SetImmutable() { std::scoped_lock wlock(this->global_mutex_); - this->immutable_ = true; + this->immutable_.store(true, std::memory_order_release); } void diff --git a/src/index/index_impl.h b/src/index/index_impl.h index 007623c94..93cab5123 100644 --- a/src/index/index_impl.h +++ b/src/index/index_impl.h @@ -57,10 +57,10 @@ class IndexImpl : public Index { if ((query)->GetNumElements() == 0) { \ return DatasetImpl::MakeEmptyDataset(); \ } -#define CHECK_IMMUTABLE_INDEX(operation_str) \ - if (this->inner_index_->immutable_) { \ - return tl::unexpected(Error(ErrorType::UNSUPPORTED_INDEX_OPERATION, \ - "immutable index no support " operation_str)); \ +#define CHECK_IMMUTABLE_INDEX(operation_str) \ + if (this->inner_index_->immutable_.load(std::memory_order_acquire)) { \ + return tl::unexpected(Error(ErrorType::UNSUPPORTED_INDEX_OPERATION, \ + "immutable index no support " operation_str)); \ } #define CHECK_NONEMPTY_DATASET(dataset) \ From f895451b87a3d19a9046b52ab798603df43497f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B0=B4=E8=8A=9D?= Date: Mon, 8 Jun 2026 11:08:56 +0800 Subject: [PATCH 2/3] fix(hgraph): re-check immutable_ in Tune() after acquiring add_mutex_ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents a TOCTOU race where Tune() passes CHECK_IMMUTABLE_INDEX before SetImmutable() runs, then proceeds to modify basic_flatten_codes_ after SetImmutable() completes, while search threads have already stopped acquiring global_mutex_. Signed-off-by: 水芝 Assisted-by: Claude:claude-opus-4-6 --- src/algorithm/hgraph/hgraph.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/algorithm/hgraph/hgraph.cpp b/src/algorithm/hgraph/hgraph.cpp index c9a90f927..b3897c12c 100644 --- a/src/algorithm/hgraph/hgraph.cpp +++ b/src/algorithm/hgraph/hgraph.cpp @@ -134,6 +134,9 @@ HGraph::Tune(const std::string& parameters, bool disable_future_tuning) { } std::scoped_lock lock(this->add_mutex_); + if (this->immutable_.load(std::memory_order_acquire)) { + return false; + } // check which code need to tune and update create_param_ptr_ bool is_tune_base_code = false; From 42568945e07f8b2544cb0abdccb5f1a80016009c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B0=B4=E8=8A=9D?= Date: Mon, 8 Jun 2026 20:21:40 +0800 Subject: [PATCH 3/3] style: apply clang-format-15 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 水芝 Assisted-by: Claude:claude-opus-4-6 --- src/index/index_impl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/index/index_impl.h b/src/index/index_impl.h index 93cab5123..4d270908b 100644 --- a/src/index/index_impl.h +++ b/src/index/index_impl.h @@ -57,10 +57,10 @@ class IndexImpl : public Index { if ((query)->GetNumElements() == 0) { \ return DatasetImpl::MakeEmptyDataset(); \ } -#define CHECK_IMMUTABLE_INDEX(operation_str) \ - if (this->inner_index_->immutable_.load(std::memory_order_acquire)) { \ - return tl::unexpected(Error(ErrorType::UNSUPPORTED_INDEX_OPERATION, \ - "immutable index no support " operation_str)); \ +#define CHECK_IMMUTABLE_INDEX(operation_str) \ + if (this->inner_index_->immutable_.load(std::memory_order_acquire)) { \ + return tl::unexpected(Error(ErrorType::UNSUPPORTED_INDEX_OPERATION, \ + "immutable index no support " operation_str)); \ } #define CHECK_NONEMPTY_DATASET(dataset) \