From cb91c4cb8671fc90441d6f1748983375e769fbb8 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 10 Jan 2023 09:50:48 +0000 Subject: [PATCH 1/5] replacing ndoes mutex in one byte mutex --- src/VecSim/algorithms/hnsw/hnsw.h | 44 ++++++++++++++++--------------- src/VecSim/utils/vecsim_stl.h | 19 +++++++++++++ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw.h b/src/VecSim/algorithms/hnsw/hnsw.h index ba1880ebe..5e4482d0a 100644 --- a/src/VecSim/algorithms/hnsw/hnsw.h +++ b/src/VecSim/algorithms/hnsw/hnsw.h @@ -103,7 +103,7 @@ class HNSWIndex : public VecSimIndexAbstract, mutable VisitedNodesHandlerPool visited_nodes_handler_pool; mutable std::mutex entry_point_guard_; mutable std::mutex index_data_guard_; - mutable vecsim_stl::vector element_neighbors_locks_; + mutable vecsim_stl::vector element_neighbors_locks_; #ifdef BUILD_TESTS #include "VecSim/algorithms/hnsw/hnsw_base_tests_friends.h" @@ -159,11 +159,10 @@ class HNSWIndex : public VecSimIndexAbstract, const std::pair &neighbor_data, idType *new_node_neighbors_list, idType *neighbor_neighbors_list, - std::unique_lock &node_lock, - std::unique_lock &neighbor_lock); - inline idType mutuallyConnectNewElement(idType new_node_id, - candidatesMaxHeap &top_candidates, - size_t level); + std::unique_lock &node_lock, + std::unique_lock &neighbor_lock); + idType mutuallyConnectNewElement(idType new_node_id, + candidatesMaxHeap &top_candidates, size_t level); template void greedySearchLevel(const void *vector_data, size_t level, idType &curObj, DistType &curDist, void *timeoutCtx = nullptr, VecSimQueryResult_Code *rc = nullptr) const; @@ -521,7 +520,7 @@ DistType HNSWIndex::processCandidate( tag_t *elements_tags, vecsim_stl::abstract_priority_queue &top_candidates, candidatesMaxHeap &candidate_set, DistType lowerBound) const { - std::unique_lock lock(element_neighbors_locks_[curNodeId]); + std::unique_lock lock(element_neighbors_locks_[curNodeId]); idType *node_links = get_linklist_at_level(curNodeId, layer); linkListSize links_num = getListCount(node_links); @@ -573,7 +572,7 @@ void HNSWIndex::processCandidate_RangeSearch( tag_t *elements_tags, std::unique_ptr &results, candidatesMaxHeap &candidate_set, DistType dyn_range, double radius) const { - std::unique_lock lock(element_neighbors_locks_[curNodeId]); + std::unique_lock lock(element_neighbors_locks_[curNodeId]); idType *node_links = get_linklist_at_level(curNodeId, layer); linkListSize links_num = getListCount(node_links); @@ -703,7 +702,7 @@ template void HNSWIndex::revisitNeighborConnections( size_t level, idType new_node_id, const std::pair &neighbor_data, idType *new_node_neighbors_list, idType *neighbor_neighbors_list, - std::unique_lock &node_lock, std::unique_lock &neighbor_lock) { + std::unique_lock &node_lock, std::unique_lock &neighbor_lock) { // Note - expect that node_lock and neighbor_lock are locked at that point. // Collect the existing neighbors and the new node as the neighbor's neighbors candidates. @@ -760,9 +759,10 @@ void HNSWIndex::revisitNeighborConnections( std::sort(nodes_to_update.begin(), nodes_to_update.end()); size_t nodes_to_update_count = nodes_to_update.size(); - std::unique_lock locks[nodes_to_update_count]; + std::unique_lock locks[nodes_to_update_count]; for (size_t i = 0; i < nodes_to_update_count; i++) { - locks[i] = std::unique_lock(element_neighbors_locks_[nodes_to_update[i]]); + locks[i] = std::unique_lock( + element_neighbors_locks_[nodes_to_update[i]]); } auto *neighbour_incoming_edges = getIncomingEdgesPtr(selected_neighbor, level); @@ -855,17 +855,19 @@ idType HNSWIndex::mutuallyConnectNewElement( for (auto &neighbor_data : selected_neighbors) { idType selected_neighbor = neighbor_data.second; // neighbor's id - std::unique_lock node_lock; - std::unique_lock neighbor_lock; + std::unique_lock node_lock; + std::unique_lock neighbor_lock; idType lower_id = (new_node_id < selected_neighbor) ? new_node_id : selected_neighbor; if (lower_id == new_node_id) { - node_lock = std::unique_lock(element_neighbors_locks_[new_node_id]); - neighbor_lock = - std::unique_lock(element_neighbors_locks_[selected_neighbor]); + node_lock = + std::unique_lock(element_neighbors_locks_[new_node_id]); + neighbor_lock = std::unique_lock( + element_neighbors_locks_[selected_neighbor]); } else { - neighbor_lock = - std::unique_lock(element_neighbors_locks_[selected_neighbor]); - node_lock = std::unique_lock(element_neighbors_locks_[new_node_id]); + neighbor_lock = std::unique_lock( + element_neighbors_locks_[selected_neighbor]); + node_lock = + std::unique_lock(element_neighbors_locks_[new_node_id]); } // get the updated count - this may change between iterations due to releasing the lock. @@ -1124,7 +1126,7 @@ void HNSWIndex::greedySearchLevel(const void *vector_data, s return; } changed = false; - std::unique_lock lock(element_neighbors_locks_[curObj]); + std::unique_lock lock(element_neighbors_locks_[curObj]); idType *node_links = get_linklist(curObj, level); linkListSize links_count = getListCount(node_links); @@ -1150,7 +1152,7 @@ void HNSWIndex::resizeIndexInternal(size_t new_max_elements) element_levels_.shrink_to_fit(); resizeLabelLookup(new_max_elements); visited_nodes_handler_pool.resize(new_max_elements); - vecsim_stl::vector(new_max_elements, this->allocator) + vecsim_stl::vector(new_max_elements, this->allocator) .swap(element_neighbors_locks_); // Reallocate base layer char *data_level0_memory_new = (char *)this->allocator->reallocate( diff --git a/src/VecSim/utils/vecsim_stl.h b/src/VecSim/utils/vecsim_stl.h index c5380c36a..ed29e2da6 100644 --- a/src/VecSim/utils/vecsim_stl.h +++ b/src/VecSim/utils/vecsim_stl.h @@ -91,4 +91,23 @@ class unordered_set alloc) {} }; +struct one_byte_mutex { + void lock() { + if (state.exchange(locked, std::memory_order_acquire) == unlocked) + return; + while (state.exchange(sleeper, std::memory_order_acquire) != unlocked) + state.wait(sleeper, std::memory_order_relaxed); + } + void unlock() { + if (state.exchange(unlocked, std::memory_order_release) == sleeper) + state.notify_one(); + } + +private: + std::atomic state{unlocked}; + + static constexpr uint8_t unlocked = 0; + static constexpr uint8_t locked = 0b01; + static constexpr uint8_t sleeper = 0b10; +}; } // namespace vecsim_stl From 95cc727f44172e3313355316be7cd1a4e7964644 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 10 Jan 2023 10:31:02 +0000 Subject: [PATCH 2/5] changed mutex type in size estimation --- src/VecSim/algorithms/hnsw/hnsw_factory.cpp | 4 ++-- tests/unit/test_allocator.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_factory.cpp b/src/VecSim/algorithms/hnsw/hnsw_factory.cpp index acbe74753..cd7f02820 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_factory.cpp +++ b/src/VecSim/algorithms/hnsw/hnsw_factory.cpp @@ -63,7 +63,7 @@ size_t EstimateInitialSize(const HNSWParams *params) { est += sizeof(size_t) * params->initialCapacity + sizeof(size_t); // element level est += sizeof(size_t) * params->initialCapacity + sizeof(size_t); // Labels lookup hash table buckets. - est += sizeof(std::mutex) * params->initialCapacity + sizeof(size_t); // lock per vector + est += sizeof(vecsim_stl::one_byte_mutex) * params->initialCapacity + sizeof(size_t); // lock per vector } // Explicit allocation calls - always allocate a header. @@ -116,7 +116,7 @@ size_t EstimateElementSize(const HNSWParams *params) { // lookup hash map. size_t size_meta_data = sizeof(tag_t) + sizeof(size_t) + sizeof(size_t) + size_label_lookup_node; - size_t size_lock = sizeof(std::mutex); + size_t size_lock = sizeof(vecsim_stl::one_byte_mutex); /* Disclaimer: we are neglecting two additional factors that consume memory: * 1. The overall bucket size in labels_lookup hash table is usually higher than the number of diff --git a/tests/unit/test_allocator.cpp b/tests/unit/test_allocator.cpp index 324c94dd9..6db143b8f 100644 --- a/tests/unit/test_allocator.cpp +++ b/tests/unit/test_allocator.cpp @@ -389,7 +389,7 @@ TYPED_TEST(IndexAllocatorTest, test_hnsw_reclaim_memory) { // except for the bucket count of the labels_lookup hash table that is calculated separately. size_t size_total_data_per_element = hnswIndex->size_data_per_element_; expected_mem_delta += (sizeof(tag_t) + sizeof(void *) + sizeof(size_t) + - size_total_data_per_element + sizeof(std::mutex)) * + size_total_data_per_element + sizeof(vecsim_stl::one_byte_mutex)) * block_size; expected_mem_delta += (hnswIndex->label_lookup_.bucket_count() - prev_bucket_count) * sizeof(size_t); From 352a92d6e96888eed6eeb098ed4844ccde0a5648 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 10 Jan 2023 10:31:14 +0000 Subject: [PATCH 3/5] format --- src/VecSim/algorithms/hnsw/hnsw_factory.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_factory.cpp b/src/VecSim/algorithms/hnsw/hnsw_factory.cpp index cd7f02820..6cde72edb 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_factory.cpp +++ b/src/VecSim/algorithms/hnsw/hnsw_factory.cpp @@ -63,7 +63,8 @@ size_t EstimateInitialSize(const HNSWParams *params) { est += sizeof(size_t) * params->initialCapacity + sizeof(size_t); // element level est += sizeof(size_t) * params->initialCapacity + sizeof(size_t); // Labels lookup hash table buckets. - est += sizeof(vecsim_stl::one_byte_mutex) * params->initialCapacity + sizeof(size_t); // lock per vector + est += sizeof(vecsim_stl::one_byte_mutex) * params->initialCapacity + + sizeof(size_t); // lock per vector } // Explicit allocation calls - always allocate a header. From 3d7414c0a827f4e1150afc290b7a42b94207fef8 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 23 Jan 2023 08:19:28 +0000 Subject: [PATCH 4/5] format --- src/VecSim/algorithms/hnsw/hnsw.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw.h b/src/VecSim/algorithms/hnsw/hnsw.h index 5e4482d0a..a6867610a 100644 --- a/src/VecSim/algorithms/hnsw/hnsw.h +++ b/src/VecSim/algorithms/hnsw/hnsw.h @@ -702,7 +702,8 @@ template void HNSWIndex::revisitNeighborConnections( size_t level, idType new_node_id, const std::pair &neighbor_data, idType *new_node_neighbors_list, idType *neighbor_neighbors_list, - std::unique_lock &node_lock, std::unique_lock &neighbor_lock) { + std::unique_lock &node_lock, + std::unique_lock &neighbor_lock) { // Note - expect that node_lock and neighbor_lock are locked at that point. // Collect the existing neighbors and the new node as the neighbor's neighbors candidates. From 14d2d44853b2b490ae860f9e4c18ba02b154de5a Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 23 Jan 2023 09:39:14 +0000 Subject: [PATCH 5/5] chnaged machine ami --- .github/workflows/default.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/default.yml b/.github/workflows/default.yml index dd63a67d1..02137be11 100644 --- a/.github/workflows/default.yml +++ b/.github/workflows/default.yml @@ -48,8 +48,8 @@ jobs: with: mode: start github-token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }} - # Ubuntu 20.04 AMI - ec2-image-id: ami-0f4feb99425e13b50 + # Ubuntu 22.04 AMI + ec2-image-id: ami-09b2a1e33ce552e68 ec2-instance-type: t3.xlarge subnet-id: ${{ secrets.AWS_EC2_SUBNET_ID }} security-group-id: ${{ secrets.AWS_EC2_SG_ID }}