Commit 7153cc6b authored by Kevin Doherty's avatar Kevin Doherty Committed by Facebook Github Bot

add eraseInto api to F14Set

Summary:
- Expose an API to erase an item from `F14Set` and do something with it before it is destroyed
- This includes (but is not limited to) being able to `std::move` it to another container while avoiding a copy
- Change erase implementations to forward to `eraseInto(...args, emptyCallback)`

Reviewed By: shixiao

Differential Revision: D7588928

fbshipit-source-id: 5e9c2bda216c5c219c333282daadd2170e6a1eba
parent 2584b1f2
...@@ -575,20 +575,20 @@ class F14BasicMap { ...@@ -575,20 +575,20 @@ class F14BasicMap {
return table_.find(key).atEnd() ? 0 : 1; return table_.find(key).atEnd() ? 0 : 1;
} }
/// prehash(key) does the work of evaluating hash_function()(key) // prehash(key) does the work of evaluating hash_function()(key)
/// (including additional bit-mixing for non-avalanching hash functions), // (including additional bit-mixing for non-avalanching hash functions),
/// wraps the result of that work in a token for later reuse, and // wraps the result of that work in a token for later reuse, and
/// begins prefetching the first steps of looking for key into the // begins prefetching the first steps of looking for key into the
/// local CPU cache. // local CPU cache.
/// //
/// The returned token may be used at any time, may be used more than // The returned token may be used at any time, may be used more than
/// once, and may be used in other F14 sets and maps. Tokens are // once, and may be used in other F14 sets and maps. Tokens are
/// transferrable between any F14 containers (maps and sets) with the // transferrable between any F14 containers (maps and sets) with the
/// same key_type and equal hash_function()s. // same key_type and equal hash_function()s.
/// //
/// Hash tokens are not hints -- it is a bug to call any method on this // Hash tokens are not hints -- it is a bug to call any method on this
/// class with a token t and key k where t isn't the result of a call // class with a token t and key k where t isn't the result of a call
/// to prehash(k2) with k2 == k. // to prehash(k2) with k2 == k.
F14HashToken prehash(key_type const& key) const { F14HashToken prehash(key_type const& key) const {
return table_.prehash(key); return table_.prehash(key);
} }
......
...@@ -340,22 +340,44 @@ class F14BasicSet { ...@@ -340,22 +340,44 @@ class F14BasicSet {
} }
FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) { FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) {
return eraseInto(pos, [](value_type&) {});
}
iterator erase(const_iterator first, const_iterator last) {
return eraseInto(first, last, [](value_type&) {});
}
size_type erase(key_type const& key) {
return eraseInto(key, [](value_type&) {});
}
// eraseInto contains the same overloads as erase but provides
// an additional callback argument which is called with a non-const lvalue
// reference to the item directly before it is destroyed. This can be
// used to extract an item out of a F14Set while avoiding a copy.
template <typename BeforeDestroy>
FOLLY_ALWAYS_INLINE iterator
eraseInto(const_iterator pos, BeforeDestroy const& beforeDestroy) {
// If we are inlined then gcc and clang can optimize away all of the // If we are inlined then gcc and clang can optimize away all of the
// work of ++pos if the caller discards it. // work of ++pos if the caller discards it.
table_.erase(table_.unwrapIter(pos)); table_.eraseInto(table_.unwrapIter(pos), beforeDestroy);
return ++pos; return ++pos;
} }
iterator erase(const_iterator first, const_iterator last) { template <typename BeforeDestroy>
iterator eraseInto(
const_iterator first,
const_iterator last,
BeforeDestroy const& beforeDestroy) {
while (first != last) { while (first != last) {
table_.erase(table_.unwrapIter(first)); first = eraseInto(first, beforeDestroy);
++first;
} }
return first; return first;
} }
size_type erase(key_type const& key) { template <typename BeforeDestroy>
return table_.erase(key); size_type eraseInto(key_type const& key, BeforeDestroy const& beforeDestroy) {
return table_.eraseInto(key, beforeDestroy);
} }
//// PUBLIC - Lookup //// PUBLIC - Lookup
...@@ -369,20 +391,20 @@ class F14BasicSet { ...@@ -369,20 +391,20 @@ class F14BasicSet {
return table_.find(key).atEnd() ? 0 : 1; return table_.find(key).atEnd() ? 0 : 1;
} }
/// prehash(key) does the work of evaluating hash_function()(key) // prehash(key) does the work of evaluating hash_function()(key)
/// (including additional bit-mixing for non-avalanching hash functions), // (including additional bit-mixing for non-avalanching hash functions),
/// wraps the result of that work in a token for later reuse, and // wraps the result of that work in a token for later reuse, and
/// begins prefetching of the first steps of looking for key into the // begins prefetching of the first steps of looking for key into the
/// local CPU cache. // local CPU cache.
/// //
/// The returned token may be used at any time, may be used more than // The returned token may be used at any time, may be used more than
/// once, and may be used in other F14 sets and maps. Tokens are // once, and may be used in other F14 sets and maps. Tokens are
/// transferrable between any F14 containers (maps and sets) with the // transferrable between any F14 containers (maps and sets) with the
/// same key_type and equal hash_function()s. // same key_type and equal hash_function()s.
/// //
/// Hash tokens are not hints -- it is a bug to call any method on this // Hash tokens are not hints -- it is a bug to call any method on this
/// class with a token t and key k where t isn't the result of a call // class with a token t and key k where t isn't the result of a call
/// to prehash(k2) with k2 == k. // to prehash(k2) with k2 == k.
F14HashToken prehash(key_type const& key) const { F14HashToken prehash(key_type const& key) const {
return table_.prehash(key); return table_.prehash(key);
} }
...@@ -645,6 +667,7 @@ class F14VectorSet ...@@ -645,6 +667,7 @@ class F14VectorSet
using typename Super::const_iterator; using typename Super::const_iterator;
using typename Super::iterator; using typename Super::iterator;
using typename Super::key_type; using typename Super::key_type;
using typename Super::value_type;
using reverse_iterator = typename Policy::ReverseIter; using reverse_iterator = typename Policy::ReverseIter;
using const_reverse_iterator = typename Policy::ConstReverseIter; using const_reverse_iterator = typename Policy::ConstReverseIter;
...@@ -737,14 +760,17 @@ class F14VectorSet ...@@ -737,14 +760,17 @@ class F14VectorSet
} }
private: private:
void eraseUnderlying(typename Policy::ItemIter underlying) { template <typename BeforeDestroy>
void eraseUnderlying(
typename Policy::ItemIter underlying,
BeforeDestroy const& beforeDestroy) {
Alloc& a = this->table_.alloc(); Alloc& a = this->table_.alloc();
auto values = this->table_.values_; auto values = this->table_.values_;
// destroy the value and remove the ptr from the base table // destroy the value and remove the ptr from the base table
auto index = underlying.item(); auto index = underlying.item();
this->table_.eraseInto(underlying, beforeDestroy);
std::allocator_traits<Alloc>::destroy(a, std::addressof(values[index])); std::allocator_traits<Alloc>::destroy(a, std::addressof(values[index]));
this->table_.erase(underlying);
// move the last element in values_ down and fix up the inbound index // move the last element in values_ down and fix up the inbound index
auto tailIndex = this->size(); auto tailIndex = this->size();
...@@ -760,25 +786,46 @@ class F14VectorSet ...@@ -760,25 +786,46 @@ class F14VectorSet
public: public:
FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) { FOLLY_ALWAYS_INLINE iterator erase(const_iterator pos) {
return eraseInto(pos, [](value_type&) {});
}
iterator erase(const_iterator first, const_iterator last) {
return eraseInto(first, last, [](value_type&) {});
}
std::size_t erase(key_type const& key) {
return eraseInto(key, [](value_type&) {});
}
template <typename BeforeDestroy>
FOLLY_ALWAYS_INLINE iterator
eraseInto(const_iterator pos, BeforeDestroy const& beforeDestroy) {
auto underlying = this->table_.find( auto underlying = this->table_.find(
f14::detail::VectorContainerIndexSearch{this->table_.iterToIndex(pos)}); f14::detail::VectorContainerIndexSearch{this->table_.iterToIndex(pos)});
eraseUnderlying(underlying); eraseUnderlying(underlying, beforeDestroy);
return ++pos; return ++pos;
} }
iterator erase(const_iterator first, const_iterator last) { template <typename BeforeDestroy>
iterator eraseInto(
const_iterator first,
const_iterator last,
BeforeDestroy const& beforeDestroy) {
while (first != last) { while (first != last) {
first = erase(first); first = eraseInto(first, beforeDestroy);
} }
return first; return first;
} }
std::size_t erase(key_type const& key) { template <typename BeforeDestroy>
std::size_t eraseInto(
key_type const& key,
BeforeDestroy const& beforeDestroy) {
auto underlying = this->table_.find(key); auto underlying = this->table_.find(key);
if (underlying.atEnd()) { if (underlying.atEnd()) {
return 0; return 0;
} else { } else {
eraseUnderlying(underlying); eraseUnderlying(underlying, beforeDestroy);
return 1; return 1;
} }
} }
......
...@@ -406,6 +406,10 @@ class ValueContainerPolicy : public BasePolicy< ...@@ -406,6 +406,10 @@ class ValueContainerPolicy : public BasePolicy<
return item; return item;
} }
Value& valueAtItemForExtract(Item& item) {
return item;
}
template <typename... Args> template <typename... Args>
void void
constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) { constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) {
...@@ -649,6 +653,10 @@ class NodeContainerPolicy ...@@ -649,6 +653,10 @@ class NodeContainerPolicy
return *item; return *item;
} }
Value& valueAtItemForExtract(Item& item) {
return *item;
}
template <typename... Args> template <typename... Args>
void void
constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) { constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) {
...@@ -919,6 +927,10 @@ class VectorContainerPolicy : public BasePolicy< ...@@ -919,6 +927,10 @@ class VectorContainerPolicy : public BasePolicy<
return {item}; return {item};
} }
Value& valueAtItemForExtract(Item& item) {
return values_[item];
}
void constructValueAtItem( void constructValueAtItem(
std::size_t /*size*/, std::size_t /*size*/,
Item* itemAddr, Item* itemAddr,
......
...@@ -1557,33 +1557,39 @@ class F14Table : public Policy { ...@@ -1557,33 +1557,39 @@ class F14Table : public Policy {
public: public:
// The item needs to still be hashable during this call. If you want // The item needs to still be hashable during this call. If you want
// to intercept the item before it is destroyed (to extract it, for // to intercept the value before it is destroyed (to extract it, for
// example), use erase(pos, beforeDestroy). // example), use eraseInto(pos, beforeDestroy).
void erase(ItemIter pos) {
eraseInto(pos, [](value_type&) {});
}
// The item needs to still be hashable during this call. If you want
// to intercept the value before it is destroyed (to extract it, for
// example), do so in the beforeDestroy callback.
template <typename BeforeDestroy> template <typename BeforeDestroy>
void erase(ItemIter pos, BeforeDestroy const& beforeDestroy) { void eraseInto(ItemIter pos, BeforeDestroy const& beforeDestroy) {
HashPair hp{}; HashPair hp{};
if (pos.chunk()->hostedOverflowCount() != 0) { if (pos.chunk()->hostedOverflowCount() != 0) {
hp = splitHash(this->computeItemHash(pos.citem())); hp = splitHash(this->computeItemHash(pos.citem()));
} }
beforeDestroy(pos.item()); beforeDestroy(this->valueAtItemForExtract(pos.item()));
eraseImpl(pos, hp); eraseImpl(pos, hp);
} }
// The item needs to still be hashable during this call. If you want
// to intercept the item before it is destroyed (to extract it, for
// example), use erase(pos, beforeDestroy).
void erase(ItemIter pos) {
return erase(pos, [](Item const&) {});
}
template <typename K> template <typename K>
std::size_t erase(K const& key) { std::size_t erase(K const& key) {
return eraseInto(key, [](value_type&) {});
}
template <typename K, typename BeforeDestroy>
std::size_t eraseInto(K const& key, BeforeDestroy const& beforeDestroy) {
if (UNLIKELY(size() == 0)) { if (UNLIKELY(size() == 0)) {
return 0; return 0;
} }
auto hp = splitHash(this->computeKeyHash(key)); auto hp = splitHash(this->computeKeyHash(key));
auto iter = findImpl(hp, key); auto iter = findImpl(hp, key);
if (!iter.atEnd()) { if (!iter.atEnd()) {
beforeDestroy(this->valueAtItemForExtract(iter.item()));
eraseImpl(iter, hp); eraseImpl(iter, hp);
return 1; return 1;
} else { } else {
......
...@@ -519,6 +519,62 @@ TEST(F14FastSet, moveOnly) { ...@@ -519,6 +519,62 @@ TEST(F14FastSet, moveOnly) {
runMoveOnlyTest<F14FastSet<f14::MoveOnlyTestInt>>(); runMoveOnlyTest<F14FastSet<f14::MoveOnlyTestInt>>();
} }
template <typename S>
void runEraseIntoTest() {
S t0;
S t1;
auto insertIntoT0 = [&t0](auto& value) {
EXPECT_FALSE(value.destroyed);
t0.emplace(std::move(value));
};
t0.insert(10);
t1.insert(20);
t1.eraseInto(t1.begin(), insertIntoT0);
EXPECT_TRUE(t1.empty());
EXPECT_EQ(t0.size(), 2);
EXPECT_TRUE(t0.find(10) != t0.end());
EXPECT_TRUE(t0.find(20) != t0.end());
t1.insert(20);
t1.insert(30);
t1.insert(40);
t1.eraseInto(t1.begin(), t1.end(), insertIntoT0);
EXPECT_TRUE(t1.empty());
EXPECT_EQ(t0.size(), 4);
EXPECT_TRUE(t0.find(30) != t0.end());
EXPECT_TRUE(t0.find(40) != t0.end());
t1.insert(50);
size_t erased = t1.eraseInto(*t1.find(50), insertIntoT0);
EXPECT_EQ(erased, 1);
EXPECT_TRUE(t1.empty());
EXPECT_EQ(t0.size(), 5);
EXPECT_TRUE(t0.find(50) != t0.end());
typename S::value_type key{60};
erased = t1.eraseInto(key, insertIntoT0);
EXPECT_EQ(erased, 0);
EXPECT_EQ(t0.size(), 5);
}
TEST(F14ValueSet, eraseInto) {
runEraseIntoTest<F14ValueSet<f14::MoveOnlyTestInt>>();
}
TEST(F14NodeSet, eraseInto) {
runEraseIntoTest<F14NodeSet<f14::MoveOnlyTestInt>>();
}
TEST(F14VectorSet, eraseInto) {
runEraseIntoTest<F14VectorSet<f14::MoveOnlyTestInt>>();
}
TEST(F14FastSet, eraseInto) {
runEraseIntoTest<F14FastSet<f14::MoveOnlyTestInt>>();
}
TEST(F14ValueSet, heterogeneous) { TEST(F14ValueSet, heterogeneous) {
// note: std::string is implicitly convertible to but not from StringPiece // note: std::string is implicitly convertible to but not from StringPiece
using Hasher = folly::transparent<folly::hasher<folly::StringPiece>>; using Hasher = folly::transparent<folly::hasher<folly::StringPiece>>;
......
...@@ -90,6 +90,7 @@ std::size_t p99Probe(std::vector<std::size_t> const& probeLengths) { ...@@ -90,6 +90,7 @@ std::size_t p99Probe(std::vector<std::size_t> const& probeLengths) {
struct MoveOnlyTestInt { struct MoveOnlyTestInt {
int x; int x;
bool destroyed{false};
MoveOnlyTestInt() noexcept : x(0) {} MoveOnlyTestInt() noexcept : x(0) {}
/* implicit */ MoveOnlyTestInt(int x0) : x(x0) {} /* implicit */ MoveOnlyTestInt(int x0) : x(x0) {}
...@@ -97,12 +98,17 @@ struct MoveOnlyTestInt { ...@@ -97,12 +98,17 @@ struct MoveOnlyTestInt {
MoveOnlyTestInt(MoveOnlyTestInt const&) = delete; MoveOnlyTestInt(MoveOnlyTestInt const&) = delete;
MoveOnlyTestInt& operator=(MoveOnlyTestInt&& rhs) noexcept { MoveOnlyTestInt& operator=(MoveOnlyTestInt&& rhs) noexcept {
x = rhs.x; x = rhs.x;
destroyed = rhs.destroyed;
return *this; return *this;
} }
MoveOnlyTestInt& operator=(MoveOnlyTestInt const&) = delete; MoveOnlyTestInt& operator=(MoveOnlyTestInt const&) = delete;
~MoveOnlyTestInt() {
destroyed = true;
}
bool operator==(MoveOnlyTestInt const& rhs) const { bool operator==(MoveOnlyTestInt const& rhs) const {
return x == rhs.x; return x == rhs.x && destroyed == rhs.destroyed;
} }
bool operator!=(MoveOnlyTestInt const& rhs) const { bool operator!=(MoveOnlyTestInt const& rhs) const {
return !(*this == rhs); return !(*this == rhs);
......
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