Commit 9a33b9b1 authored by Nathan Bronson's avatar Nathan Bronson Committed by Facebook Github Bot

destructuring for F14*Set::emplace

Summary:
This diff optimizes the operation of F14 set emplace when
the argument list happens to be a single reference to a key_type.
An intermediate key_type value is removed in that case, which avoids a
move in all cases and avoids a copy when the emplace finds that the key
is already present in the set.

Reviewed By: yfeldblum

Differential Revision: D7845720

fbshipit-source-id: 16a639f53993d9843ffe265edd58c0d74de0c1f7
parent ede7e6eb
......@@ -449,7 +449,7 @@ class F14BasicMap {
return emplaceItem(std::move(p.first), std::move(p.second));
}
template <typename U1, class... Args2>
template <typename U1, typename... Args2>
std::enable_if_t<
std::is_same<folly::remove_cvref_t<U1>, key_type>::value,
std::pair<ItemIter, bool>>
......@@ -467,9 +467,9 @@ class F14BasicMap {
std::move(second_args)});
}
template <class... Args1, class... Args2>
template <typename... Args1, typename... Args2>
std::enable_if_t<
std::tuple_size<std::tuple<Args1...>>::value != 1 ||
sizeof...(Args1) != 1 ||
!std::is_same<
folly::remove_cvref_t<
std::tuple_element_t<0, std::tuple<Args1..., value_type>>>,
......
......@@ -27,6 +27,8 @@
* @author Xiao Shi <xshi@fb.com>
*/
#include <tuple>
#include <folly/lang/SafeAssert.h>
#include <folly/container/F14Set-pre.h>
......@@ -122,6 +124,10 @@ class F14BasicSet {
using iterator = typename Policy::Iter;
using const_iterator = iterator;
private:
using ItemIter = typename Policy::ItemIter;
public:
//// PUBLIC - Member functions
F14BasicSet() noexcept(F14Table<Policy>::kDefaultConstructIsNoexcept)
......@@ -276,16 +282,11 @@ class F14BasicSet {
}
std::pair<iterator, bool> insert(value_type const& value) {
auto rv = table_.tryEmplaceValue(value, value);
return std::make_pair(table_.makeIter(rv.first), rv.second);
return emplace(value);
}
std::pair<iterator, bool> insert(value_type&& value) {
// tryEmplaceValue guarantees not to touch the first arg after touching
// any others, so although this looks fishy it is okay
value_type const& searchKey = value;
auto rv = table_.tryEmplaceValue(searchKey, std::move(value));
return std::make_pair(table_.makeIter(rv.first), rv.second);
return emplace(std::move(value));
}
// std::unordered_set's hinted insertion API is misleading. No
......@@ -349,15 +350,40 @@ class F14BasicSet {
insert(ilist.begin(), ilist.end());
}
// node API doesn't make sense for value set, which stores values inline
private:
std::pair<ItemIter, bool> emplaceItem() {
// rare but valid
return table_.tryEmplaceValue(key_type{});
}
std::pair<ItemIter, bool> emplaceItem(key_type&& key) {
// best case
return table_.tryEmplaceValue(key, std::move(key));
}
std::pair<ItemIter, bool> emplaceItem(key_type const& key) {
// okay case, no construction unless we will actually insert
return table_.tryEmplaceValue(key, key);
}
// emplace won't actually be more efficient than insert until we
// add heterogeneous lookup, but it is still useful now from a code
// compactness standpoint.
template <typename... Args>
std::enable_if_t<
sizeof...(Args) != 1 ||
!std::is_same<
folly::remove_cvref_t<
std::tuple_element_t<0, std::tuple<Args...>>>,
key_type>::value,
std::pair<ItemIter, bool>>
emplaceItem(Args&&... args) {
key_type key(std::forward<Args>(args)...);
return table_.tryEmplaceValue(key, std::move(key));
}
public:
template <class... Args>
std::pair<iterator, bool> emplace(Args&&... args) {
key_type key(std::forward<Args>(args)...);
return insert(std::move(key));
auto rv = emplaceItem(std::forward<Args>(args)...);
return std::make_pair(table_.makeIter(rv.first), rv.second);
}
template <class... Args>
......
......@@ -16,6 +16,8 @@
#pragma once
#include <tuple>
#include <folly/container/detail/F14Table.h>
#include <folly/hash/Hash.h>
#include <folly/lang/SafeAssert.h>
......
......@@ -25,7 +25,6 @@
#include <limits>
#include <memory>
#include <new>
#include <tuple>
#include <type_traits>
#include <utility>
#include <vector>
......
......@@ -929,6 +929,8 @@ void runInsertAndEmplace(std::string const& name) {
EXPECT_EQ(m.count(k), 0);
m.emplace();
EXPECT_EQ(m.count(k), 1);
m.emplace();
EXPECT_EQ(m.count(k), 1);
}
TEST(F14ValueMap, destructuring) {
......
......@@ -514,6 +514,151 @@ TEST(F14ValueSet, steady_state_stats) {
F14TableStats::compute(h);
}
// S should be a set of Tracked<0>. F should take a set
// and a key_type const& or key_type&& and cause it to be inserted
template <typename S, typename F>
void runInsertCases(std::string const& /* name */, F const& insertFunc) {
static_assert(std::is_same<typename S::value_type, Tracked<0>>::value, "");
{
typename S::value_type k{0};
S s;
resetTracking();
insertFunc(s, k);
// fresh key, value_type const& ->
// copy is expected
EXPECT_EQ(Tracked<0>::counts.dist(Counts{1, 0, 0, 0}), 0);
}
{
typename S::value_type k{0};
S s;
resetTracking();
insertFunc(s, std::move(k));
// fresh key, value_type&& ->
// move is expected
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 1, 0, 0}), 0);
}
}
struct DoInsert {
template <typename M, typename P>
void operator()(M& m, P&& p) const {
m.insert(std::forward<P>(p));
}
};
struct DoEmplace1 {
template <typename M, typename P>
void operator()(M& m, P&& p) const {
m.emplace(std::forward<P>(p));
}
};
template <typename S>
void runInsertAndEmplace() {
{
typename S::value_type k1{0};
typename S::value_type k2{0};
S s;
resetTracking();
EXPECT_TRUE(s.insert(k1).second);
// copy is expected on successful insert
EXPECT_EQ(Tracked<0>::counts.dist(Counts{1, 0, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.insert(k2).second);
// no copies or moves on failing insert
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 0, 0}), 0);
}
{
typename S::value_type k1{0};
typename S::value_type k2{0};
S s;
resetTracking();
EXPECT_TRUE(s.insert(std::move(k1)).second);
// move is expected on successful insert
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 1, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.insert(std::move(k2)).second);
// no copies or moves on failing insert
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 0, 0}), 0);
}
{
typename S::value_type k1{0};
typename S::value_type k2{0};
uint64_t k3 = 0;
uint64_t k4 = 10;
S s;
resetTracking();
EXPECT_TRUE(s.emplace(k1).second);
// copy is expected on successful emplace
EXPECT_EQ(Tracked<0>::counts.dist(Counts{1, 0, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.emplace(k2).second);
// no copies or moves on failing emplace with value_type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.emplace(k3).second);
// copy convert expected for failing emplace with wrong type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 1, 0}), 0);
resetTracking();
EXPECT_TRUE(s.emplace(k4).second);
// copy convert + move expected for successful emplace with wrong type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 1, 1, 0}), 0);
}
{
typename S::value_type k1{0};
typename S::value_type k2{0};
uint64_t k3 = 0;
uint64_t k4 = 10;
S s;
resetTracking();
EXPECT_TRUE(s.emplace(std::move(k1)).second);
// move is expected on successful emplace
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 1, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.emplace(std::move(k2)).second);
// no copies or moves on failing emplace with value_type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 0, 0}), 0);
resetTracking();
EXPECT_FALSE(s.emplace(std::move(k3)).second);
// move convert expected for failing emplace with wrong type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 0, 0, 1}), 0);
resetTracking();
EXPECT_TRUE(s.emplace(std::move(k4)).second);
// move convert + move expected for successful emplace with wrong type
EXPECT_EQ(Tracked<0>::counts.dist(Counts{0, 1, 0, 1}), 0);
}
// Calling the default pair constructor via emplace is valid, but not
// very useful in real life. Verify that it works.
S s;
typename S::value_type k;
EXPECT_EQ(s.count(k), 0);
s.emplace();
EXPECT_EQ(s.count(k), 1);
s.emplace();
EXPECT_EQ(s.count(k), 1);
}
TEST(F14ValueSet, destructuring) {
runInsertAndEmplace<F14ValueSet<Tracked<0>>>();
}
TEST(F14NodeSet, destructuring) {
runInsertAndEmplace<F14NodeSet<Tracked<0>>>();
}
TEST(F14VectorSet, destructuring) {
runInsertAndEmplace<F14VectorSet<Tracked<0>>>();
}
TEST(F14ValueSet, vectorMaxSize) {
F14ValueSet<int> s;
EXPECT_EQ(s.max_size(), std::numeric_limits<uint64_t>::max() / sizeof(int));
......
......@@ -211,10 +211,14 @@ struct Tracked {
sumCounts.defaultConstruct++;
counts.defaultConstruct++;
}
/* implicit */ Tracked(uint64_t val) : val_{val} {
/* implicit */ Tracked(uint64_t const& val) : val_{val} {
sumCounts.copyConvert++;
counts.copyConvert++;
}
/* implicit */ Tracked(uint64_t&& val) : val_{val} {
sumCounts.moveConvert++;
counts.moveConvert++;
}
Tracked(Tracked const& rhs) : val_{rhs.val_} {
sumCounts.copyConstruct++;
counts.copyConstruct++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment