This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Rb_tree constructor optimization
- From: François Dumont <frs dot dumont at gmail dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 13 Sep 2017 21:57:25 +0200
- Subject: Re: Rb_tree constructor optimization
- Authentication-results: sourceware.org; auth=none
- References: <4026eacd-cb49-e756-c855-627cd5a40206@gmail.com> <20170908155011.GE4582@redhat.com>
On 08/09/2017 17:50, Jonathan Wakely wrote:
Since we know __a == __x.get_allocator() we could just do:
_Rb_tree(_Rb_tree&& __x, _Node_allocator&&, true_type)
noexcept(is_nothrow_move_constructible<_Rb_tree_impl<_Compare>>::value)
: _M_impl(std::move(__x._M_impl))
{ }
This means we don't need the new constructor.
You want to consider that a always equal allocator is stateless and
so that the provided allocator rvalue reference do not need to be moved.
IMHO you can have allocator with state that do not participate in
comparison like some monitoring info.
I'm not confortable with that and prefer to keep current behavior
so I propose this new patch considering all your other remarks.
I change noexcept qualification on [multi]map/set constructors to
just rely on _Rep_type constructor noexcept qualification to not
duplicate it.
François
diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h
index 0e8a98a..cdd2e7c 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -235,8 +235,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/// Allocator-extended move constructor.
map(map&& __m, const allocator_type& __a)
- noexcept(is_nothrow_copy_constructible<_Compare>::value
- && _Alloc_traits::_S_always_equal())
+ noexcept( noexcept(
+ _Rep_type(declval<_Rep_type>(), declval<_Pair_alloc_type>())) )
: _M_t(std::move(__m._M_t), _Pair_alloc_type(__a)) { }
/// Allocator-extended initialier-list constructor.
diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h
index 7e3cea4..d32104d 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -232,8 +232,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/// Allocator-extended move constructor.
multimap(multimap&& __m, const allocator_type& __a)
- noexcept(is_nothrow_copy_constructible<_Compare>::value
- && _Alloc_traits::_S_always_equal())
+ noexcept( noexcept(
+ _Rep_type(declval<_Rep_type>(), declval<_Pair_alloc_type>())) )
: _M_t(std::move(__m._M_t), _Pair_alloc_type(__a)) { }
/// Allocator-extended initialier-list constructor.
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index 517e77e..9ab4ab7 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -244,8 +244,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/// Allocator-extended move constructor.
multiset(multiset&& __m, const allocator_type& __a)
- noexcept(is_nothrow_copy_constructible<_Compare>::value
- && _Alloc_traits::_S_always_equal())
+ noexcept( noexcept(
+ _Rep_type(declval<_Rep_type>(), declval<_Key_alloc_type>())) )
: _M_t(std::move(__m._M_t), _Key_alloc_type(__a)) { }
/// Allocator-extended initialier-list constructor.
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index e804a7c..6b64bcd 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -248,8 +248,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/// Allocator-extended move constructor.
set(set&& __x, const allocator_type& __a)
- noexcept(is_nothrow_copy_constructible<_Compare>::value
- && _Alloc_traits::_S_always_equal())
+ noexcept( noexcept(
+ _Rep_type(declval<_Rep_type>(), declval<_Key_alloc_type>())) )
: _M_t(std::move(__x._M_t), _Key_alloc_type(__a)) { }
/// Allocator-extended initialier-list constructor.
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index c2417f1..7aacc24 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -704,6 +704,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#else
_Rb_tree_impl(_Rb_tree_impl&&) = default;
+ _Rb_tree_impl(_Rb_tree_impl&& __x, _Node_allocator&& __a)
+ : _Node_allocator(std::move(__a)),
+ _Base_key_compare(std::move(__x)),
+ _Rb_tree_header(std::move(__x))
+ { }
+
_Rb_tree_impl(const _Key_compare& __comp, _Node_allocator&& __a)
: _Node_allocator(std::move(__a)), _Base_key_compare(__comp)
{ }
@@ -947,7 +953,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _Rb_tree(std::move(__x), _Node_allocator(__a))
{ }
- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a);
+ private:
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, true_type)
+ : _M_impl(std::move(__x._M_impl), std::move(__a))
+ { }
+
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, false_type)
+ : _M_impl(__x._M_impl._M_key_compare, std::move(__a))
+ {
+ if (__x._M_root() != nullptr)
+ _M_move_data(__x, false_type{});
+ }
+
+ public:
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a)
+ noexcept(is_nothrow_move_constructible<_Rb_tree_impl<_Compare>>::value
+ && _Alloc_traits::_S_always_equal())
+ : _Rb_tree(std::move(__x), std::move(__a),
+ typename _Alloc_traits::is_always_equal{})
+ { }
#endif
~_Rb_tree() _GLIBCXX_NOEXCEPT
@@ -1347,22 +1371,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
private:
// Move elements from container with equal allocator.
void
- _M_move_data(_Rb_tree& __x, std::true_type)
+ _M_move_data(_Rb_tree& __x, true_type)
{ _M_impl._M_move_data(__x._M_impl); }
// Move elements from container with possibly non-equal allocator,
// which might result in a copy not a move.
void
- _M_move_data(_Rb_tree&, std::false_type);
+ _M_move_data(_Rb_tree&, false_type);
// Move assignment from container with equal allocator.
void
- _M_move_assign(_Rb_tree&, std::true_type);
+ _M_move_assign(_Rb_tree&, true_type);
// Move assignment from container with possibly non-equal allocator,
// which might result in a copy not a move.
void
- _M_move_assign(_Rb_tree&, std::false_type);
+ _M_move_assign(_Rb_tree&, false_type);
#endif
#if __cplusplus > 201402L
@@ -1590,23 +1614,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if __cplusplus >= 201103L
template<typename _Key, typename _Val, typename _KeyOfValue,
typename _Compare, typename _Alloc>
- _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a)
- : _M_impl(__x._M_impl._M_key_compare, std::move(__a))
- {
- using __eq = typename _Alloc_traits::is_always_equal;
- if (__x._M_root() != nullptr)
- _M_move_data(__x, __eq());
- }
-
- template<typename _Key, typename _Val, typename _KeyOfValue,
- typename _Compare, typename _Alloc>
void
_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
- _M_move_data(_Rb_tree& __x, std::false_type)
+ _M_move_data(_Rb_tree& __x, false_type)
{
if (_M_get_Node_allocator() == __x._M_get_Node_allocator())
- _M_move_data(__x, std::true_type());
+ _M_move_data(__x, true_type());
else
{
_Alloc_node __an(*this);
@@ -1628,7 +1641,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
clear();
if (__x._M_root() != nullptr)
- _M_move_data(__x, std::true_type());
+ _M_move_data(__x, true_type());
std::__alloc_on_move(_M_get_Node_allocator(),
__x._M_get_Node_allocator());
}