From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17305 invoked by alias); 8 Sep 2017 15:50:15 -0000 Mailing-List: contact libstdc++-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libstdc++-owner@gcc.gnu.org Received: (qmail 17046 invoked by uid 89); 8 Sep 2017 15:50:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=HContent-Transfer-Encoding:8bit X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Sep 2017 15:50:13 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6166E750; Fri, 8 Sep 2017 15:50:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6166E750 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jwakely@redhat.com Received: from localhost (unknown [10.33.36.109]) by smtp.corp.redhat.com (Postfix) with ESMTP id DD4EA5C882; Fri, 8 Sep 2017 15:50:11 +0000 (UTC) Date: Fri, 08 Sep 2017 15:50:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: Rb_tree constructor optimization Message-ID: <20170908155011.GE4582@redhat.com> References: <4026eacd-cb49-e756-c855-627cd5a40206@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4026eacd-cb49-e756-c855-627cd5a40206@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.8.3 (2017-05-23) X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00012.txt.bz2 On 28/08/17 21:12 +0200, François Dumont wrote: >Hi > > Here is the always equal allocator optimization for associative >containers. > > Tested under Linux x86_64. > > * include/bits/stl_tree.h > (_Rb_tree_impl(_Rb_tree_impl&&, _Node_allocator&&)): New. > (_Rb_tree(_Rb_tree&&, _Node_allocator&&, std::true_type)): New. > (_Rb_tree(_Rb_tree&&, _Node_allocator&&, std::false_type)): Likewise. > (_Rb_tree(_Rb_tree&&, _Node_allocator&&)): Adapt, use latters. > > Ok to apply ? > >François > > >diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h >index c2417f1..f7d34e3 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) noexcept This is an unconditional noexcept, but ... >+ : _Node_allocator(std::move(__a)), >+ _Base_key_compare(std::move(__x)), This constructor has a conditional noexcept. That's a bug. >+ _Rb_tree_header(std::move(__x)) >+ { } However, I don't think we need this new constructor anyway, see below. > _Rb_tree_impl(const _Key_compare& __comp, _Node_allocator&& __a) > : _Node_allocator(std::move(__a)), _Base_key_compare(__comp) > { } >@@ -947,7 +953,18 @@ _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, std::true_type) noexcept This noexcept should be conditional. >+ : _M_impl(std::move(__x._M_impl), std::move(__a)) >+ { } 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. Or equivalently, just delegate to the move constructor: _Rb_tree(_Rb_tree&& __x, _Node_allocator&&, true_type) noexcept(is_nothrow_move_constructible<_Rb_tree>::value) : _Rb_tree(std::move(__x)) { } >+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, std::false_type); >+ >+ public: >+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a) >+ : _Rb_tree(std::move(__x), std::move(__a), >+ typename _Alloc_traits::is_always_equal{}) >+ { } > #endif > > ~_Rb_tree() _GLIBCXX_NOEXCEPT >@@ -1591,12 +1608,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > template typename _Compare, typename _Alloc> > _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>:: >- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a) >+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, std::false_type __eq) > : _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()); >+ _M_move_data(__x, __eq); > } I think this constructor is simple enough that it should be defined in-class, so it's inline. There's no need to qualify true_type and false_type with the std namespace (I don't know why some of the existing code does that).