apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1788334 - in /apr/apr/trunk: CHANGES include/apr_allocator.h memory/unix/apr_pools.c
Date Tue, 28 Mar 2017 12:39:37 GMT
On 28 March 2017 at 01:02, Yann Ylavic <ylavic.dev@gmail.com> wrote:
> On Mon, Mar 27, 2017 at 3:55 PM, Ivan Zhakov <ivan@visualsvn.com> wrote:
>> On 24 March 2017 at 00:34,  <ylavic@apache.org> wrote:
>>> +
>>> +/**
>>> + * Return the aligned (round up) size that an allocator would use for
>>> + * the given size.
>>> + * @param size The size to align
>>> + * @return The aligned size (or zero on apr_size_t overflow)
>>> + */
>>> +APR_DECLARE(apr_size_t) apr_allocator_align(apr_size_t size);
>> What is the purpose of this API? Currently caller may use
>> apr_memnode_t.endp to find actually allocated memory size.
>
> The purpose is to get this size before allocating any memory.
> It is (newly) used in APR by apr_bucket_alloc_aligned_floor (itself
> used by apr_bucket_file_set_buf_size), where the purpose is to take
> into account and be precise with the overhead of the
> (bucket_)allocator(s), and avoid over-allocating (wasted) memory.
Hi Yann, thanks for explanation!

>
>>
>> Anyway I think it maybe worth to add apr_allocator_t argument to
>> apr_allocator_align() function even currently allocation alignment is
>> the same for all allocators.
>
> I see your point, but since there is no way to do this yet and that'd
> require another API change, maybe we could do this at that time?
> If you are concerned about the current name which would better fit an
> allocator argument, let's use apr_allocator_default_align() for now...
> or e.g. apr_allocator_align_of() later ;)
>
> Hmm, this makes me think that if apr_allocator_align() need an
> alloctor, apr_bucket_alloc_aligned_floor() needs a bucket_alloc too,
> and both functions may be less useful (or easy to use) if they require
> the allocator the memory will finally be allocated on (say
> apr_allocator_align() has to be used at the init time of an
> application where the runtime allocator is not created yet).
>
> Also why would one want a different allocator alignment since (s)he
> can apr_allocator_alloc() any size in the first place? The alignment
> looks more like a property of the system than the allocator itself for
> me...
I'm not sure that *actual* allocation size could be predicted in all
cases and possible apr_allocator_t implementation. Currently
apr_bucket_alloc_aligned_floor() and apr_bucket_alloc() logic should
be kept in sync, which is error prone imho.

After looking to apr_bucket_alloc code I suggest to add
apr_bucket_node_size(const void *node) that returns actual allocated
node size. apr_file_bucket implementation could allocate desired
buffer size and then query actual allocated size via
apr_bucket_node_size. Apache Serf bucket_allocator uses similar
approach.

-- 
Ivan Zhakov

Mime
View raw message