[PATCH] greener apr allocator

[Maxim Yegorushkin <ma...@gmail.com>]

Hi there,

I am using apr as a system layer in the development of a cross-platform
financial system in London. While using it there has been spotted an
opportunity to reduce the memory footprint of the allocator, to make it a
bit greener. Not to save a forest ;), however, to provide extra 16 bytes
(32-bit system) to the user from each apr_memnode_t allocation.

In a test run, a newly created pool is now able to accommodate 1015 8-byte
objects before requesting a new memory node, instead of 1013.

Summary of changes:

* There is a bug in several memory allocation functions in apr_pools.c,
  where memory request size is compared with free memory available from the
  node this way:

    /* If the active node has enough bytes left, use it. */
    if (size < (apr_size_t)(active->endp - active->first_avail)) {

  Thus, if size is 8 and the active node has 8 bytes left free, the strict
  comparison yields false. The comparisons changed to <=. This change
  provides extra 8 bytes of memory to the user (since the minimum allocation
  size is 8).

* apr_memnode_t structure cleaned up.

  apr_memnode_t::endp member has been removed. Instead, the value is
  calculated this way:

    static inline char* memnode_end(apr_memnode_t* node)
    {
        return (char*)node + (1 + node->index << BOUNDARY_INDEX);
    }

    static inline apr_size_t memnode_space_left(apr_memnode_t* node)
    {
        return memnode_end(node) - node->first_avail;
    }

  apr_memnode_t::free_index member is removed. The value is calculated this
  way:

    static inline apr_size_t memnode_free_index(apr_memnode_t* node)
    {
        return APR_ALIGN(memnode_space_left(node) + 1, BOUNDARY_SIZE)
            - BOUNDARY_SIZE >> BOUNDARY_INDEX;
    }

  Removing these two members yields another 8 extra bytes.

* allocator_alloc() has been refactored to eliminate race conditions. It
  used to hold the mutex while changing apr_allocator_t members, but not
  while reading.

* Some duplicate code was extracted into functions. apr_pool.c code is now
  63 lines shorter.

Please comment it.

--
Max

Re: [PATCH] greener apr allocator

[Joe Orton <jo...@redhat.com>]

On Fri, Jan 04, 2008 at 11:02:26AM +0000, Maxim Yegorushkin wrote:
> Summary of changes:
> 
> * There is a bug in several memory allocation functions in apr_pools.c,
>   where memory request size is compared with free memory available from the
>   node this way:
> 
>     /* If the active node has enough bytes left, use it. */
>     if (size < (apr_size_t)(active->endp - active->first_avail)) {
> 
>   Thus, if size is 8 and the active node has 8 bytes left free, the strict
>   comparison yields false. The comparisons changed to <=. This change
>   provides extra 8 bytes of memory to the user (since the minimum allocation
>   size is 8).

Good catch!  I've committed your changes which fix this and factor out 
the code to check it.

> * apr_memnode_t structure cleaned up.
> 
>   apr_memnode_t::endp member has been removed. Instead, the value is
>   calculated this way:
...

I'm a bit wary about this; it trades off CPU for memory, so I'm forced 
to ask: what's the performance impact?

> * allocator_alloc() has been refactored to eliminate race conditions. It
>   used to hold the mutex while changing apr_allocator_t members, but not
>   while reading.
>
> * Some duplicate code was extracted into functions. apr_pool.c code is now
>   63 lines shorter.

I can't quite follow all these changes because it's all in one big 
patch.  Any chance you could split it up into several 
functionally-separate patches?

Regards,

joe