Commit fd30521a authored by Nick Terrell's avatar Nick Terrell Committed by Facebook Github Bot

Remove unnecessary copy in sorted_vector_types insert with hint

Summary:
When inserting an item with a hint, the item will be always copied, even
if it is already present in the map. This could cause an exception to be
thrown when it doesn't need to be. The standard doesn't require this, but
it is an optimization anyways.

Reviewed By: yfeldblum

Differential Revision: D16585971

fbshipit-source-id: f0a8eeb0bd86f07ef110f91cbef8ab422ff55480
parent 4bbc9cd2
......@@ -131,29 +131,29 @@ int distance_if_multipass(Iterator first, Iterator last) {
return std::distance(first, last);
}
template <class OurContainer, class Vector, class GrowthPolicy>
template <class OurContainer, class Vector, class GrowthPolicy, class Value>
typename OurContainer::iterator insert_with_hint(
OurContainer& sorted,
Vector& cont,
typename OurContainer::const_iterator hint,
typename OurContainer::value_type&& value,
Value&& value,
GrowthPolicy& po) {
const typename OurContainer::value_compare& cmp(sorted.value_comp());
if (hint == cont.end() || cmp(value, *hint)) {
if (hint == cont.begin() || cmp(*(hint - 1), value)) {
hint = po.increase_capacity(cont, hint);
return cont.insert(hint, std::move(value));
return cont.insert(hint, std::forward<Value>(value));
} else {
return sorted.insert(std::move(value)).first;
return sorted.insert(std::forward<Value>(value)).first;
}
}
if (cmp(*hint, value)) {
if (hint + 1 == cont.end() || cmp(value, *(hint + 1))) {
hint = po.increase_capacity(cont, hint + 1);
return cont.insert(hint, std::move(value));
return cont.insert(hint, std::forward<Value>(value));
} else {
return sorted.insert(std::move(value)).first;
return sorted.insert(std::forward<Value>(value)).first;
}
}
......@@ -430,7 +430,8 @@ class sorted_vector_set : detail::growth_policy_wrapper<GrowthPolicy> {
}
iterator insert(const_iterator hint, const value_type& value) {
return insert(hint, std::move(value_type(value)));
return detail::insert_with_hint(
*this, m_.cont_, hint, value, get_growth_policy());
}
iterator insert(const_iterator hint, value_type&& value) {
......@@ -884,7 +885,8 @@ class sorted_vector_map : detail::growth_policy_wrapper<GrowthPolicy> {
}
iterator insert(const_iterator hint, const value_type& value) {
return insert(hint, std::move(value_type(value)));
return detail::insert_with_hint(
*this, m_.cont_, hint, value, get_growth_policy());
}
iterator insert(const_iterator hint, value_type&& value) {
......
......@@ -63,12 +63,14 @@ struct OneAtATimePolicy {
};
struct CountCopyCtor {
explicit CountCopyCtor() : val_(0) {}
explicit CountCopyCtor() : val_(0), count_(0) {}
explicit CountCopyCtor(int val) : val_(val), count_(0) {}
CountCopyCtor(const CountCopyCtor& c) noexcept
: val_(c.val_), count_(c.count_ + 1) {}
: val_(c.val_), count_(c.count_ + 1) {
++gCount_;
}
bool operator<(const CountCopyCtor& o) const {
return val_ < o.val_;
......@@ -76,8 +78,11 @@ struct CountCopyCtor {
int val_;
int count_;
static int gCount_;
};
int CountCopyCtor::gCount_ = 0;
struct Opaque {
int value;
friend bool operator==(Opaque a, Opaque b) {
......@@ -877,3 +882,29 @@ TEST(SortedVectorTypes, TestPmrAllocatorScoped) {
}
#endif
TEST(SortedVectorTypes, TestInsertHintCopy) {
sorted_vector_set<CountCopyCtor> set;
sorted_vector_map<CountCopyCtor, int> map;
CountCopyCtor skey;
std::pair<CountCopyCtor, int> mkey;
CountCopyCtor::gCount_ = 0;
set.insert(set.end(), skey);
map.insert(map.end(), mkey);
EXPECT_EQ(CountCopyCtor::gCount_, 2);
set.emplace(CountCopyCtor(1));
map.emplace(CountCopyCtor(1), 1);
CountCopyCtor::gCount_ = 0;
for (size_t i = 0; i <= set.size(); ++i) {
auto sit = set.begin();
auto mit = map.begin();
std::advance(sit, i);
std::advance(mit, i);
set.insert(sit, skey);
map.insert(mit, mkey);
}
EXPECT_EQ(CountCopyCtor::gCount_, 0);
}
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