[haiku-commits] Re: r35601 - in haiku/trunk: headers/private/kernel/slab src/system/kernel/cache src/system/kernel/slab src/system/kernel/vm
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 24 Feb 2010 18:43:36 +0100
Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> On 2010-02-24 at 15:43:20 [+0100], axeld@xxxxxxxxxxxxxxxx wrote:
> > - if (magazine) {
> > - _push(depot->full, magazine);
> > - depot->full_count++;
> > + if (magazine != NULL) {
> > + if (depot->full_count < depot->max_count) {
> > + _push(depot->full, magazine);
> > + depot->full_count++;
> > + } else
> > + freeMagazine = magazine;
> A disadvantage of this implementation is that a relatively recently
> used
> magazine is discarded while older ones are kept, which from a caching
> perspective is definitely suboptimal. Have you by any chance tested
> how that
> affects performance?
No, I haven't, but I won't suspect a big impact, as the magazine is put
into a stack where it's not really likely to be reused that early. But
it's probably better to take the chances and pop older ones, even though
that one will be the last one that has been pushed (so the difference
between the two might be minimal).
Alternatively, one could use a doubly linked list or now even an array,
and really remove the oldest entry.
> > + if ((cache->flags & CACHE_NO_DEPOT) == 0) {
> > + object_depot_make_empty(&cache->depot, 0);
> > + // TODO: what flags?
> The apropriate alloc/free flags.
Sure, but at this point we don't really know what's appropriate (at
least not the usual convention as used by the caller -- and that's what
I tried to express with that comment).
> I suppose CACHE_DONT_WAIT_FOR_MEMORY could
> be specified to prevent the somewhat pathological situation that the
> low
> memory handler has to wait for memory (when the object depot for the
> magazines object cache wants to allocate a magazine and has to go to
> MemoryManager; or when a HashedObjectCache's hash table is resized).
> I guess
> it would be better to introduce a special flag CACHE_DONT_ALLOCATE
> for this
> case, though.
Sounds good, feel free to do that :-)
Bye,
Axel.
Other related posts:
