Permalink
Browse files

Make AtomicHashMap support move constructible types

Summary: modified AtomicHashArray and AtomicHashMap to support move constructible types

Test Plan: tested with fbcode/folly/test/AtomicHashArrayTest.cpp and fbcode/folly/test/AtomicHashMapTest.cpp

Reviewed By: philipp@fb.com

FB internal diff: D527270
  • Loading branch information...
1 parent f1c708f commit bc46f015ee17b1c8fb00a546aff8fd0e80b029e4 Wei Wu committed with jdelong Jul 23, 2012
@@ -78,12 +78,12 @@ findInternal(const KeyT key_in) {
* default.
*/
template <class KeyT, class ValueT, class HashFcn>
+template <class T>
typename AtomicHashArray<KeyT, ValueT, HashFcn>::SimpleRetT
AtomicHashArray<KeyT, ValueT, HashFcn>::
-insertInternal(const value_type& record) {
+insertInternal(KeyT key_in, T&& value) {
const short NO_NEW_INSERTS = 1;
const short NO_PENDING_INSERTS = 2;
- const KeyT key_in = record.first;
CHECK_NE(key_in, kEmptyKey_);
CHECK_NE(key_in, kLockedKey_);
CHECK_NE(key_in, kErasedKey_);
@@ -131,7 +131,7 @@ insertInternal(const value_type& record) {
* constructed a lhs to use an assignment operator on when
* values are being set.
*/
- new (&cell->second) ValueT(record.second);
+ new (&cell->second) ValueT(std::forward<T>(value));
unlockCell(cell, key_in); // Sets the new key
} catch (...) {
// Transition back to empty key---requires handling
@@ -150,7 +150,11 @@ class AtomicHashArray : boost::noncopyable {
* iterator is set to the existing entry.
*/
std::pair<iterator,bool> insert(const value_type& r) {
- SimpleRetT ret = insertInternal(r);
+ SimpleRetT ret = insertInternal(r.first, r.second);
+ return std::make_pair(iterator(this, ret.idx), ret.success);
+ }
+ std::pair<iterator,bool> insert(value_type&& r) {
+ SimpleRetT ret = insertInternal(r.first, std::move(r.second));
return std::make_pair(iterator(this, ret.idx), ret.success);
}
@@ -213,7 +217,8 @@ class AtomicHashArray : boost::noncopyable {
SimpleRetT() {}
};
- SimpleRetT insertInternal(const value_type& record);
+ template <class T>
+ SimpleRetT insertInternal(KeyT key, T&& value);
SimpleRetT findInternal(const KeyT key);
@@ -46,18 +46,29 @@ AtomicHashMap(size_t size, const Config& config)
template <typename KeyT, typename ValueT, typename HashFcn>
std::pair<typename AtomicHashMap<KeyT,ValueT,HashFcn>::iterator,bool>
AtomicHashMap<KeyT, ValueT, HashFcn>::
-insert(const value_type& r) {
- SimpleRetT ret = insertInternal(r);
+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>
+std::pair<typename AtomicHashMap<KeyT,ValueT,HashFcn>::iterator,bool>
+AtomicHashMap<KeyT, ValueT, HashFcn>::
+insert(key_type k, mapped_type&& v) {
+ SimpleRetT ret = insertInternal(k, std::move(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);
}
// insertInternal -- Allocates new sub maps as existing ones fill up.
template <typename KeyT, typename ValueT, typename HashFcn>
+template <class T>
typename AtomicHashMap<KeyT, ValueT, HashFcn>::SimpleRetT
AtomicHashMap<KeyT, ValueT, HashFcn>::
-insertInternal(const value_type& r) {
+insertInternal(key_type key, T&& value) {
beginInsertInternal:
int nextMapIdx = // this maintains our state
numMapsAllocated_.load(std::memory_order_acquire);
@@ -66,7 +77,7 @@ insertInternal(const value_type& r) {
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(r);
+ ret = subMap->insertInternal(key, std::forward<T>(value));
if (ret.idx == subMap->capacity_) {
continue; //map is full, so try the next one
}
@@ -121,7 +132,7 @@ insertInternal(const value_type& r) {
// 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(r);
+ ret = loadedMap->insertInternal(key, std::forward<T>(value));
if (ret.idx != loadedMap->capacity_) {
return SimpleRetT(nextMapIdx, ret.idx, ret.success);
}
View
@@ -225,10 +225,14 @@ class AtomicHashMap : boost::noncopyable {
* all sub maps are full, no element is inserted, and
* AtomicHashMapFullError is thrown.
*/
- std::pair<iterator,bool> insert(const value_type& r);
- std::pair<iterator,bool> insert(key_type k, const mapped_type& v) {
- return insert(value_type(k, v));
+ std::pair<iterator,bool> insert(const value_type& r) {
+ return insert(r.first, r.second);
}
+ 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));
+ }
+ std::pair<iterator,bool> insert(key_type k, mapped_type&& v);
/*
* find --
@@ -336,7 +340,26 @@ class AtomicHashMap : boost::noncopyable {
/* Advanced functions for direct access: */
inline uint32_t recToIdx(const value_type& r, bool mayInsert = true) {
- SimpleRetT ret = mayInsert ? insertInternal(r) : findInternal(r.first);
+ SimpleRetT ret = mayInsert ?
+ insertInternal(r.first, r.second) : findInternal(r.first);
+ return encodeIndex(ret.i, ret.j);
+ }
+
+ inline uint32_t recToIdx(value_type&& r, bool mayInsert = true) {
+ SimpleRetT ret = mayInsert ?
+ insertInternal(r.first, std::move(r.second)) : findInternal(r.first);
+ return encodeIndex(ret.i, ret.j);
+ }
+
+ inline uint32_t recToIdx(key_type k, const mapped_type& v,
+ bool mayInsert = true) {
+ SimpleRetT ret = mayInsert ? insertInternal(k, v) : findInternal(k);
+ return encodeIndex(ret.i, ret.j);
+ }
+
+ inline uint32_t recToIdx(key_type k, mapped_type&& v, bool mayInsert = true) {
+ SimpleRetT ret = mayInsert ?
+ insertInternal(k, std::move(v)) : findInternal(k);
return encodeIndex(ret.i, ret.j);
}
@@ -367,7 +390,8 @@ class AtomicHashMap : boost::noncopyable {
SimpleRetT() {}
};
- SimpleRetT insertInternal(const value_type& r);
+ template <class T>
+ SimpleRetT insertInternal(KeyT key, T&& value);
SimpleRetT findInternal(const KeyT k) const;
@@ -75,17 +75,35 @@ void testMap() {
}
}
+template<class KeyT, class ValueT>
+void testNoncopyableMap() {
+ typedef AtomicHashArray<KeyT, std::unique_ptr<ValueT>> MyArr;
+ auto arr = MyArr::create(150);
+ 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++) {
+ auto ret = arr->find(i);
+ EXPECT_EQ(*(ret->second), i);
+ }
+}
+
+
TEST(Aha, InsertErase_i32_i32) {
testMap<int32_t,int32_t>();
+ testNoncopyableMap<int32_t,int32_t>();
}
TEST(Aha, InsertErase_i64_i32) {
testMap<int64_t,int32_t>();
+ testNoncopyableMap<int64_t,int32_t>();
}
TEST(Aha, InsertErase_i64_i64) {
testMap<int64_t,int64_t>();
+ testNoncopyableMap<int64_t,int64_t>();
}
TEST(Aha, InsertErase_i32_i64) {
testMap<int32_t,int64_t>();
+ testNoncopyableMap<int32_t,int64_t>();
}
TEST(Aha, InsertErase_i32_str) {
testMap<int32_t,string>();
@@ -21,7 +21,7 @@
#include <sys/time.h>
#include <thread>
#include <atomic>
-
+#include <memory>
#include "folly/Benchmark.h"
#include "folly/Conv.h"
@@ -66,6 +66,29 @@ TEST(Ahm, BasicStrings) {
EXPECT_EQ(myMap.find(999)->first, 999);
}
+
+TEST(Ahm, BasicNoncopyable) {
+ typedef AtomicHashMap<int64_t,std::unique_ptr<int>> AHM;
+ AHM myMap(1024);
+ EXPECT_TRUE(myMap.begin() == myMap.end());
+
+ for (int i = 0; i < 50; ++i) {
+ myMap.insert(make_pair(i, std::unique_ptr<int>(new int(i))));
+ }
+ for (int i = 50; i < 100; ++i) {
+ myMap.insert(i, std::unique_ptr<int>(new int (i)));
+ }
+ for (int i = 0; i < 100; ++i) {
+ EXPECT_EQ(*(myMap.find(i)->second), i);
+ }
+ for (int i = 0; i < 100; i+=4) {
+ myMap.erase(i);
+ }
+ for (int i = 0; i < 100; i+=4) {
+ EXPECT_EQ(myMap.find(i), myMap.end());
+ }
+}
+
typedef int32_t KeyT;
typedef int64_t KeyTBig;
typedef int32_t ValueT;
@@ -168,9 +191,9 @@ TEST(Ahm, iterator) {
class Counters {
private:
- // NOTE: Unfortunately can't currently put a std::atomic<int64_t> in
- // the value in ahm since it doesn't support non-copyable but
- // move-constructible value types yet.
+ // Note: Unfortunately can't currently put a std::atomic<int64_t> in
+ // the value in ahm since it doesn't support types that are both non-copy
+ // and non-move constructible yet.
AtomicHashMap<int64_t,int64_t> ahm;
public:

0 comments on commit bc46f01

Please sign in to comment.