[haiku-commits] Re: r35601 - in haiku/trunk: headers/private/kernel/slab src/system/kernel/cache src/system/kernel/slab src/system/kernel/vm
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 24 Feb 2010 20:18:01 +0100
On 2010-02-24 at 18:43:36 [+0100], Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
wrote:
> 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.
If the allocating/freeing happens in bursts larger than twice the magazine
size (not exactly an unusual pattern), then that's exactly what will
happen. The behavior now is that in a free burst we return the most
recently used objects (save for one to two magazines) to the slab.
> 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.
Yep, I thought of an array, too. Since the magazine capacity is fixed, that
would be rather easy to do.
> > > + 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).
Huh?
> > 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 :-)
Feel free to create a ticket. :-)
CU, Ingo
Other related posts:
