From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66916 invoked by alias); 3 Jul 2015 12:22:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 66898 invoked by uid 89); 3 Jul 2015 12:22:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 03 Jul 2015 12:22:38 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 37C93AC63; Fri, 3 Jul 2015 12:22:35 +0000 (UTC) Date: Fri, 03 Jul 2015 12:22:00 -0000 From: Martin Jambor To: Trevor Saunders , Martin =?utf-8?B?TGnFoWth?= , GCC Patches , rdsandiford@googlemail.com Subject: Re: [RFC, PATCH] Split pool_allocator and create a new object_allocator Message-ID: <20150703122234.GF2361@virgil.suse.cz> Mail-Followup-To: Trevor Saunders , Martin =?utf-8?B?TGnFoWth?= , GCC Patches , rdsandiford@googlemail.com References: <55914DB1.9000700@suse.cz> <87mvze724k.fsf@googlemail.com> <20150702210838.GB4274@tsaunders-iceball.corp.tor1.mozilla.com> <87lhex8vs1.fsf@googlemail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87lhex8vs1.fsf@googlemail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00197.txt.bz2 Hi, On Fri, Jul 03, 2015 at 09:55:58AM +0100, Richard Sandiford wrote: > Trevor Saunders writes: > > On Thu, Jul 02, 2015 at 09:09:31PM +0100, Richard Sandiford wrote: > >> Martin Liška writes: > >> > diff --git a/gcc/asan.c b/gcc/asan.c > >> > index e89817e..dabd6f1 100644 > >> > --- a/gcc/asan.c > >> > +++ b/gcc/asan.c > >> > @@ -362,20 +362,20 @@ struct asan_mem_ref > >> > /* Pool allocation new operator. */ > >> > inline void *operator new (size_t) > >> > { > >> > - return pool.allocate (); > >> > + return ::new (pool.allocate ()) asan_mem_ref (); > >> > } > >> > > >> > /* Delete operator utilizing pool allocation. */ > >> > inline void operator delete (void *ptr) > >> > { > >> > - pool.remove ((asan_mem_ref *) ptr); > >> > + pool.remove (ptr); > >> > } > >> > > >> > /* Memory allocation pool. */ > >> > - static pool_allocator pool; > >> > + static pool_allocator pool; > >> > }; > >> > >> I'm probably going over old ground/wounds, sorry, but what's the benefit > >> of having this sort of pattern? Why not simply have object_allocators > >> and make callers use pool.allocate () and pool.remove (x) (with pool.remove > >> calling the destructor) instead of new and delete? It feels wrong to me > >> to tie the data type to a particular allocation object like this. > > > > Well the big question is what does allocate() do about construction? if > > it seems wierd for it to not call the ctor, but I'm not sure we can do a > > good job of forwarding args to allocate() with C++98. > > If you need non-default constructors then: > > new (pool) type (aaa, bbb)...; > > doesn't seem too bad. I agree object_allocator's allocate () should call > the constructor. > but then the pool allocator must not call placement new on the allocated memory itself because that would result in double construction. And calling placement new was explicitely requested in the previous thread about allocators, so we still need two kinds of allocators, typed and untyped. Or just the untyped allocators and requiring that users construct their objects via placement new. In fact, they might have to call placement new even if there is no constructor because of the weird aliasing issue. Two kinds of pool-allocators seem the lesser evil to me. > >> And using the pool allocator functions directly has the nice property > >> that you can tell when a delete/remove isn't necessary because the pool > >> itself is being cleared. > > > > Well, all these cases involve a pool with static storage lifetime right? > > so actually if you don't delete things in these pool they are > > effectively leaked. > > They might have a static storage lifetime now, but it doesn't seem like > a good idea to hard-bake that into the interface Does that mean that operators new and delete are considered evil? > (by saying that for > these types you should use new and delete, but for other pool-allocated > types you should use object_allocators). Depending on what kind of pool allocator you use, you will be forced to either call placement new or not, so the inconsistency will be there anyway. I'm using pool allocators for classes with non-default constructors a lot in the HSA branch so I'd appreciate an early settlement of this issue. I think I slightly prefer overloading new and delete to using placement new (at least in new code) because then users just allocate stuff as usual and there is one central point where thing can be changed. But I do not have strong feelings and will comply with whatever we can agree on. > Maybe I just have bad memories > from doing the SWITCHABLE_TARGET stuff, but there I was changing a lot > of state that was "obviously" static in the old days, but that needed > to become non-static to support vaguely-efficient switching between > different subtargets. The same kind of thing is likely to happen again. > I assume things like the jit would prefer not to have new global state > with load-time construction. I'm not sure I follow this branch of the discussion, the allocators of any kind surely can dynamically allocated themselves? Thanks, Martin