From patchwork Thu Jun 26 11:31:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 364497 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BD10F1400F0 for ; Thu, 26 Jun 2014 21:31:22 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; q=dns; s= default; b=j8vNhn6DanjPznZOkWbTyKwFlivx/+RCvYHK9O+5AnGrfoELVVp64 TzPGNLwz/8uVMQT84NyuzTmFnjjg5kjpEuo/UfRTyw0i4JchCxTBcoXHmjsx5o5N U35er0DZlnMSpsKqCVp1FzpTLn6qVgsQtR7NvR79ixyGM+3BmtvtQk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; s=default; bh=r7yrw0bNY9SZHzHemdOm8bvWGY8=; b=Pwj6pA3gKwHPfLnnY4IFKL5BMQef HXijEAiVGmewWjL5Vr8pqhgpj2y3cHAcxm/PWuIRbTbb4FFKuTpecbn1rHOaDZ2o DqOA7PmJ0J5YQei72ievygaBuBlbEk5xRvi7i6Xc/fNrW09zv9XGOKQnXMm1H77r RQJD9aJ1OUWalNc= Received: (qmail 5314 invoked by alias); 26 Jun 2014 11:31:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 5296 invoked by uid 89); 26 Jun 2014 11:31:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 26 Jun 2014 11:31:14 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5QBVCkE018295 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 26 Jun 2014 07:31:12 -0400 Received: from localhost (vpn1-4-174.ams2.redhat.com [10.36.4.174]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5QBVBmM029477; Thu, 26 Jun 2014 07:31:11 -0400 Date: Thu, 26 Jun 2014 12:31:10 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: =?iso-8859-1?Q?Fran=E7ois?= Dumont Subject: Re: [patch] Simplify allocator use Message-ID: <20140626113110.GG2711@redhat.com> References: <20140625205624.GD2711@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140625205624.GD2711@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) On 25/06/14 21:56 +0100, Jonathan Wakely wrote: >The other adds an RAII type to help manage pointers obtained from >allocators. The new type means I can remove several ugly try-catch >blocks that are all very similar in structure and have been bothering >me for some time. The new type also makes it trivial to support >allocators with fancy pointers, fixing long-standing (but not very >important) bugs in std::promise and std::shared_ptr. This patch applies the __allocated_ptr type to hashtable_policy.h to remove most explicit deallocation (yay!) The buckets are still allocated and deallocated manually, because __allocated_ptr only works for allocations of single objects, not arrays. As well as __allocated_ptr this change relies on two things: 1) the node type has a trivial destructor, so we don't actually need to call it, we can just reuse or release its storage. (See 3.8 [basic.life] p1) 2) allocator_traits::construct and allocator_traits::destroy can be used with an allocator that has a different value_type, so we don't need to create a rebound copy to destroy every element, we can just use the node-allocator. (See http://cplusplus.github.io/LWG/lwg-active.html#2218 which is Open, but I've discussed the issue with Howard, Pablo and others, and I think libc++ already relies on this assumption). François, could you check it, and let me know if you see anything wrong or have any comments? commit d2fd02daab715c79c766bc0a476d1d01da1fc305 Author: Jonathan Wakely Date: Thu Jun 26 12:28:56 2014 +0100 * include/bits/hashtable_policy.h (_ReuseOrAllocNode::operator()): Use __allocated_ptr. (_Hashtable_alloc::_M_allocate_node): Likewise. (_Hashtable_alloc::_M_deallocate_node): Likewise. diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 606fbab..ed6b2d7 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -31,6 +31,8 @@ #ifndef _HASHTABLE_POLICY_H #define _HASHTABLE_POLICY_H 1 +#include + namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -137,20 +139,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __node_type* __node = _M_nodes; _M_nodes = _M_nodes->_M_next(); __node->_M_nxt = nullptr; - __value_alloc_type __a(_M_h._M_node_allocator()); - __value_alloc_traits::destroy(__a, __node->_M_valptr()); - __try - { - __value_alloc_traits::construct(__a, __node->_M_valptr(), - std::forward<_Arg>(__arg)); - } - __catch(...) - { - __node->~__node_type(); - __node_alloc_traits::deallocate(_M_h._M_node_allocator(), - __node, 1); - __throw_exception_again; - } + auto& __a = _M_h._M_node_allocator(); + __node_alloc_traits::destroy(__a, __node->_M_valptr()); + __allocated_ptr<_NodeAlloc> __guard{__a, __node}; + __node_alloc_traits::construct(__a, __node->_M_valptr(), + std::forward<_Arg>(__arg)); + __guard = nullptr; return __node; } return _M_h._M_allocate_node(std::forward<_Arg>(__arg)); @@ -1947,33 +1941,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION typename _Hashtable_alloc<_NodeAlloc>::__node_type* _Hashtable_alloc<_NodeAlloc>::_M_allocate_node(_Args&&... __args) { - auto __nptr = __node_alloc_traits::allocate(_M_node_allocator(), 1); - __node_type* __n = std::__addressof(*__nptr); - __try - { - __value_alloc_type __a(_M_node_allocator()); - ::new ((void*)__n) __node_type; - __value_alloc_traits::construct(__a, __n->_M_valptr(), - std::forward<_Args>(__args)...); - return __n; - } - __catch(...) - { - __node_alloc_traits::deallocate(_M_node_allocator(), __nptr, 1); - __throw_exception_again; - } + auto& __a = _M_node_allocator(); + auto __guard = std::__allocate_guarded(__a); + __node_type* __n = __guard.get(); + ::new ((void*)__n) __node_type; + __node_alloc_traits::construct(__a, __n->_M_valptr(), + std::forward<_Args>(__args)...); + __guard = nullptr; + return __n; } template void _Hashtable_alloc<_NodeAlloc>::_M_deallocate_node(__node_type* __n) { - typedef typename __node_alloc_traits::pointer _Ptr; - auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__n); - __value_alloc_type __a(_M_node_allocator()); - __value_alloc_traits::destroy(__a, __n->_M_valptr()); - __n->~__node_type(); - __node_alloc_traits::deallocate(_M_node_allocator(), __ptr, 1); + static_assert(std::is_trivially_destructible<__node_type>::value, + "Nodes must not require non-trivial destruction"); + auto& __alloc = _M_node_allocator(); + __allocated_ptr<__node_alloc_type> __guard{__alloc, __n}; + __node_alloc_traits::destroy(__alloc, __n->_M_valptr()); } template