apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject Re: cvs commit: apr/include apr_sms_blocks.h
Date Wed, 13 Jun 2001 21:06:24 GMT
On 13 Jun 2001 dreid@apache.org wrote:

>   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:

>   static void *apr_sms_blocks_malloc(apr_sms_t *sms,
>                                      apr_size_t size)
>   {
>       void *mem;
>
>       if (size > BLOCKS_T(sms)->block_sz)
>           return NULL;
>
>       if ((mem = BLOCKS_T(sms)->free_list) != NULL) {
>           BLOCKS_T(sms)->free_list = ((block_t*)mem)->nxt;
>           return mem;
>       }
>
>       mem = BLOCKS_T(sms)->ptr;
>       BLOCKS_T(sms)->ptr += BLOCKS_T(sms)->block_sz;
>
>       if (BLOCKS_T(sms)->ptr > BLOCKS_T(sms)->endp)
>           return NULL;
>
>       return mem;
>   }

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.

>   static void *apr_sms_blocks_calloc(apr_sms_t *sms,
>                                      apr_size_t size)
>   {
>       void *mem;
>
>       if (size > BLOCKS_T(sms)->block_sz)
>           return NULL;
>
>       if ((mem = BLOCKS_T(sms)->free_list) != NULL) {
>           BLOCKS_T(sms)->free_list = ((block_t*)mem)->nxt;
>           return mem;
>       }
>
>       mem = BLOCKS_T(sms)->ptr;
>       BLOCKS_T(sms)->ptr += BLOCKS_T(sms)->block_sz;
>
>       if (BLOCKS_T(sms)->ptr > BLOCKS_T(sms)->endp)
>           return NULL;
>
>       memset(mem, '\0', BLOCKS_T(sms)->block_sz);
>
>       return mem;
>   }

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

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...


>   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


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA




Mime
View raw message