Re: [Tarantool-patches] [small] Revert "Free all slabs on region reset"

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Alexander Turenko <alexander.turenko@tarantool.org> [20/01/30 14:06]:
> > > The Georgy's fix for small keeps linear traversal over allocated slabs
> > > on region_reset(). Fixed region_reserve() releases one slab at max per
> > > allocation, so this linear traversal will persist for several
> > > region_reset() calls in row.
> > 
> > Why do you think region_reset() should traverse over empty slabs?
> > Empty slabs should always remain at the end of the list, let's
> > stick to this invariant.
> 
> Don't get the point. Do you propose to stop this traverse on a first
> unused slabs? AFAIU, there may be slabs with zero slab->used in a
> middle. We cannot stop based on `slab->used` value.

Let's make sure it's impossible. This is basically what Georgy's
patch is doing already.

> > region_free()/region_reset() should be (and is in most cases)
> > bounded, since it is called as soon as region_used() reaches 128KB.
> > So yes, it's a list traversal, but it is done only once in every
> > dozen or so requests and is bounded by a list which is a few
> > dozens of slabs.
> 
> A list traversal on **each** region_reset(), not once. 20k, not few
> dozens. It does not look okay. It is even seen on the benchmark in
> #4736 (if uncomment per-batch RPS prints and run it on the patched
> version).

20k is caused by the bug, it's not worth considering.
Typically it's 128/4k = 32 elements.

> > The fix that Georgy made in May was not only about tests. It was
> > improving region memory usage (and thus indirectly performance),
> > too.
> 
> 1. This need to be verified.
> 2. Even if it increases performance on some workloads, it should not
>    significantly degrade others.
> 
> Small deallocations on during allocation is the good idea. Let's file an
> issue against this.
> 
> Linear traversal in region_reset() is the bad one, IMHO. Especially
> considering memory access pattern: it is list traversal, not an array.
> Especially if we need to traverse a slab N/2 times in average (N is the
> list size). At least until the upper limit of the list size is not
> proven to be small on any workload.

Well, OK. I guess you need time to sort this out.

-- 
Konstantin Osipov, Moscow, Russia