From 3a0e372bbfa2fa112c200f1d2d862a65b5c5c985 Mon Sep 17 00:00:00 2001 From: Brian McKinnon Date: Fri, 7 May 2021 21:42:03 -0500 Subject: [PATCH 1/2] Add if_contains_unsafe function that does not lock the parallel hash map mutex Add find_as_pair function, since make_iterator in find was very expensive. With an equal number of read and write operations in a sample program, more time was being spent in the read then write. --- parallel_hashmap/phmap.h | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index f897e28..e37774b 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -3376,12 +3376,18 @@ private: protected: template - iterator find(const key_arg& key, size_t hashval, L &mutexlock) { + std::pair find_as_pair(const key_arg& key, size_t hashval, L& mutexlock) + { Inner& inner = sets_[subidx(hashval)]; - auto& set = inner.set_; + auto& set = inner.set_; mutexlock = std::move(L(inner)); - auto it = set.find(key, hashval); - return make_iterator(&inner, it); + return std::make_pair(&inner, set.find(key, hashval)); + } + + template + iterator find(const key_arg& key, size_t hashval, L& mutexlock) { + auto res = find_as_pair(key, hashval, mutexlock); + return make_iterator(res.first, res.second); } template @@ -3603,7 +3609,7 @@ public: template iterator try_emplace_with_hash(size_t hashval, const_iterator, const key_arg& k, Args&&... args) { return try_emplace_with_hash(hashval, k, std::forward(args)...).first; - } + } // if map contains key, lambda is called with the mapped value (under read lock protection), // and if_contains returns true. This is a const API and lambda should not modify the value @@ -3614,6 +3620,15 @@ public: modify_if_impl(key, std::forward(f)); } + // if map contains key, lambda is called with the mapped value without read lock protection, + // and if_contains_unsafe returns true. This is a const API and lambda should not modify the value + // ----------------------------------------------------------------------------------------- + template + bool if_contains_unsafe(const key_arg& key, F&& f) const { + return const_cast(this)->template + modify_if_impl::DoNothing>(key, std::forward(f)); + } + // if map contains key, lambda is called with the mapped value (under write lock protection), // and modify_if returns true. This is a non-const API and lambda is allowed to modify the mapped value // ---------------------------------------------------------------------------------------------------- @@ -3674,10 +3689,10 @@ private: static_assert(std::is_invocable::value); #endif L m; - auto it = this->template find(key, this->hash(key), m); - if (it == this->end()) + auto res = this->template find_as_pair(key, this->hash(key), m); + if (res.second == res.first->set_.end()) return false; - std::forward(f)(Policy::value(&*it)); + std::forward(f)(Policy::value(&*res.second)); return true; } From b4efc91e7f735304f47696ffb52c3ebfdaa5c0cd Mon Sep 17 00:00:00 2001 From: Gregory Popovitch Date: Sat, 8 May 2021 07:07:50 -0400 Subject: [PATCH 2/2] use `find_as_pair` in `erase_if_impl` as well. --- parallel_hashmap/phmap.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index e37774b..c725f87 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -3622,6 +3622,7 @@ public: // if map contains key, lambda is called with the mapped value without read lock protection, // and if_contains_unsafe returns true. This is a const API and lambda should not modify the value + // This should be used only if we know that no other thread may be mutating the map at the time. // ----------------------------------------------------------------------------------------- template bool if_contains_unsafe(const key_arg& key, F&& f) const { @@ -3702,12 +3703,11 @@ private: static_assert(std::is_invocable::value); #endif L m; - auto it = this->template find(key, this->hash(key), m); - if (it == this->end()) - return false; - if (std::forward(f)(Policy::value(&*it))) + auto res = this->template find_as_pair(key, this->hash(key), m); + if (res.second != res.first->set_.end() && + std::forward(f)(Policy::value(&*res.second))) { - this->erase(it); + res.first->set_.erase(res.second); return true; } return false;