diff --git a/folly/AtomicHashArray-inl.h b/folly/AtomicHashArray-inl.h index 5d1a5b1e4fc..8982728b8be 100644 --- a/folly/AtomicHashArray-inl.h +++ b/folly/AtomicHashArray-inl.h @@ -78,12 +78,12 @@ findInternal(const KeyT key_in) { * default. */ template +template typename AtomicHashArray::SimpleRetT AtomicHashArray:: -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(value)); 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 e1bcc5d026a..69e80dfb243 100644 --- a/folly/AtomicHashArray.h +++ b/folly/AtomicHashArray.h @@ -150,7 +150,11 @@ class AtomicHashArray : boost::noncopyable { * iterator is set to the existing entry. */ std::pair 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 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 + SimpleRetT insertInternal(KeyT key, T&& value); SimpleRetT findInternal(const KeyT key); diff --git a/folly/AtomicHashMap-inl.h b/folly/AtomicHashMap-inl.h index 01919f24032..ec90654abdb 100644 --- a/folly/AtomicHashMap-inl.h +++ b/folly/AtomicHashMap-inl.h @@ -46,8 +46,18 @@ AtomicHashMap(size_t size, const Config& config) template std::pair::iterator,bool> AtomicHashMap:: -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 +std::pair::iterator,bool> +AtomicHashMap:: +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); @@ -55,9 +65,10 @@ insert(const value_type& r) { // insertInternal -- Allocates new sub maps as existing ones fill up. template +template typename AtomicHashMap::SimpleRetT AtomicHashMap:: -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(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(value)); if (ret.idx != loadedMap->capacity_) { return SimpleRetT(nextMapIdx, ret.idx, ret.success); } diff --git a/folly/AtomicHashMap.h b/folly/AtomicHashMap.h index 8e02e39eb71..86d01ecfc3e 100644 --- a/folly/AtomicHashMap.h +++ b/folly/AtomicHashMap.h @@ -225,10 +225,14 @@ class AtomicHashMap : boost::noncopyable { * all sub maps are full, no element is inserted, and * AtomicHashMapFullError is thrown. */ - std::pair insert(const value_type& r); - std::pair insert(key_type k, const mapped_type& v) { - return insert(value_type(k, v)); + std::pair insert(const value_type& r) { + return insert(r.first, r.second); } + std::pair insert(key_type k, const mapped_type& v); + std::pair insert(value_type&& r) { + return insert(r.first, std::move(r.second)); + } + std::pair 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 + SimpleRetT insertInternal(KeyT key, T&& value); SimpleRetT findInternal(const KeyT k) const; diff --git a/folly/test/AtomicHashArrayTest.cpp b/folly/test/AtomicHashArrayTest.cpp index 42fe1bc175d..bebaaf941f8 100644 --- a/folly/test/AtomicHashArrayTest.cpp +++ b/folly/test/AtomicHashArrayTest.cpp @@ -75,17 +75,35 @@ void testMap() { } } +template +void testNoncopyableMap() { + typedef AtomicHashArray> MyArr; + auto arr = MyArr::create(150); + for (int i = 0; i < 100; i++) { + arr->insert(make_pair(i,std::unique_ptr(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(); + testNoncopyableMap(); } TEST(Aha, InsertErase_i64_i32) { testMap(); + testNoncopyableMap(); } TEST(Aha, InsertErase_i64_i64) { testMap(); + testNoncopyableMap(); } TEST(Aha, InsertErase_i32_i64) { testMap(); + testNoncopyableMap(); } TEST(Aha, InsertErase_i32_str) { testMap(); diff --git a/folly/test/AtomicHashMapTest.cpp b/folly/test/AtomicHashMapTest.cpp index d869c1cf083..84105965221 100644 --- a/folly/test/AtomicHashMapTest.cpp +++ b/folly/test/AtomicHashMapTest.cpp @@ -21,7 +21,7 @@ #include #include #include - +#include #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> 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(new int(i)))); + } + for (int i = 50; i < 100; ++i) { + myMap.insert(i, std::unique_ptr(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 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 in + // the value in ahm since it doesn't support types that are both non-copy + // and non-move constructible yet. AtomicHashMap ahm; public: