On 07/03/2015 03:07 PM, Richard Sandiford wrote:
> Martin Jambor <mjam...@suse.cz> writes:
>> On Fri, Jul 03, 2015 at 09:55:58AM +0100, Richard Sandiford wrote:
>>> Trevor Saunders <tbsau...@tbsaunde.org> writes:
>>>> On Thu, Jul 02, 2015 at 09:09:31PM +0100, Richard Sandiford wrote:
>>>>> Martin Liška <mli...@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>
&)
+ Pool allocators connected to these classes will be back transformed to static
variables and
one would call new et_forest (my_et_forest_allocator)
Please confirm that aforementioned changes make sense and I'll come up with
patch.
Thanks,
Martin
