diff --git a/README.md b/README.md index b6c4576..2f38906 100644 --- a/README.md +++ b/README.md @@ -287,7 +287,7 @@ Parallel Hashmap containers follow the thread safety rules of the Standard C++ l - It is safe to read and write to one instance of a type even if another thread is reading or writing to a different instance of the same type. For example, given hash tables A and B of the same type, it is safe if A is being written in thread 1 and B is being read in thread 2. -- The *parallel* tables can be made internally thread-safe for concurrent write access, by providing a synchronization type (for example [std::mutex](https://en.cppreference.com/w/cpp/thread/mutex)) as the last template argument. Because locking is performed at the *submap* level, a high level of concurrency can still be achieved. However please be aware that returned iterators are not protected by the mutex, so they cannot be used reliably on a hash map which can be changed by another thread. Again, the internal synchronization does not allow to retrieve data from a table that is being modified in another thread. +- The *parallel* tables can be made internally thread-safe for concurrent read and write access, by providing a synchronization type (for example [std::mutex](https://en.cppreference.com/w/cpp/thread/mutex)) as the last template argument. Because locking is performed at the *submap* level, a high level of concurrency can still be achieved. Read access can be done safely using `if_contains()`, which passes a reference value to the callback while holding the *submap* lock. However, please be aware that returned iterators are not protected by the mutex, so they cannot be used reliably on a hash map which can be changed by another thread. - Examples on how to use various mutex types, including boost::mutex, boost::shared_mutex and absl::Mutex can be found in `examples/bench.cc` diff --git a/parallel_hashmap/btree.h b/parallel_hashmap/btree.h index 1e6ea36..74eaf1e 100644 --- a/parallel_hashmap/btree.h +++ b/parallel_hashmap/btree.h @@ -712,8 +712,8 @@ namespace phmap { template < typename Compare, typename K, typename LK, - phmap::enable_if_t>::value, + phmap::enable_if_t>::value, int> = 0> constexpr phmap::weak_ordering do_three_way_comparison(const Compare &compare, const K &x, const LK &y) { @@ -721,8 +721,8 @@ namespace phmap { } template < typename Compare, typename K, typename LK, - phmap::enable_if_t>::value, + phmap::enable_if_t>::value, int> = 0> constexpr phmap::weak_ordering do_three_way_comparison(const Compare &compare, const K &x, const LK &y) { @@ -743,7 +743,7 @@ namespace container_internal { // comparator. template using btree_is_key_compare_to = - std::is_convertible, + std::is_convertible, phmap::weak_ordering>; struct StringBtreeDefaultLess { @@ -2507,7 +2507,7 @@ namespace container_internal { // Verify that key_compare returns an phmap::{weak,strong}_ordering or bool. using compare_result_type = - phmap::result_of_t; + phmap::invoke_result_t; static_assert( std::is_same::value || std::is_convertible::value, diff --git a/parallel_hashmap/phmap.h b/parallel_hashmap/phmap.h index bacdeea..47cf42a 100644 --- a/parallel_hashmap/phmap.h +++ b/parallel_hashmap/phmap.h @@ -2866,7 +2866,7 @@ public: Inner& inner = sets_[subidx(hashval)]; auto& set = inner.set_; typename Lockable::UniqueLock m(inner); - return make_iterator(&inner, set.lazy_emplace(key, hashval, std::forward(f))); + return make_iterator(&inner, set.lazy_emplace_with_hash(key, hashval, std::forward(f))); } // Extension API: support for heterogeneous keys. @@ -3032,11 +3032,8 @@ public: // -------------------------------------------------------------------- template iterator find(const key_arg& key, size_t hashval) { - Inner& inner = sets_[subidx(hashval)]; - auto& set = inner.set_; - typename Lockable::SharedLock m(inner); - auto it = set.find(key, hashval); - return make_iterator(&inner, it); + typename Lockable::SharedLock m; + return find(key, hashval, m); } template @@ -3194,6 +3191,15 @@ private: } protected: + template + iterator find(const key_arg& key, size_t hashval, typename Lockable::SharedLock &mutexlock) { + Inner& inner = sets_[subidx(hashval)]; + auto& set = inner.set_; + mutexlock = std::move(typename Lockable::SharedLock(inner)); + auto it = set.find(key, hashval); + return make_iterator(&inner, it); + } + template std::tuple find_or_prepare_insert(const K& key, typename Lockable::UniqueLock &mutexlock) { @@ -3219,7 +3225,7 @@ protected: } template - size_t hash(const K& key) { + size_t hash(const K& key) const { return HashElement{hash_ref()}(key); } @@ -3387,6 +3393,19 @@ public: return Policy::value(&*it); } + template + bool if_contains(const key_arg& key, F&& f) const { +#if __cplusplus >= 201703L + static_assert(std::is_invocable::value); +#endif + typename Lockable::SharedLock m; + auto it = const_cast(this)->find(key, this->hash(key), m); + if (it == this->end()) + return false; + std::forward(f)(Policy::value(&*it)); + return true; + } + template MappedReference

operator[](key_arg&& key) { return Policy::value(&*try_emplace(std::forward(key)).first); diff --git a/parallel_hashmap/phmap_base.h b/parallel_hashmap/phmap_base.h index 4cc3e00..2797682 100644 --- a/parallel_hashmap/phmap_base.h +++ b/parallel_hashmap/phmap_base.h @@ -328,8 +328,12 @@ using common_type_t = typename std::common_type::type; template using underlying_type_t = typename std::underlying_type::type; -template -using result_of_t = typename std::result_of::type; +template< class F, class... ArgTypes> +#if __cplusplus >= 201703L +using invoke_result_t = typename std::invoke_result_t; +#else +using invoke_result_t = typename std::result_of::type; +#endif namespace type_traits_internal { diff --git a/parallel_hashmap/phmap_fwd_decl.h b/parallel_hashmap/phmap_fwd_decl.h index 482dbab..f90417d 100644 --- a/parallel_hashmap/phmap_fwd_decl.h +++ b/parallel_hashmap/phmap_fwd_decl.h @@ -21,14 +21,14 @@ #include #include -#if defined(PHMAP_USE_ABSL_HASH) +#if defined(PHMAP_USE_ABSL_HASH) && !defined(ABSL_HASH_HASH_H_) namespace absl { template struct Hash; }; #endif namespace phmap { #if defined(PHMAP_USE_ABSL_HASH) - template using Hash = absl::Hash; + template using Hash = ::absl::Hash; #else template struct Hash; #endif diff --git a/parallel_hashmap/phmap_utils.h b/parallel_hashmap/phmap_utils.h index 0f38efa..72d4e71 100644 --- a/parallel_hashmap/phmap_utils.h +++ b/parallel_hashmap/phmap_utils.h @@ -33,6 +33,13 @@ #include #include "phmap_bits.h" +// --------------------------------------------------------------- +// Absl forward declaration requires global scope. +// --------------------------------------------------------------- +#if defined(PHMAP_USE_ABSL_HASH) && !defined(phmap_fwd_decl_h_guard_) && !defined(ABSL_HASH_HASH_H_) + namespace absl { template struct Hash; }; +#endif + namespace phmap { @@ -132,9 +139,8 @@ public: }; #if defined(PHMAP_USE_ABSL_HASH) && !defined(phmap_fwd_decl_h_guard_) - namespace absl { template struct Hash; }; - template using Hash = absl::Hash; -#else + template using Hash = ::absl::Hash; +#elif !defined(PHMAP_USE_ABSL_HASH) // --------------------------------------------------------------- // phmap::Hash // --------------------------------------------------------------- diff --git a/tests/btree_test.cc b/tests/btree_test.cc index 858c6df..31e73c0 100644 --- a/tests/btree_test.cc +++ b/tests/btree_test.cc @@ -1269,7 +1269,7 @@ namespace { "key_compare_to_adapter should have adapted this comparator."); static_assert( std::is_same>::value, + phmap::invoke_result_t>::value, "Adapted comparator should be a key-compare-to comparator."); } template @@ -1280,7 +1280,7 @@ namespace { "key_compare_to_adapter shouldn't have adapted this comparator."); static_assert( std::is_same>::value, + phmap::invoke_result_t>::value, "Un-adapted comparator should return bool."); } diff --git a/tests/btree_test.h b/tests/btree_test.h index 7f7438f..66c331d 100644 --- a/tests/btree_test.h +++ b/tests/btree_test.h @@ -440,8 +440,9 @@ namespace container_internal { class CountingAllocator : public std::allocator { public: using Alloc = std::allocator; - using pointer = typename Alloc::pointer; - using size_type = typename Alloc::size_type; + using AllocTraits = typename std::allocator_traits; + using pointer = typename AllocTraits::pointer; + using size_type = typename AllocTraits::size_type; CountingAllocator() : bytes_used_(nullptr) {} explicit CountingAllocator(int64_t* b) : bytes_used_(b) {} @@ -451,14 +452,14 @@ namespace container_internal { : Alloc(x), bytes_used_(x.bytes_used_) {} pointer allocate(size_type n, - std::allocator::const_pointer hint = nullptr) { + std::allocator_traits>::const_pointer hint = nullptr) { assert(bytes_used_ != nullptr); *bytes_used_ += n * sizeof(T); - return Alloc::allocate(n, hint); + return AllocTraits::allocate(*this, n, hint); } void deallocate(pointer p, size_type n) { - Alloc::deallocate(p, n); + AllocTraits::deallocate(*this, p, n); assert(bytes_used_ != nullptr); *bytes_used_ -= n * sizeof(T); } diff --git a/tests/flat_hash_map_test.cc b/tests/flat_hash_map_test.cc index 316c9f7..5f0e57f 100644 --- a/tests/flat_hash_map_test.cc +++ b/tests/flat_hash_map_test.cc @@ -244,6 +244,7 @@ TEST(THIS_TEST_NAME, MergeExtractInsert) { m.insert(std::move(node)); EXPECT_THAT(m, UnorderedElementsAre(Pair(1, 17), Pair(2, 9))); } + #if !defined(__ANDROID__) && !defined(__APPLE__) && !defined(__EMSCRIPTEN__) && defined(PHMAP_HAVE_STD_ANY) TEST(THIS_TEST_NAME, Any) { ThisMap m; diff --git a/tests/parallel_flat_hash_map_mutex_test.cc b/tests/parallel_flat_hash_map_mutex_test.cc index 9c20932..73dbf7b 100644 --- a/tests/parallel_flat_hash_map_mutex_test.cc +++ b/tests/parallel_flat_hash_map_mutex_test.cc @@ -9,4 +9,4 @@ #define THIS_EXTRA_TPL_PARAMS , 4, boost::upgrade_mutex #endif -#include "flat_hash_map_test.cc" +#include "parallel_hash_map_test.cc" diff --git a/tests/parallel_flat_hash_map_test.cc b/tests/parallel_flat_hash_map_test.cc index d1e6288..a1a8149 100644 --- a/tests/parallel_flat_hash_map_test.cc +++ b/tests/parallel_flat_hash_map_test.cc @@ -1,4 +1,4 @@ #define THIS_HASH_MAP parallel_flat_hash_map #define THIS_TEST_NAME ParallelFlatHashMap -#include "flat_hash_map_test.cc" +#include "parallel_hash_map_test.cc" diff --git a/tests/parallel_hash_map_test.cc b/tests/parallel_hash_map_test.cc new file mode 100644 index 0000000..c08b578 --- /dev/null +++ b/tests/parallel_hash_map_test.cc @@ -0,0 +1,27 @@ +#ifndef THIS_HASH_MAP + #define THIS_HASH_MAP parallel_flat_hash_map + #define THIS_TEST_NAME ParallelFlatHashMap +#endif + +#include "flat_hash_map_test.cc" + +namespace phmap { +namespace container_internal { +namespace { + +TEST(THIS_TEST_NAME, ThreadSafeContains) { + // We can't test mutable keys, or non-copyable keys with ThisMap. + // Test that the nodes have the proper API. + ThisMap m = { {1, 7}, {2, 9} }; + auto val = 0; + + auto func = [&val](int& v) { val = v; }; + EXPECT_TRUE(m.if_contains(2, func)); + EXPECT_EQ(val, 9); + + EXPECT_FALSE(m.if_contains(3, func)); +} + +} // namespace +} // namespace container_internal +} // namespace phmap diff --git a/tests/parallel_node_hash_map_test.cc b/tests/parallel_node_hash_map_test.cc index 615a0ba..f589d69 100644 --- a/tests/parallel_node_hash_map_test.cc +++ b/tests/parallel_node_hash_map_test.cc @@ -1,4 +1,4 @@ #define THIS_HASH_MAP parallel_node_hash_map #define THIS_TEST_NAME ParallelNodeHashMap -#include "flat_hash_map_test.cc" +#include "parallel_hash_map_test.cc"