Skip to content

Commit

Permalink
emplace() support for AtomicHashArray/Map
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Justin Gibbs authored and facebook-github-bot-1 committed Sep 24, 2015
1 parent 9013b5e commit 800a6af
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 45 deletions.
11 changes: 3 additions & 8 deletions folly/AtomicHashArray-inl.h
View file
Expand Up @@ -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_);
Expand Down Expand Up @@ -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
Expand Down
20 changes: 15 additions & 5 deletions folly/AtomicHashArray.h
View file
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);

Expand Down
27 changes: 8 additions & 19 deletions folly/AtomicHashMap-inl.h
View file
Expand Up @@ -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);
Expand All @@ -68,18 +57,18 @@ 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);
typename SubMap::SimpleRetT ret;
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
}
Expand Down Expand Up @@ -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);
}
Expand Down
27 changes: 19 additions & 8 deletions folly/AtomicHashMap.h
View file
Expand Up @@ -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 --
*
Expand All @@ -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 --
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 8 additions & 2 deletions folly/test/AtomicHashArrayTest.cpp
View file
Expand Up @@ -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);
}
Expand Down
12 changes: 9 additions & 3 deletions folly/test/AtomicHashMapTest.cpp
View file
Expand Up @@ -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());
}
}
Expand Down

0 comments on commit 800a6af

Please sign in to comment.