emplace() support for AtomicHashArray/Map

Summary: Allow objects that only support in-place construction (no copy/move
constructor) to be stored in AtomicHashMap and AtomicHashArray via the
emplace() method. This uses variadic template parameters and perfect
forwarding to allow the arguments for any valid constructor for the
object to be used during insertion.

Reviewed By: @nbronson

Differential Revision: D2458152

diff --git a/folly/AtomicHashArray-inl.h b/folly/AtomicHashArray-inl.h
index 80272d4a8c1aad1e5bf2a51ed0b533f624b5d247..7d6b13e1f9e22b03a5a5800fbac27d0576d063ec 100644 (file)
--- a/folly/AtomicHashArray-inl.h
+++ b/folly/AtomicHashArray-inl.h
@@ -83,11 +83,11 @@ findInternal(const KeyT key_in) {
  */
 template <class KeyT, class ValueT,
           class HashFcn, class EqualFcn, class Allocator>
-template <class T>
+template <typename... ArgTs>
 typename AtomicHashArray<KeyT, ValueT,
          HashFcn, EqualFcn, Allocator>::SimpleRetT
 AtomicHashArray<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::
-insertInternal(KeyT key_in, T&& value) {
+insertInternal(KeyT key_in, ArgTs&&... vCtorArgs) {
   const short NO_NEW_INSERTS = 1;
   const short NO_PENDING_INSERTS = 2;
   CHECK_NE(key_in, kEmptyKey_);
@@ -133,12 +133,7 @@ insertInternal(KeyT key_in, T&& value) {
           // Write the value - done before unlocking
           try {
             DCHECK(relaxedLoadKey(*cell) == kLockedKey_);
-            /*
-             * This happens using the copy constructor because we won't have
-             * constructed a lhs to use an assignment operator on when
-             * values are being set.
-             */
-            new (&cell->second) ValueT(std::forward<T>(value));
+            new (&cell->second) ValueT(std::forward<ArgTs>(vCtorArgs)...);
             unlockCell(cell, key_in); // Sets the new key
           } catch (...) {
             // Transition back to empty key---requires handling

diff --git a/folly/AtomicHashArray.h b/folly/AtomicHashArray.h
index 6da92b6596ec35b47f3b64feb745f1b9f4afd7bf..fbe976d2c6ce142917b55158fceaf2eadd633ad8 100644 (file)
--- a/folly/AtomicHashArray.h
+++ b/folly/AtomicHashArray.h
@@ -161,11 +161,21 @@ class AtomicHashArray : boost::noncopyable {
    *   iterator is set to the existing entry.
    */
   std::pair<iterator,bool> insert(const value_type& r) {
-    SimpleRetT ret = insertInternal(r.first, r.second);
-    return std::make_pair(iterator(this, ret.idx), ret.success);
+    return emplace(r.first, r.second);
   }
   std::pair<iterator,bool> insert(value_type&& r) {
-    SimpleRetT ret = insertInternal(r.first, std::move(r.second));
+    return emplace(r.first, std::move(r.second));
+  }
+
+  /*
+   * emplace --
+   *
+   *   Same contract as insert(), but performs in-place construction
+   *   of the value type using the specified arguments.
+   */
+  template <typename... ArgTs>
+  std::pair<iterator,bool> emplace(KeyT key_in, ArgTs&&... vCtorArgs) {
+    SimpleRetT ret = insertInternal(key_in, std::forward<ArgTs>(vCtorArgs)...);
     return std::make_pair(iterator(this, ret.idx), ret.success);
   }
 
@@ -237,8 +247,8 @@ class AtomicHashArray : boost::noncopyable {
     SimpleRetT() = default;
   };
 
-  template <class T>
-  SimpleRetT insertInternal(KeyT key, T&& value);
+  template <typename... ArgTs>
+  SimpleRetT insertInternal(KeyT key, ArgTs&&... vCtorArgs);
 
   SimpleRetT findInternal(const KeyT key);
 

diff --git a/folly/AtomicHashMap-inl.h b/folly/AtomicHashMap-inl.h
index 92c0a1507012da83bbfe76b4fe8e3960d557e1d6..6d8bb917ee2dac266e2db52dd677959cabcccb36 100644 (file)
--- a/folly/AtomicHashMap-inl.h
+++ b/folly/AtomicHashMap-inl.h
@@ -40,26 +40,15 @@ AtomicHashMap(size_t finalSizeEst, const Config& config)
   numMapsAllocated_.store(1, std::memory_order_relaxed);
 }
 
-// insert --
+// emplace --
 template <typename KeyT, typename ValueT,
           typename HashFcn, typename EqualFcn, typename Allocator>
+template <typename... ArgTs>
 std::pair<typename AtomicHashMap<KeyT, ValueT, HashFcn,
                                  EqualFcn, Allocator>::iterator, bool>
 AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::
-insert(key_type k, const mapped_type& v) {
-  SimpleRetT ret = insertInternal(k,v);
-  SubMap* subMap = subMaps_[ret.i].load(std::memory_order_relaxed);
-  return std::make_pair(iterator(this, ret.i, subMap->makeIter(ret.j)),
-                        ret.success);
-}
-
-template <typename KeyT, typename ValueT,
-          typename HashFcn, typename EqualFcn, typename Allocator>
-std::pair<typename AtomicHashMap<KeyT, ValueT, HashFcn,
-                                 EqualFcn, Allocator>::iterator, bool>
-AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::
-insert(key_type k, mapped_type&& v) {
-  SimpleRetT ret = insertInternal(k, std::move(v));
+emplace(key_type k, ArgTs&&... vCtorArgs) {
+  SimpleRetT ret = insertInternal(k, std::forward<ArgTs>(vCtorArgs)...);
   SubMap* subMap = subMaps_[ret.i].load(std::memory_order_relaxed);
   return std::make_pair(iterator(this, ret.i, subMap->makeIter(ret.j)),
                         ret.success);
@@ -68,10 +57,10 @@ insert(key_type k, mapped_type&& v) {
 // insertInternal -- Allocates new sub maps as existing ones fill up.
 template <typename KeyT, typename ValueT,
           typename HashFcn, typename EqualFcn, typename Allocator>
-template <class T>
+template <typename... ArgTs>
 typename AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::SimpleRetT
 AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::
-insertInternal(key_type key, T&& value) {
+insertInternal(key_type key, ArgTs&&... vCtorArgs) {
  beginInsertInternal:
   auto nextMapIdx = // this maintains our state
     numMapsAllocated_.load(std::memory_order_acquire);
@@ -79,7 +68,7 @@ insertInternal(key_type key, T&& value) {
   FOR_EACH_RANGE(i, 0, nextMapIdx) {
     // insert in each map successively.  If one succeeds, we're done!
     SubMap* subMap = subMaps_[i].load(std::memory_order_relaxed);
-    ret = subMap->insertInternal(key, std::forward<T>(value));
+    ret = subMap->insertInternal(key, std::forward<ArgTs>(vCtorArgs)...);
     if (ret.idx == subMap->capacity_) {
       continue;  //map is full, so try the next one
     }
@@ -134,7 +123,7 @@ insertInternal(key_type key, T&& value) {
   // just did a spin wait with an acquire load on numMapsAllocated_.
   SubMap* loadedMap = subMaps_[nextMapIdx].load(std::memory_order_relaxed);
   DCHECK(loadedMap && loadedMap != (SubMap*)kLockedPtr_);
-  ret = loadedMap->insertInternal(key, std::forward<T>(value));
+  ret = loadedMap->insertInternal(key, std::forward<ArgTs>(vCtorArgs)...);
   if (ret.idx != loadedMap->capacity_) {
     return SimpleRetT(nextMapIdx, ret.idx, ret.success);
   }

diff --git a/folly/AtomicHashMap.h b/folly/AtomicHashMap.h
index fb35388732e85740377aa47102bfe17929fb8d4b..90c59661fa81d1de7ca63b0ce3fa72479d2fc10b 100644 (file)
--- a/folly/AtomicHashMap.h
+++ b/folly/AtomicHashMap.h
@@ -205,8 +205,6 @@ class AtomicHashMap : boost::noncopyable {
   key_equal key_eq() const { return key_equal(); }
   hasher hash_function() const { return hasher(); }
 
-  // TODO: emplace() support would be nice.
-
   /*
    * insert --
    *
@@ -223,13 +221,26 @@ class AtomicHashMap : boost::noncopyable {
    *   AtomicHashMapFullError is thrown.
    */
   std::pair<iterator,bool> insert(const value_type& r) {
-    return insert(r.first, r.second);
+    return emplace(r.first, r.second);
+  }
+  std::pair<iterator,bool> insert(key_type k, const mapped_type& v) {
+    return emplace(k, v);
   }
-  std::pair<iterator,bool> insert(key_type k, const mapped_type& v);
   std::pair<iterator,bool> insert(value_type&& r) {
-    return insert(r.first, std::move(r.second));
+    return emplace(r.first, std::move(r.second));
   }
-  std::pair<iterator,bool> insert(key_type k, mapped_type&& v);
+  std::pair<iterator,bool> insert(key_type k, mapped_type&& v) {
+    return emplace(k, std::move(v));
+  }
+
+  /*
+   * emplace --
+   *
+   *   Same contract as insert(), but performs in-place construction
+   *   of the value type using the specified arguments.
+   */
+  template <typename... ArgTs>
+  std::pair<iterator,bool> emplace(key_type k, ArgTs&&... vCtorArg);
 
   /*
    * find --
@@ -391,8 +402,8 @@ class AtomicHashMap : boost::noncopyable {
     SimpleRetT() = default;
   };
 
-  template <class T>
-  SimpleRetT insertInternal(KeyT key, T&& value);
+  template <typename... ArgTs>
+  SimpleRetT insertInternal(KeyT key, ArgTs&&... value);
 
   SimpleRetT findInternal(const KeyT k) const;
 

diff --git a/folly/test/AtomicHashArrayTest.cpp b/folly/test/AtomicHashArrayTest.cpp
index c1eaf6d91a69780024aede5b05099eac70240445..3c7c32f29cac26c802df0c0ce93272f8c31898f2 100644 (file)
--- a/folly/test/AtomicHashArrayTest.cpp
+++ b/folly/test/AtomicHashArrayTest.cpp
@@ -148,11 +148,17 @@ template<class KeyT, class ValueT, class Allocator = std::allocator<char>>
 void testNoncopyableMap() {
   typedef AtomicHashArray<KeyT, std::unique_ptr<ValueT>, std::hash<KeyT>,
                           std::equal_to<KeyT>, Allocator> MyArr;
-  auto arr = MyArr::create(150);
+  auto arr = MyArr::create(250);
   for (int i = 0; i < 100; i++) {
     arr->insert(make_pair(i,std::unique_ptr<ValueT>(new ValueT(i))));
   }
-  for (int i = 0; i < 100; i++) {
+  for (int i = 100; i < 150; i++) {
+    arr->emplace(i,new ValueT(i));
+  }
+  for (int i = 150; i < 200; i++) {
+    arr->emplace(i,new ValueT(i),std::default_delete<ValueT>());
+  }
+  for (int i = 0; i < 200; i++) {
     auto ret = arr->find(i);
     EXPECT_EQ(*(ret->second), i);
   }

diff --git a/folly/test/AtomicHashMapTest.cpp b/folly/test/AtomicHashMapTest.cpp
index a613a7c25b2c7e2db004ca64567ad0312d6ce383..0a65d361494aa7babdcd950e777baceb9a5c5e48 100644 (file)
--- a/folly/test/AtomicHashMapTest.cpp
+++ b/folly/test/AtomicHashMapTest.cpp
@@ -78,13 +78,19 @@ TEST(Ahm, BasicNoncopyable) {
   for (int i = 50; i < 100; ++i) {
     myMap.insert(i, std::unique_ptr<int>(new int (i)));
   }
-  for (int i = 0; i < 100; ++i) {
+  for (int i = 100; i < 150; ++i) {
+    myMap.emplace(i, new int (i));
+  }
+  for (int i = 150; i < 200; ++i) {
+    myMap.emplace(i, new int (i), std::default_delete<int>());
+  }
+  for (int i = 0; i < 200; ++i) {
     EXPECT_EQ(*(myMap.find(i)->second), i);
   }
-  for (int i = 0; i < 100; i+=4) {
+  for (int i = 0; i < 200; i+=4) {
     myMap.erase(i);
   }
-  for (int i = 0; i < 100; i+=4) {
+  for (int i = 0; i < 200; i+=4) {
     EXPECT_EQ(myMap.find(i), myMap.end());
   }
 }