apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sander Striker" <stri...@samba-tng.org>
Subject RE: cvs commit: apr/include apr_sms_blocks.h
Date Wed, 13 Jun 2001 22:16:23 GMT
> >   Log:
> >   Add a new sms module.
> >
> >   This one basically takes a slice of memory (defaults to 8k) and then
> >   hands out pieces of it of a given size as fast as possible.  When free
> >   is called on an allocated block it's added to a free list and then
> >   re-allocated.
>
> Thanks!  Buckets memory allocation, here I come.

:-)

> A few suggestions:

[...]
> Instead of returning NULL if size > block_sz, how about calling
> apr_sms_malloc(sms->parent)?  Same goes if the free list is empty and the
> initial block is completely used.  So if the parent is an apr_sms_std,
> then we fall back on plain old malloc() at the cost of lower performance.
> Say, for example, that you allocate 1K blocks out of your 8K hunk of
> memory, and all 8 blocks are allocated.  When we ask for a 9th one, it'd
> be much nicer to get one a little slower than to not get one at all.

Yes, but you would have anticipated for that wouldn't you (thus setting
a large enough size)?

Somewhat longer answer:
We can not just call apr_sms_malloc() on the parent for this one.
You'd have to track all the nodes that went out this way (to be able
to do the reset). If this is needed, then we'll have to implement it.
The way of doing this without creating to much extra overhead will result
in recycling larger blocks as if the were blocks of size 'block_size' (1K
in your example). This only applies for blocks that were returned using
apr_sms_free().

>>   static void *apr_sms_blocks_calloc(apr_sms_t *sms,
>>                                      apr_size_t size)

[...]
> AFAICT this doesn't need to be implemented... apr_sms_default_calloc()
> should be fine.

Ah, yes. David and I got a bit obsessed with speed last night. This is
a bit quicker than letting apr_sms_default_calloc() handle it. Come to
think of it, if we do a single memset() in the reset we can get away
with setting both calloc_fn and malloc_fn to the same function :)

> Speaking of which, the same could be done for apr_sms_std_calloc if
> !HAVE_CALLOC, but then again, I'm of the mind that we don't even need to
> check for calloc() at all and can just assume it exists.  The buckets code
> has used calloc() without checking for its existence first for ages, and
> nobody's ever complained...

Just to be safe...

> >   static apr_status_t apr_sms_blocks_reset(apr_sms_t *sms)
> >   {
> >       BLOCKS_T(sms)->ptr = (char *)sms + SIZEOF_BLOCKS_T;
> >       BLOCKS_T(sms)->free_list = NULL;
> >
> >       return APR_SUCCESS;
> >   }
>
> I had been about to ask why you didn't just carve up the ->ptr into as
> many blocks as possible in the create function and put them all onto the
> free_list ahead of time so that you can use only the free_list in
> _malloc()... but now I see why.  =-)  Clever.

:-)

> --Cliff

Sander


Mime
View raw message