You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ivan Novick <in...@greenplum.com> on 2010/03/17 23:18:50 UTC

apr_hash_set allocation failure behavior

Hello,

I am doing some testing with APR version 1.2.12

When adding to a hash table using apr_hash_set ... If memory can not be
allocated what is the expected behavior?

I am seeing apr_hash_set calls expand_array.

expand_array core dumps if memory can not be allocated.

Is this expected?  Is there a way to get an error code for a failed insert
to a table rather than a coredump?

Cheers,
Ivan


Re: apr_hash_set allocation failure behavior

Posted by Nick Kew <ni...@apache.org>.
On 17 Mar 2010, at 22:18, Ivan Novick wrote:

> Hello,
> 
> I am doing some testing with APR version 1.2.12
> 
> When adding to a hash table using apr_hash_set ... If memory can not be
> allocated what is the expected behavior?
> 
> I am seeing apr_hash_set calls expand_array.
> 
> expand_array core dumps if memory can not be allocated.
> 
> Is this expected?  Is there a way to get an error code for a failed insert
> to a table rather than a coredump?

There's a kind-of half-excuse for that.  Namely, if you're out of memory,
you're already in so much trouble segfault is neither here nor there.

But of course it's bad.  It might be helpful if you could provide a backtrace
of exactly what fails.  And the answer to your question is unfortunately no.

-- 
Nick Kew

Re: apr_hash_set allocation failure behavior

Posted by Neil Conway <ne...@gmail.com>.
On Wed, Mar 17, 2010 at 5:40 PM, Ivan Novick <in...@greenplum.com> wrote:
> Regarding the amount of allocation of memory for a large hashtable:
>
> It seems the overhead is limited to 2 X overhead.  For example if I am
> growing a table from 1 entry to 1024 entries would have an array of size 16,
> 32, 64, 128, 256, 512, 1024 = 2048 entries allocated instead of 1024 entries
>
> Also this overhead is just for the pointers... If each item in the table is
> 4KB for example the overall overhead of the data structure is an extra
> 100-200 Bytes per entry whereas the data itself is much larger than that.
>
> I am missing something that makes the situation much worse?

No, that's accurate. If the # of hash table entries is small relative
to the size of each entry, it isn't a serious problem.

Neil

Re: apr_hash_set allocation failure behavior

Posted by Ivan Novick <in...@greenplum.com>.
On 3/17/10 3:35 PM, "Neil Conway" <ne...@gmail.com> wrote:

> Note that I'd be hesitant to use apr_hash for large tables, unless you
> can accurately pre-size it: see
> http://markmail.org/message/ljylkgde37xf3wdm and related threads (the
> referenced patch ultimately had to be reverted, because it broke code
> that accessed hash tables from a cleanup function in the same pool).

Regarding the amount of allocation of memory for a large hashtable:

It seems the overhead is limited to 2 X overhead.  For example if I am growing a table from 1 entry to 1024 entries would have an array of size 16, 32, 64, 128, 256, 512, 1024 = 2048 entries allocated instead of 1024 entries

Also this overhead is just for the pointers... If each item in the table is 4KB for example the overall overhead of the data structure is an extra 100-200 Bytes per entry whereas the data itself is much larger than that.

I am missing something that makes the situation much worse?

Cheers,
Ivan Novick

Re: apr_hash_set allocation failure behavior

Posted by Neil Conway <ne...@gmail.com>.
On Wed, Mar 17, 2010 at 3:18 PM, Ivan Novick <in...@greenplum.com> wrote:
> When adding to a hash table using apr_hash_set ... If memory can not be
> allocated what is the expected behavior?
>
> I am seeing apr_hash_set calls expand_array.
>
> expand_array core dumps if memory can not be allocated.
>
> Is this expected?  Is there a way to get an error code for a failed insert
> to a table rather than a coredump?

Seems easy enough to just make expand_array() give up if alloc_array()
fails: the next call to apr_hash_set() will try to expand the array
again (if warranted). See attached patch.

Note that I'd be hesitant to use apr_hash for large tables, unless you
can accurately pre-size it: see
http://markmail.org/message/ljylkgde37xf3wdm and related threads (the
referenced patch ultimately had to be reverted, because it broke code
that accessed hash tables from a cleanup function in the same pool).

Neil

Re: apr_hash_set allocation failure behavior

Posted by Ivan Novick <in...@greenplum.com>.
On 3/17/10 3:39 PM, "Graham Leggett" <mi...@sharp.fm> wrote:

>On 18 Mar 2010, at 12:18 AM, Ivan Novick wrote:

>> I am doing some testing with APR version 1.2.12
>>
>> When adding to a hash table using apr_hash_set ... If memory can not
>> be
>> allocated what is the expected behavior?
>>
>> I am seeing apr_hash_set calls expand_array.
>>
>> expand_array core dumps if memory can not be allocated.
>>
>> Is this expected?  Is there a way to get an error code for a failed
>> insert
>> to a table rather than a coredump?

>This is definitely wrong, APR should return APR_ENOMEM if memory
>cannot be allocated.

>More specifically, the APR pools code gives the caller (you) the
>choice of behaviour when the memory allocation fails. You can choose
>to have APR call a function of your choice, or you can choose APR to
>return APR_ENOMEM.

>httpd chooses to call a function that terminates the server child when
>out of memory, and as a result httpd makes no attempt to cater for out
>of memory conditions, but other callers of APR don't have to, and APR
>itself should definitely respect APR_ENOMEM.


When you say this wrong, do you mean the code is wrong or that the analysis of what the code does is wrong?

In the code below, if alloc_array returns NULL, this should core dump.  In fact this code returns void so I don't see how it could be passing back information about failed memory allocation with a return code.

Cheers,
Ivan

/*
 * Expanding a hash table
 */

static void expand_array(apr_hash_t *ht)
{
    apr_hash_index_t *hi;
    apr_hash_entry_t **new_array;
    unsigned int new_max;

    new_max = ht->max * 2 + 1;
    new_array = alloc_array(ht, new_max);
    for (hi = apr_hash_first(NULL, ht); hi; hi = apr_hash_next(hi)) {
        unsigned int i = hi->this->hash & new_max;
        hi->this->next = new_array[i];
        new_array[i] = hi->this;
    }
    ht->array = new_array;
    ht->max = new_max;
}


Re: apr_hash_set allocation failure behavior

Posted by Graham Leggett <mi...@sharp.fm>.
On 18 Mar 2010, at 12:18 AM, Ivan Novick wrote:

> I am doing some testing with APR version 1.2.12
>
> When adding to a hash table using apr_hash_set ... If memory can not  
> be
> allocated what is the expected behavior?
>
> I am seeing apr_hash_set calls expand_array.
>
> expand_array core dumps if memory can not be allocated.
>
> Is this expected?  Is there a way to get an error code for a failed  
> insert
> to a table rather than a coredump?

This is definitely wrong, APR should return APR_ENOMEM if memory  
cannot be allocated.

More specifically, the APR pools code gives the caller (you) the  
choice of behaviour when the memory allocation fails. You can choose  
to have APR call a function of your choice, or you can choose APR to  
return APR_ENOMEM.

httpd chooses to call a function that terminates the server child when  
out of memory, and as a result httpd makes no attempt to cater for out  
of memory conditions, but other callers of APR don't have to, and APR  
itself should definitely respect APR_ENOMEM.

Regards,
Graham
--