Fix PR12999 - unordered_set::insert calls operator new when no insert occurs

Summary:
when `unordered_set::insert(value_type&&)` was called it would be treated like `unordered_set::emplace(Args&&)` and it would allocate and construct a node before trying to insert it.
This caused unnecessary allocations when the value was already in the set. This patch adds an overload to `__hash_table::__insert_unique` that specifically handles `value_type&&` more link `value_type const &`. 

This patch also adds a single unified insert function for values into  `__hash_table` called `__insert_unique_value` that handles the cases for `__insert_unique(value_type&&)` and `__insert_unique(value_type const &)`. 

This patch fixes PR12999: http://llvm.org/bugs/show_bug.cgi?id=12999.




Reviewers: mclow.lists, titus, danalbert

Reviewed By: danalbert

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D7570

git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@239666 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Eric Fiselier 2015-06-13 07:18:32 +00:00
parent 3a0e430cd4
commit fdae69aa13
2 changed files with 87 additions and 2 deletions

View File

@ -896,11 +896,21 @@ public:
iterator __emplace_hint_multi(const_iterator __p, _Args&&... __args);
#endif // !defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) && !defined(_LIBCPP_HAS_NO_VARIADICS)
#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
template <class _ValueTp>
_LIBCPP_INLINE_VISIBILITY
pair<iterator, bool> __insert_unique_value(_ValueTp&& __x);
#else
_LIBCPP_INLINE_VISIBILITY
pair<iterator, bool> __insert_unique_value(const value_type& __x);
#endif
pair<iterator, bool> __insert_unique(const value_type& __x);
#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
pair<iterator, bool> __insert_unique(value_type&& __x);
template <class _Pp>
pair<iterator, bool> __insert_unique(_Pp&& __x);
pair<iterator, bool> __insert_unique(_Pp&& __x);
#endif // _LIBCPP_HAS_NO_RVALUE_REFERENCES
#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
@ -1739,6 +1749,26 @@ template <class _Tp, class _Hash, class _Equal, class _Alloc>
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__insert_unique(const value_type& __x)
{
return __insert_unique_value(__x);
}
#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
template <class _Tp, class _Hash, class _Equal, class _Alloc>
template <class _ValueTp>
_LIBCPP_INLINE_VISIBILITY
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__insert_unique_value(_ValueTp&& __x)
#else
template <class _Tp, class _Hash, class _Equal, class _Alloc>
_LIBCPP_INLINE_VISIBILITY
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__insert_unique_value(const value_type& __x)
#endif
{
#if defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES)
typedef const value_type& _ValueTp;
#endif
size_t __hash = hash_function()(__x);
size_type __bc = bucket_count();
bool __inserted = false;
@ -1760,7 +1790,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__insert_unique(const value_type& __x)
}
}
{
__node_holder __h = __construct_node(__x, __hash);
__node_holder __h = __construct_node(_VSTD::forward<_ValueTp>(__x), __hash);
if (size()+1 > __bc * max_load_factor() || __bc == 0)
{
rehash(_VSTD::max<size_type>(2 * __bc + !__is_hash_power2(__bc),
@ -1843,6 +1873,13 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__emplace_hint_multi(
#endif // _LIBCPP_HAS_NO_VARIADICS
template <class _Tp, class _Hash, class _Equal, class _Alloc>
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>
__hash_table<_Tp, _Hash, _Equal, _Alloc>::__insert_unique(value_type&& __x)
{
return __insert_unique_value(_VSTD::move(__x));
}
template <class _Tp, class _Hash, class _Equal, class _Alloc>
template <class _Pp>
pair<typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator, bool>

View File

@ -0,0 +1,48 @@
//===----------------------------------------------------------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is dual licensed under the MIT and the University of Illinois Open
// Source Licenses. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
// Check that we don't allocate when trying to insert a duplicate value into a
// unordered_set. See PR12999 http://llvm.org/bugs/show_bug.cgi?id=12999
#include <cassert>
#include <unordered_set>
#include "count_new.hpp"
#include "MoveOnly.h"
int main()
{
{
std::unordered_set<int> s;
assert(globalMemCounter.checkNewCalledEq(0));
for(int i=0; i < 100; ++i)
s.insert(3);
assert(s.size() == 1);
assert(s.count(3) == 1);
assert(globalMemCounter.checkNewCalledEq(2));
}
assert(globalMemCounter.checkOutstandingNewEq(0));
globalMemCounter.reset();
#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
{
std::unordered_set<MoveOnly> s;
assert(globalMemCounter.checkNewCalledEq(0));
for(int i=0; i<100; i++)
s.insert(MoveOnly(3));
assert(s.size() == 1);
assert(s.count(MoveOnly(3)) == 1);
assert(globalMemCounter.checkNewCalledEq(2));
}
assert(globalMemCounter.checkOutstandingNewEq(0));
globalMemCounter.reset();
#endif
}