Re: Chunked mempools, a second verdict

On 3 May 2001, at 23:49, Henrik Nordstrom <hno@hem.passagen.se> wrote:

> > /* Tune API */
> > extern void memPoolSetChunkSize(MemPool * pool, size_t chunksize);
> > extern void memPoolSetIdleLimit(size_t new_idle_limit);
>
> Fine, as long as these are optional.

 you mean use of them is optional? setChunkSize can always be, but
 for idle_limit we'd need then hardwire some default.

> > /* Stats API */
> > extern int memPoolGetCount(); /* might be unnecessary */
> > extern MemPool *memPoolGetPool(int index);
> > extern int memPoolGetStats(MemPoolStats * stats, MemPool * pool);
>
> I think the memPoolGetCount/memPoolGetPool interface should be replaced
> by a iterator interface
> extern MemPool * memPoolGetFirstPool(void);
> extern MemPool * memPoolGetNextPool(MemPool *previous_pool);

 As it is currently, it is somewhat clumsy to determine NextPool
 based on *prev_pool. Would require linear search.
 We would like to ignore *previous_pool initially.
 With a guarantee that after First(), Next() would iterate through
 all pools, as long as First,Destroy,Create is not called?

> or even one using a abstract iterator type
>
> extern MemPoolIterator * memPoolIterate(void);
> extern MemPool * memPoolGetNextPool(MemPoolIterator **);
> and optionally (in which case one * above goes away)
> extern void memPoolIterateDone(MemPoolIterator **)

 Why would that be good? (I've not seen something like that yet)
 Sounds complex to me. At least not intuitive.

> Note: The ** is to be able to NULL:ify the pointer as described above.

> > /* Stats history API */
> > extern void memPoolCheckRates(); /* stats history checkpoints */
>
> Explain please.

 see my other post about perpool alloc rates/sec column in cachemgr.
 currently, memReport is modifying memMeter structs it receives from
 memPoolGetStats(), by copying current counter aside. This call is
 meant to ask the library to do that internally for us.
 To maintain historical stats outside the pool itself would be
 trouble, and no easy way to get notified when pool is destroyed.

> > extern void memPoolUpdateTime(time_t time); /* better avoid */
>
> Cut. Use current_dtime or a similar global variable.

 As it is done now? This kind of violates the goal for the library
 to be independant of the source it is used with. But its simplest.

> > /* Module housekeeping API */
> > extern void memPoolInit(void);
> > extern void memPoolClean(time_t maxage); /* maybe pass curtime also? */
> > If we implement selfIniting, selfcleaning memPools, may omit both.
>
> The Init call should not be needed I think. Can be done automatically on
> memPoolCreate() and undone when the last pool is memPoolDestroy():ed.

 ok, I'll hide the Init call.

> Self-cleaning memory pools would be nice, but either way is fine.

> > extern void memMeterSyncHWater(MemMeter * m);
> > This shouldn't be part of an API. Why not a macro?
>
> Please explain where/how MemMeter is used. If internal to MemPool.c then
> move it in there and it is fully out of the discussion.

 in mem.c Short/Medium/Long Strings_pools are handled via memAllocBuf().
 It uses same Meter types and same macros for updating. memMeterSyncHWater()
 is the only one implemented as a func instead of macro. so its shared.
 We could duplicate it and make both copies private, but it works as a
 macro equally well.
 And we could scrap these stats from mem.c, because these String pools
 are represented on memPool reports in much better detail anyway. The
 only type not covered is "Other Strings", as it is passed to xmalloc/
 xfree, but the stats are gathered. I'm not sure if these other strings
 have ever seen a single hit.

> Now, please write up the MemPool API in programmers guide and it should
> be ready for HEAD after some testing. I have started a section for you
> at the end showing the documentation syntax we use in other API
> definitons.

 ok, i'll see that.

------------------------------------
 Andres Kroonmaa <andre@online.ee>
 CTO, Delfi Online
 Tel: 6501 731, Fax: 6501 708
 Pärnu mnt. 158, Tallinn,
 11317 Estonia
Received on Thu May 03 2001 - 16:49:01 MDT