Re: mempool patches

On Mon, 24 Aug 1998, Stephen R. van den Berg wrote:

> Since, in all practical uses the mem-usage of squid tends to grow and
> then stabilise, I've changed the behaviour of mempool a bit to allocate
> in larger chunks as the usage grows. This effectively reduces overhead
> and fragmentation.

The patch intends to replace memory pool's "stacks of individually allocated
objects" with a never-free, rapidly growing blocks of memory.

The extra memory overhead for keeping mempools stacks is usually a few KB
only, negligible for any reasonable configuration. I do not have any hard
numbers for fragmentation, but my guess it is not a big problem with a good
malloc library.

Preallocation of memory could be easily done within the current scheme. If
anybody thinks there will be a noticeable win from that, please let me know.

For those developers who want to apply the patch, please note that not all
new code is in #ifdefs. Thus, you cannot switch to the old behaviour once the
patch is applied. Also note that your pools will never shrink after applying
the patch. Actually, from what I can tell, there will be no way to enforce
the idle memory limit at all.

> + obj = xmalloc(step*pool->obj_size);
> + memMeterAdd(pool->meter.alloc, step);
> + memMeterAdd(TheMeter.alloc, step*pool->obj_size);
> + while(--step) {
> + memPoolFree(pool, obj);
> + obj=(char*)obj+pool->obj_size;

Not sure, but looks like you are freeing the memory that just got allocated:
"obj = xmalloc(...); ... memPoolFree(pool, obj);" ?

> memPoolFree(MemPool * pool, void *obj)
> {
> assert(pool && obj);
> + assert(pool->obj_size >= sizeof pool->pstack);

"Sizeof(ptr)" does not return the size of the object pointed by "ptr". It
returns the size of "ptr". This added assertion disallows the pooling of
objects smaller than "sizeof(void*)" bytes, as far as I can tell.

>- memset(obj, 0, pool->obj_size);
>- stackPush(&pool->pstack, obj);
>+ *(void**)obj = pool->pstack;
>+ pool->pstack = obj;

Sorry, but I failed to understand what this is supposed to do.

> +#if 0 /* bogus assertion? srb */
> assert(pool->meter.idle.level <= pool->meter.alloc.level);
> +#endif

The assertion is not bogus. It reads "idle memory must be at most the size of
allocated memory". In other words, something that was not allocated cannot
become idle. In fact, just a few days ago we detected a bug thanks to this
assertion. If it fires after applying the patch, the accounting part of the
pools probably got broken by the patch.

Alex.
Received on Tue Jul 29 2003 - 13:15:52 MDT