From fbd4dc6ca0d4125014be7362f0d5b808c41525f1 Mon Sep 17 00:00:00 2001 From: Ijtihed Kilani Date: Fri, 19 Jun 2026 20:41:33 +0300 Subject: [PATCH] flann: use per-thread storage for heap pool to remove lock contention --- .../flann/hierarchical_clustering_index.h | 6 +- .../include/opencv2/flann/kdtree_index.h | 6 +- .../include/opencv2/flann/kmeans_index.h | 6 +- modules/flann/test/test_heap.cpp | 100 ++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 modules/flann/test/test_heap.cpp diff --git a/modules/flann/include/opencv2/flann/hierarchical_clustering_index.h b/modules/flann/include/opencv2/flann/hierarchical_clustering_index.h index 56ae38e8c1..979c771960 100644 --- a/modules/flann/include/opencv2/flann/hierarchical_clustering_index.h +++ b/modules/flann/include/opencv2/flann/hierarchical_clustering_index.h @@ -532,7 +532,11 @@ public: const bool explore_all_trees = get_param(searchParams,"explore_all_trees",false); // Priority queue storing intermediate branches in the best-bin-first search - const cv::Ptr>& heap = Heap::getPooledInstance(cv::utils::getThreadID(), (int)size_); + // Kept in thread_local storage so each thread owns an independent heap + // and no process-wide lock is taken on the search hot path (issue #25281). + thread_local cv::Ptr> heap = cv::makePtr>((int)size_); + heap->clear(); + heap->reserve((int)size_); std::vector checked(size_,false); int checks = 0; diff --git a/modules/flann/include/opencv2/flann/kdtree_index.h b/modules/flann/include/opencv2/flann/kdtree_index.h index 569844e475..a279aeae77 100644 --- a/modules/flann/include/opencv2/flann/kdtree_index.h +++ b/modules/flann/include/opencv2/flann/kdtree_index.h @@ -449,7 +449,11 @@ private: DynamicBitset checked(size_); // Priority queue storing intermediate branches in the best-bin-first search - const cv::Ptr>& heap = Heap::getPooledInstance(cv::utils::getThreadID(), (int)size_); + // Kept in thread_local storage so each thread owns an independent heap + // and no process-wide lock is taken on the search hot path (issue #25281). + thread_local cv::Ptr> heap = cv::makePtr>((int)size_); + heap->clear(); + heap->reserve((int)size_); /* Search once through each tree down to root. */ for (i = 0; i < trees_; ++i) { diff --git a/modules/flann/include/opencv2/flann/kmeans_index.h b/modules/flann/include/opencv2/flann/kmeans_index.h index b079b9f5e5..7270508c52 100644 --- a/modules/flann/include/opencv2/flann/kmeans_index.h +++ b/modules/flann/include/opencv2/flann/kmeans_index.h @@ -528,7 +528,11 @@ public: } else { // Priority queue storing intermediate branches in the best-bin-first search - const cv::Ptr>& heap = Heap::getPooledInstance(cv::utils::getThreadID(), (int)size_); + // Kept in thread_local storage so each thread owns an independent heap + // and no process-wide lock is taken on the search hot path (issue #25281). + thread_local cv::Ptr> heap = cv::makePtr>((int)size_); + heap->clear(); + heap->reserve((int)size_); int checks = 0; for (int i=0; i + +#if !defined(OPENCV_DISABLE_THREAD_SUPPORT) +#include +#endif + +namespace opencv_test { namespace { + +using cvflann::Heap; + +// Basic single-threaded behaviour: popMin returns the stored elements in +// ascending order and reports emptiness correctly. +TEST(Flann_Heap, popMin_returns_sorted) +{ + const int capacity = 8; + Heap heap(capacity); + + const int values[] = {5, 1, 4, 2, 8, 3, 7, 6}; + for (int v : values) + heap.insert(v); + + EXPECT_EQ(heap.size(), capacity); + + int prev = -1, out = 0; + int popped = 0; + while (heap.popMin(out)) + { + EXPECT_GT(out, prev); // strictly increasing + prev = out; + ++popped; + } + EXPECT_EQ(popped, capacity); + EXPECT_TRUE(heap.empty()); +} + +// Capacity is a hard limit: extra inserts are dropped, not stored. +TEST(Flann_Heap, insert_respects_capacity) +{ + const int capacity = 3; + Heap heap(capacity); + for (int i = 0; i < 10; ++i) + heap.insert(i); + EXPECT_EQ(heap.size(), capacity); +} + +#if !defined(OPENCV_DISABLE_THREAD_SUPPORT) + +// getPooledInstance() is no longer used on the FLANN search hot path, but it +// remains public API, so keep it covered: many threads each key the shared +// pool with their own thread id and must get usable, correctly ordered heaps +// without tripping the internal use_count guard. +TEST(Flann_Heap, getPooledInstance_concurrent_usage) +{ + const int numThreads = 8; + const int capacity = 32; + std::vector threads; + std::vector ok(numThreads, 0); + + for (int t = 0; t < numThreads; ++t) + { + threads.emplace_back([&, t]() + { + bool good = true; + for (int iter = 0; iter < 200 && good; ++iter) + { + cv::Ptr> heap = + Heap::getPooledInstance(cv::utils::getThreadID(), capacity); + for (int v = capacity - 1; v >= 0; --v) + heap->insert(v); + + int prev = -1, out = 0, count = 0; + while (heap->popMin(out)) + { + if (out <= prev) { good = false; break; } + prev = out; + ++count; + } + if (count != capacity) good = false; + } + ok[t] = good ? 1 : 0; + }); + } + for (auto& th : threads) + th.join(); + + for (int t = 0; t < numThreads; ++t) + EXPECT_EQ((int)ok[t], 1) << "thread " << t << " produced incorrect heap results"; +} + +#endif // !OPENCV_DISABLE_THREAD_SUPPORT + +}} // namespace