From 21f14d7a8e8b9fd1823cfd55f59c39001c17b2ec Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Sun, 12 Nov 2023 18:49:11 -0500 Subject: [PATCH 1/2] Add support for read/write locks --- parallel_hashmap/phmap.h | 99 ++++++++++++++++++++------------- parallel_hashmap/phmap_base.h | 102 ++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 38 deletions(-) diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index 611704f..656268a 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -1547,12 +1547,13 @@ class raw_hash_set template iterator lazy_emplace_with_hash(const key_arg& key, size_t hashval, F&& f) { - auto res = find_or_prepare_insert(key, hashval); - if (res.second) { - lazy_emplace_at(res.first, std::forward(f)); - this->set_ctrl(res.first, H2(hashval)); + size_t offset = _find_key(key, hashval); + if (offset == (size_t)-1) { + offset = prepare_insert(hashval); + lazy_emplace_at(offset, std::forward(f)); + this->set_ctrl(offset, H2(hashval)); } - return iterator_at(res.first); + return iterator_at(offset); } template @@ -1564,12 +1565,13 @@ class raw_hash_set template void emplace_single_with_hash(const key_arg& key, size_t hashval, F&& f) { - auto res = find_or_prepare_insert(key, hashval); - if (res.second) { - lazy_emplace_at(res.first, std::forward(f)); - this->set_ctrl(res.first, H2(hashval)); + size_t offset = _find_key(key, hashval); + if (offset == (size_t)-1) { + offset = prepare_insert(hashval); + lazy_emplace_at(offset, std::forward(f)); + this->set_ctrl(offset, H2(hashval)); } else - _erase(iterator_at(res.first)); + _erase(iterator_at(offset)); } @@ -1897,12 +1899,14 @@ class raw_hash_set std::pair emplace_decomposable(const K& key, size_t hashval, Args&&... args) { - auto res = find_or_prepare_insert(key, hashval); - if (res.second) { - emplace_at(res.first, std::forward(args)...); - this->set_ctrl(res.first, H2(hashval)); + size_t offset = _find_key(key, hashval); + if (offset == (size_t)-1) { + offset = prepare_insert(hashval); + emplace_at(offset, std::forward(args)...); + this->set_ctrl(offset, H2(hashval)); + return {iterator_at(offset), true}; } - return {iterator_at(res.first), res.second}; + return {iterator_at(offset), false}; } struct EmplaceDecomposable @@ -2194,7 +2198,7 @@ class raw_hash_set protected: template - std::pair find_or_prepare_insert(const K& key, size_t hashval) { + size_t _find_key(const K& key, size_t hashval) { auto seq = probe(hashval); while (true) { Group g{ctrl_ + seq.offset()}; @@ -2202,12 +2206,21 @@ class raw_hash_set if (PHMAP_PREDICT_TRUE(PolicyTraits::apply( EqualElement{key, eq_ref()}, PolicyTraits::element(slots_ + seq.offset((size_t)i))))) - return {seq.offset((size_t)i), false}; + return seq.offset((size_t)i); } if (PHMAP_PREDICT_TRUE(g.MatchEmpty())) break; seq.next(); } - return {prepare_insert(hashval), true}; + return (size_t)-1; + } + + + template + std::pair find_or_prepare_insert(const K& key, size_t hashval) { + size_t offset = _find_key(key, hashval); + if (offset == (size_t)-1) + return {prepare_insert(hashval), true}; + return {offset, false}; } size_t prepare_insert(size_t hashval) PHMAP_ATTRIBUTE_NOINLINE { @@ -2479,26 +2492,30 @@ class raw_hash_map : public raw_hash_set template std::pair insert_or_assign_impl(K&& k, V&& v) { size_t hashval = this->hash(k); - auto res = this->find_or_prepare_insert(k, hashval); - if (res.second) { - this->emplace_at(res.first, std::forward(k), std::forward(v)); - this->set_ctrl(res.first, H2(hashval)); - } else - Policy::value(&*this->iterator_at(res.first)) = std::forward(v); - return {this->iterator_at(res.first), res.second}; + size_t offset = this->_find_key(k, hashval); + if (offset == (size_t)-1) { + offset = this->prepare_insert(hashval); + this->emplace_at(offset, std::forward(k), std::forward(v)); + this->set_ctrl(offset, H2(hashval)); + return {this->iterator_at(offset), true}; + } + Policy::value(&*this->iterator_at(offset)) = std::forward(v); + return {this->iterator_at(offset), false}; } template std::pair try_emplace_impl(K&& k, Args&&... args) { size_t hashval = this->hash(k); - auto res = this->find_or_prepare_insert(k, hashval); - if (res.second) { - this->emplace_at(res.first, std::piecewise_construct, + size_t offset = this->_find_key(k, hashval); + if (offset == (size_t)-1) { + offset = this->prepare_insert(hashval); + this->emplace_at(offset, std::piecewise_construct, std::forward_as_tuple(std::forward(k)), std::forward_as_tuple(std::forward(args)...)); - this->set_ctrl(res.first, H2(hashval)); + this->set_ctrl(offset, H2(hashval)); + return {this->iterator_at(offset), true}; } - return {this->iterator_at(res.first), res.second}; + return {this->iterator_at(offset), false}; } }; @@ -2565,6 +2582,7 @@ class parallel_hash_set protected: using Lockable = phmap::LockableImpl; + // -------------------------------------------------------------------- struct Inner : public Lockable @@ -3324,7 +3342,7 @@ class parallel_hash_set template bool lazy_emplace_l(const key_arg& key, FExists&& fExists, FEmplace&& fEmplace) { size_t hashval = this->hash(key); - typename Lockable::UniqueLock m; + typename Lockable::ReadWriteLock m; auto res = this->find_or_prepare_insert_with_hash(hashval, key, m); Inner* inner = std::get<0>(res); if (std::get<2>(res)) { @@ -3781,17 +3799,22 @@ class parallel_hash_set template std::tuple - find_or_prepare_insert_with_hash(size_t hashval, const K& key, typename Lockable::UniqueLock &mutexlock) { + find_or_prepare_insert_with_hash(size_t hashval, const K& key, typename Lockable::ReadWriteLock &mutexlock) { Inner& inner = sets_[subidx(hashval)]; auto& set = inner.set_; - mutexlock = std::move(typename Lockable::UniqueLock(inner)); - auto p = set.find_or_prepare_insert(key, hashval); // std::pair - return std::make_tuple(&inner, p.first, p.second); + mutexlock = std::move(typename Lockable::ReadWriteLock(inner)); + size_t offset = set._find_key(key, hashval); + if (offset == (size_t)-1) { + mutexlock.switch_to_unique(); + offset = set.prepare_insert(hashval); + return std::make_tuple(&inner, offset, true); + } + return std::make_tuple(&inner, offset, false); } template std::tuple - find_or_prepare_insert(const K& key, typename Lockable::UniqueLock &mutexlock) { + find_or_prepare_insert(const K& key, typename Lockable::ReadWriteLock &mutexlock) { return find_or_prepare_insert_with_hash(this->hash(key), key, mutexlock); } @@ -4060,7 +4083,7 @@ class parallel_hash_map : public parallel_hash_set std::pair insert_or_assign_impl(K&& k, V&& v) { size_t hashval = this->hash(k); - typename Lockable::UniqueLock m; + typename Lockable::ReadWriteLock m; auto res = this->find_or_prepare_insert_with_hash(hashval, k, m); typename Base::Inner *inner = std::get<0>(res); if (std::get<2>(res)) { @@ -4080,7 +4103,7 @@ class parallel_hash_map : public parallel_hash_set std::pair try_emplace_impl_with_hash(size_t hashval, K&& k, Args&&... args) { - typename Lockable::UniqueLock m; + typename Lockable::ReadWriteLock m; auto res = this->find_or_prepare_insert_with_hash(hashval, k, m); typename Base::Inner *inner = std::get<0>(res); if (std::get<2>(res)) { diff --git a/parallel_hashmap/phmap_base.h b/parallel_hashmap/phmap_base.h index f1134b3..b471481 100644 --- a/parallel_hashmap/phmap_base.h +++ b/parallel_hashmap/phmap_base.h @@ -4631,6 +4631,11 @@ class LockableBaseImpl DoNothing& operator=(DoNothing&&) noexcept { return *this; } void swap(DoNothing &) {} bool owns_lock() const noexcept { return true; } + void lock() {} + void unlock() {} + void lock_shared() {} + void unlock_shared() {} + void switch_to_unique() {} }; // ---------------------------------------------------- @@ -4705,6 +4710,8 @@ class LockableBaseImpl } mutex_type *mutex() const noexcept { return m_; } + + void switch_to_unique() {} private: mutex_type *m_; @@ -4784,8 +4791,98 @@ class LockableBaseImpl mutex_type *mutex() const noexcept { return m_; } + void switch_to_unique() {} + + private: + mutex_type *m_; + bool locked_; + }; + + // ---------------------------------------------------- + class ReadWriteLock + { + public: + using mutex_type = MutexType; + + ReadWriteLock() : m_(nullptr), locked_(false), locked_shared_(false) {} + + explicit ReadWriteLock(mutex_type &m) : m_(&m), locked_(false), locked_shared_(true) { + m_->lock_shared(); + } + + ReadWriteLock(mutex_type& m, defer_lock_t) noexcept : + m_(&m), locked_(false), locked_shared_(false) + {} + + ReadWriteLock(ReadWriteLock &&o) noexcept : + m_(std::move(o.m_)), locked_(o.locked_), locked_shared_(o.locked_shared_) { + o.locked_ = false; + o.locked_shared_ = false; + o.m_ = nullptr; + } + + ReadWriteLock& operator=(ReadWriteLock&& other) noexcept { + ReadWriteLock temp(std::move(other)); + swap(temp); + return *this; + } + + ~ReadWriteLock() { + if (locked_shared_) + m_->unlock_shared(); + else if (locked_) + m_->unlock(); + } + + void lock_shared() { + if (!locked_shared_) { + m_->lock_shared(); + locked_shared_ = true; + } + } + + void unlock_shared() { + if (locked_shared_) { + m_->unlock_shared(); + locked_shared_ = false; + } + } + + void lock() { + if (!locked_) { + m_->lock(); + locked_ = true; + } + } + + void unlock() { + if (locked_) { + m_->unlock(); + locked_ = false; + } + } + + bool owns_lock() const noexcept { return locked_; } + bool owns_shared_lock() const noexcept { return locked_shared_; } + + void swap(ReadWriteLock &o) noexcept { + std::swap(m_, o.m_); + std::swap(locked_, o.locked_); + std::swap(locked_shared_, o.locked_shared_); + } + + mutex_type *mutex() const noexcept { return m_; } + + void switch_to_unique() { + assert(locked_shared_); + unlock_shared(); + lock(); + } + + private: mutex_type *m_; + bool locked_shared_; bool locked_; }; @@ -4877,6 +4974,7 @@ class LockableImpl : public Mtx_ using SharedLock = typename Base::WriteLock; using UpgradeLock = typename Base::WriteLock; using UniqueLock = typename Base::WriteLock; + using ReadWriteLock = typename Base::WriteLock; using SharedLocks = typename Base::WriteLocks; using UniqueLocks = typename Base::WriteLocks; using UpgradeToUnique = typename Base::DoNothing; // we already have unique ownership @@ -4893,6 +4991,7 @@ class LockableImpl: public phmap::NullMutex using Base = LockableBaseImpl; using SharedLock = typename Base::DoNothing; using UpgradeLock = typename Base::DoNothing; + using ReadWriteLock = typename Base::DoNothing; using UniqueLock = typename Base::DoNothing; using UpgradeToUnique = typename Base::DoNothing; using SharedLocks = typename Base::DoNothing; @@ -4921,6 +5020,7 @@ class LockableImpl: public phmap::NullMutex using mutex_type = phmap::AbslMutex; using Base = LockableBaseImpl; using SharedLock = typename Base::ReadLock; + using ReadWriteLock = typename Base::ReadWriteLock; using UpgradeLock = typename Base::WriteLock; using UniqueLock = typename Base::WriteLock; using SharedLocks = typename Base::ReadLocks; @@ -4943,6 +5043,7 @@ class LockableImpl: public phmap::NullMutex using mutex_type = boost::shared_mutex; using Base = LockableBaseImpl; using SharedLock = boost::shared_lock; + using ReadWriteLock = typename Base::ReadWriteLock; using UpgradeLock = boost::unique_lock; // assume can't upgrade using UniqueLock = boost::unique_lock; using SharedLocks = typename Base::ReadLocks; @@ -4965,6 +5066,7 @@ class LockableImpl: public phmap::NullMutex using mutex_type = std::shared_mutex; using Base = LockableBaseImpl; using SharedLock = std::shared_lock; + using ReadWriteLock = typename Base::ReadWriteLock; using UpgradeLock = std::unique_lock; // assume can't upgrade using UniqueLock = std::unique_lock; using SharedLocks = typename Base::ReadLocks; From ff7bd78206e749f50c15f2ab612da94986a050e5 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 13 Nov 2023 09:48:49 -0500 Subject: [PATCH 2/2] Take care of the case where another thread inserts the same key between `unlock()/lock()` --- parallel_hashmap/phmap.h | 15 ++++++++++++--- parallel_hashmap/phmap_base.h | 9 +++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index 656268a..344cb0b 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -3805,9 +3805,18 @@ class parallel_hash_set mutexlock = std::move(typename Lockable::ReadWriteLock(inner)); size_t offset = set._find_key(key, hashval); if (offset == (size_t)-1) { - mutexlock.switch_to_unique(); - offset = set.prepare_insert(hashval); - return std::make_tuple(&inner, offset, true); + if (mutexlock.switch_to_unique()) { + // we did an unlock/lock, and another thread could have inserted the same key, so we need to + // do a find() again. + offset = set._find_key(key, hashval); + if (offset == (size_t)-1) { + offset = set.prepare_insert(hashval); + return std::make_tuple(&inner, offset, true); + } + } else { + offset = set.prepare_insert(hashval); + return std::make_tuple(&inner, offset, true); + } } return std::make_tuple(&inner, offset, false); } diff --git a/parallel_hashmap/phmap_base.h b/parallel_hashmap/phmap_base.h index b471481..d3c2245 100644 --- a/parallel_hashmap/phmap_base.h +++ b/parallel_hashmap/phmap_base.h @@ -4635,7 +4635,7 @@ class LockableBaseImpl void unlock() {} void lock_shared() {} void unlock_shared() {} - void switch_to_unique() {} + bool switch_to_unique() { return false; } }; // ---------------------------------------------------- @@ -4711,7 +4711,7 @@ class LockableBaseImpl mutex_type *mutex() const noexcept { return m_; } - void switch_to_unique() {} + bool switch_to_unique() { return false; } private: mutex_type *m_; @@ -4791,7 +4791,7 @@ class LockableBaseImpl mutex_type *mutex() const noexcept { return m_; } - void switch_to_unique() {} + bool switch_to_unique() { return false; } private: mutex_type *m_; @@ -4873,10 +4873,11 @@ class LockableBaseImpl mutex_type *mutex() const noexcept { return m_; } - void switch_to_unique() { + bool switch_to_unique() { assert(locked_shared_); unlock_shared(); lock(); + return true; }