From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90244 invoked by alias); 3 Jul 2015 08:56:05 -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 90234 invoked by uid 89); 3 Jul 2015 08:56:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f175.google.com Received: from mail-wi0-f175.google.com (HELO mail-wi0-f175.google.com) (209.85.212.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 03 Jul 2015 08:56:03 +0000 Received: by wiar9 with SMTP id r9so124526753wia.1 for ; Fri, 03 Jul 2015 01:56:01 -0700 (PDT) X-Received: by 10.180.8.68 with SMTP id p4mr1847362wia.27.1435913760986; Fri, 03 Jul 2015 01:56:00 -0700 (PDT) Received: from localhost ([95.144.14.193]) by mx.google.com with ESMTPSA id ny7sm31454801wic.11.2015.07.03.01.55.59 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jul 2015 01:55:59 -0700 (PDT) From: Richard Sandiford To: Trevor Saunders Mail-Followup-To: Trevor Saunders ,Martin =?utf-8?Q?Li=C5=A1ka?= , GCC Patches , Martin Jambor , rdsandiford@googlemail.com Cc: Martin =?utf-8?Q?Li=C5=A1ka?= , GCC Patches , Martin Jambor Subject: Re: [RFC, PATCH] Split pool_allocator and create a new object_allocator References: <55914DB1.9000700@suse.cz> <87mvze724k.fsf@googlemail.com> <20150702210838.GB4274@tsaunders-iceball.corp.tor1.mozilla.com> Date: Fri, 03 Jul 2015 08:56:00 -0000 In-Reply-To: <20150702210838.GB4274@tsaunders-iceball.corp.tor1.mozilla.com> (Trevor Saunders's message of "Thu, 2 Jul 2015 17:08:38 -0400") Message-ID: <87lhex8vs1.fsf@googlemail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-07/txt/msg00184.txt.bz2 Trevor Saunders writes: > On Thu, Jul 02, 2015 at 09:09:31PM +0100, Richard Sandiford wrote: >> Martin Li=C5=A1ka 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 (); >> > } >> >=20=20 >> > /* Delete operator utilizing pool allocation. */ >> > inline void operator delete (void *ptr) >> > { >> > - pool.remove ((asan_mem_ref *) ptr); >> > + pool.remove (ptr); >> > } >> >=20=20 >> > /* Memory allocation pool. */ >> > - static pool_allocator pool; >> > + static pool_allocator pool; >> > }; >>=20 >> 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.rem= ove >> 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. > However it seems kind of wierd the operator new here is calling the > placement new on the object it allocates. Yeah. >> 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 (by saying that for these types you should use new and delete, but for other pool-allocated types you should use object_allocators). 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. Thanks, Richard