This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC, PATCH] Split pool_allocator and create a new object_allocator
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Martin LiÅka <mliska at suse dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 03 Jul 2015 17:18:57 +0100
- Subject: Re: [RFC, PATCH] Split pool_allocator and create a new object_allocator
- Authentication-results: sourceware.org; auth=none
- References: <55914DB1 dot 9000700 at suse dot cz> <87mvze724k dot fsf at googlemail dot com> <20150702210838 dot GB4274 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com> <87lhex8vs1 dot fsf at googlemail dot com> <20150703122234 dot GF2361 at virgil dot suse dot cz> <87h9pl8k4u dot fsf at googlemail dot com> <559698EB dot 6040708 at suse dot cz>
Hi Martin,
Martin LiÅka <mliska@suse.cz> writes:
> On 07/03/2015 03:07 PM, Richard Sandiford wrote:
>> Martin Jambor <mjambor@suse.cz> writes:
>>> On Fri, Jul 03, 2015 at 09:55:58AM +0100, Richard Sandiford wrote:
>>>> Trevor Saunders <tbsaunde@tbsaunde.org> writes:
>>>>> On Thu, Jul 02, 2015 at 09:09:31PM +0100, Richard Sandiford wrote:
>>>>>> Martin LiÅka <mliska@suse.cz> 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<asan_mem_ref> 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.
>>
>> But we're talking about two different methods. The "normal" allocator
>> object_allocator <T>::allocate () would use placement new and return a
>> pointer to the new object while operator new (size_t, object_allocator <T> &)
>> wouldn't call placement new and would just return a pointer to the memory.
>>
>>>>>> 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?
>>
>> Not IMO. Just that static load-time-initialized caches are not
>> necessarily a good thing. That's effectively what the pool
>> allocator is.
>>
>>>> (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.
>>
>> But how we handle argument-taking constructors is a problem that needs
>> to be solved for the pool-allocated objects that don't use a single
>> static type-specific pool. And once we solve that, we get consistency
>> across all pools:
>>
>> - if you want a new object and argumentless construction is OK,
>> use "pool.allocate ()"
>>
>> - if you want a new object and need to pass arguments to the constructor,
>> use "new (pool) some_type (arg1, arg2, ...)"
>>
>>>> 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?
>>
>> Sure, but either (a) you keep the pools as a static part of the class
>> and some initialisation and finalisation code that has tendrils into
>> all such classes or (b) you move the static pool outside of the
>> class to some new (still global) state. Explicit pool allocation,
>> like in the C days, gives you the option of putting the pool whereever
>> it needs to go without relying on the principle that you can get to
>> it from global state.
>>
>> Thanks,
>> Richard
>>
>
> Ok Richard.
>
> I've just finally understood your suggestions and I would suggest following:
>
> + I will add a new method to object_allocator<T> that will return an
> allocated memory (void*)
> (w/o calling any construction)
> + object_allocator<T>::allocate will call placement new with for a
> parameterless ctor
> + I will remove all overwritten operators new/delete on e.g. et_forest, ...
> + For these classes, I will add void* operator new (size_t,
> object_allocator<T> &)
I was thinking we'd simply use allocate () for cases where we don't
need to pass arguments to the constructor. It looks like et_forest
comes into that category. The operator new would be a single function
defined in pool-allocator.h for cases where explicit construction
is needed.
In fact, it looks from a quick grep like all current uses of pool operator
new/delete are in POD types, so there are no special constructors.
The best example I could come up with was the copy constructor in:
return new lra_live_range (*r);
which would become:
return new (*live_range_pool) lra_live_range (*r);
but perhaps we should have an object_allocator copy (T *) routine:
return live_range_pool->copy (*r);
> + Pool allocators connected to these classes will be back transformed to
> static variables and
> one would call new et_forest (my_et_forest_allocator)
Thanks, this sounds really good to me. Please make sure I'm not the
only one who thinks so though :-)
I think the "normal" remove () method should then also call the destructor.
Thanks,
Richard