Re: TLV to memPools

Andres Kroonmaa wrote:

> I'm all for using dedicated pools for things that are used often or
> that are living long. The issue comes up with stuff that lives for
> around 100 cpu ticks and whose max concurrent usage is 1 or 0. Do we
> really need to create memPool for such, going to trouble doing so?
> We have currently about 90 pools in squid, of which typically ever
> used are 60, and on average inuse at a time of sampling are 50, and
> about 10 pools are ever used for less than 10 allocs. We can convert
> all allocs to dedicated pools, but would we care?

For temporary things like TLV I am for not using any allocation as
described before.

For long lived allocations that are made 10 times I still see a benefit
in having them pooled. Not for performance but for accounting and
quality checks. But it is not a high priority issue.

Everything that is allocated "dynamically" per request or whatever
SHOULD either be pooled, or not allocated at all (i.e. static or stack
allocation). And I don't like "generic/typeless" pools for typed data.
Any typed data should have it's own pool.

> For eg. would we care that TLV is using alloc sizes of MD5, SEntry's
> 4xTimestamps, and variable URL? We know that those allocs exist only
> between MetaBuild and MetPack, ie. about 20 lines of C-code. We also
> know that they are all done per every swaplogdata. Why would we need
> a dedicated pool for each of these items?

We also know that they all exists together as one more complex entity,
which could easily be pooled. Or even statically allocated for that
matter if it is known that the "object" is short lived and only exists
within a very narrow context.

> On the other hand, if we too eagerly start using abstract pools,
> tune them to be more granular, we add overhead and loose ability
> to notice high frequency and usage allocs, along with which type
> may be leaking.

I am for using dedicated pools at most locations. My view is also that
short lived data of fixed length should be allocated statically or from
the stack. Short lived data of varying length is a bit trickier but
there is constructs like alloca() that some times makes sense.

Using pooling for short lived data which only exists within a single
function and it's siblings does not make much sense imho.

> I think that major compromise must be made here. I wouldn't even
> object if it is decided "politically" to reject existence of abstract
> pools. For some data this is unavoidable, like variable length bufs,
> strings. But we can insist that each file implements its own set of
> pools. If this makes sense.

In the case of
  String
  MemBuf
  TLV
I think it makes sense if each has it's own set of pools. This to allow
them to be better tuned for the actual use, and to maintain
understantable accounting of the allocations.

> Its actually simple to implement abstract pool ontop of cbdata,
> add API call cbdataAllocBestfit(size_t size) which picks between
> few pools or malloc(). We could even wrap xmalloc/xfree to use such
> cbdata call. And thats it, we could convert all xmallocs to cbdata.
> Would that be a good idea is another matter.

Doing it ontop of rcdata may make sense, but for most such uses I think
a MemBuf is the proper choice (which should in turn be reference
counted).

> In any case, if we even decide that abstract pools are cute, we need
> to make sure that important and frequently used types have dedicated
> pools. How would we make that sure? Would prog-guide be enough?

prog-guide is sufficient I think.

> Shouldn't focus on TLV at all. There are lots of others similar.

Name one that is similar to TLV on sizing, typing, scope and time.

Sure, there are plenty of examples that is similar on one of the aspects
at a time, but TLV has a rather unique combination of properties I'd
say, and I dislike this specific combination (variable sizing, abstract
typing, semi-local scope with leakage to FS modules, extremely
shortlived)

I'd say the unique properties of TLV calls for being a special case.

Regards
Henrik
Received on Tue Oct 09 2001 - 06:53:51 MDT