ConcurrentHashMap: Make iterators movable

Summary:
To reduce unnecessary copying for user code handling ConstIterator-s.

Added move test and change UpdateStressTest to use few concurrent iterators.

Reviewed By: yfeldblum

Differential Revision: D8587056

fbshipit-source-id: d6b592a3aa40ca8a7599b258b45a1d339088c978

folly/concurrency/ConcurrentHashMap.h

@@ -431,12 +431,26 @@ class ConcurrentHashMap {
       return *this;
     }
 
+    ConstIterator& operator=(ConstIterator&& o) noexcept {
+      if (this != &o) {
+        it_ = std::move(o.it_);
+        segment_ = std::exchange(o.segment_, uint64_t(NumShards));
+        parent_ = std::exchange(o.parent_, nullptr);
+      }
+      return *this;
+    }
+
     ConstIterator(const ConstIterator& o) {
       parent_ = o.parent_;
       it_ = o.it_;
       segment_ = o.segment_;
     }
 
+    ConstIterator(ConstIterator&& o) noexcept
+        : it_(std::move(o.it_)),
+          segment_(std::exchange(o.segment_, uint64_t(NumShards))),
+          parent_(std::exchange(o.parent_, nullptr)) {}
+
     ConstIterator(const ConcurrentHashMap* parent, uint64_t segment)
         : segment_(segment), parent_(parent) {}
 

folly/concurrency/detail/ConcurrentHashMap-detail.h

@@ -761,13 +761,12 @@ class alignas(64) ConcurrentHashMapSegment {
       bucket_count_ = o.bucket_count_;
     }
 
-    /* implicit */ Iterator(Iterator&& o) noexcept
-        : hazptrs_(std::move(o.hazptrs_)) {
-      node_ = o.node_;
-      buckets_ = o.buckets_;
-      idx_ = o.idx_;
-      bucket_count_ = o.bucket_count_;
-    }
+    Iterator(Iterator&& o) noexcept
+        : hazptrs_(std::move(o.hazptrs_)),
+          node_(std::exchange(o.node_, nullptr)),
+          buckets_(std::exchange(o.buckets_, nullptr)),
+          bucket_count_(std::exchange(o.bucket_count_, 0)),
+          idx_(std::exchange(o.idx_, 0)) {}
 
     // These are accessed directly from the functions above
     hazptr_array<3, Atom> hazptrs_;

folly/concurrency/test/ConcurrentHashMapTest.cpp

@@ -337,14 +337,18 @@ TEST(ConcurrentHashMap, UpdateStressTest) {
         unsigned long k = folly::hash::jenkins_rev_mix32((i + offset));
         k = k % (iters / num_threads) + offset;
         unsigned long val = 3;
-        auto res = m.find(k);
-        EXPECT_NE(res, m.cend());
-        EXPECT_EQ(k, res->second);
-        auto r = m.assign(k, res->second);
-        EXPECT_TRUE(r);
-        res = m.find(k);
-        EXPECT_NE(res, m.cend());
-        EXPECT_EQ(k, res->second);
+        {
+          auto res = m.find(k);
+          EXPECT_NE(res, m.cend());
+          EXPECT_EQ(k, res->second);
+          auto r = m.assign(k, res->second);
+          EXPECT_TRUE(r);
+        }
+        {
+          auto res = m.find(k);
+          EXPECT_NE(res, m.cend());
+          EXPECT_EQ(k, res->second);
+        }
         // Another random insertion to force table resizes
         val = size + i + offset;
         EXPECT_TRUE(m.insert(val, val).second);
@@ -722,3 +726,23 @@ TEST(ConcurrentHashMap, ForEachLoop) {
   }
   EXPECT_EQ(iters, 1);
 }
+
+TEST(ConcurrentHashMap, IteratorMove) {
+  using CHM = ConcurrentHashMap<int, int>;
+  using Iter = CHM::ConstIterator;
+  struct Foo {
+    Iter it;
+    explicit Foo(Iter&& it_) : it(std::move(it_)) {}
+    Foo(Foo&&) = default;
+    Foo& operator=(Foo&&) = default;
+  };
+  CHM map;
+  int k = 111;
+  int v = 999999;
+  map.insert(k, v);
+  Foo foo(map.find(k));
+  ASSERT_EQ(foo.it->second, v);
+  Foo foo2(map.find(0));
+  foo2 = std::move(foo);
+  ASSERT_EQ(foo2.it->second, v);
+}