Merge pull request #48 from bpmckinnon/master

Add thread-safe accessors to parallel_hash_map

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`
 

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>(f)));
+        return make_iterator(&inner, set.lazy_emplace_with_hash(key, hashval, std::forward<F>(f)));
     }
 
     // Extension API: support for heterogeneous keys.
@@ -3032,11 +3032,8 @@ public:
     // --------------------------------------------------------------------
     template <class K = key_type>
     iterator find(const key_arg<K>& 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 <class K = key_type>
@@ -3194,6 +3191,15 @@ private:
     }
 
 protected:
+    template <class K = key_type>
+    iterator find(const key_arg<K>& 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 <class K>
     std::tuple<Inner*, size_t, bool> 
     find_or_prepare_insert(const K& key, typename Lockable::UniqueLock &mutexlock) {
@@ -3219,7 +3225,7 @@ protected:
     }
 
     template <class K>
-    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 <class K = key_type, class F>
+    bool if_contains(const key_arg<K>& key, F&& f) const {
+#if __cplusplus >= 201703L
+        static_assert(std::is_invocable<F, mapped_type&>::value);
+#endif
+        typename Lockable::SharedLock m;
+        auto it = const_cast<parallel_hash_map*>(this)->find(key, this->hash(key), m);
+        if (it == this->end())
+            return false;
+        std::forward<F>(f)(Policy::value(&*it));
+        return true;
+    }
+
     template <class K = key_type, class P = Policy, K* = nullptr>
     MappedReference<P> operator[](key_arg<K>&& key) {
         return Policy::value(&*try_emplace(std::forward<K>(key)).first);

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<int, std::any> m;

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"

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"

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<int, int> 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

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"