[Tarantool-patches] [PATCH] small: unite the oscillation cache of all mempools

[Konstantin Osipov]

* Kirill Yukhin <kyukhin at tarantool.org> [20/01/30 11:02]:
> Hello,
> 
> Thanks a lot for your answer!
> 
> On 30 янв 00:46, Konstantin Osipov wrote:
> > * Kirill Yukhin <kyukhin at tarantool.org> [20/01/29 13:30]:
> > > On 29 янв 04:48, Maksim Kulis wrote:
> > > > The task is to unite the oscillation cache of all mempools.
> > > > In the function slab_put_with_order() we check if there is any 
> > > > slab of the largest size except the current one, in order to 
> > > > avoid oscillation. 
> > > > https://github.com/tarantool/small/blob/master/small/slab_cache.c#L349
> > > 
> > > Thanks a lot for your patch. Since the change is performance
> > > related, before we start off review process, please provide a
> > > testcase (for tarantool or whatever) which will clearly show
> > > that there're cases where performance is actually increased.
> > > 
> > > We won't accept "nice looking" perf patches.
> > 
> > It's not about performance :)
> 
> Okay.
> 
> > The patch reduces memory fragmentation quite a bit on small-arena
> > systems. Imagine a 1G system with slab_alloc_factor =1.06
> > (default). It can easily have up to 50-100 mempools in slab arena,
> > each holding up one mslab, up to 100-400MB in total. 
> > 
> > The whole point of holding up to such mslab is to avoid mmap()
> > system call (which is costly) in cases like 
> > 
> > for (i = 1..10000)
> >     mslab_alloc()
> >     mslab_free()
> > 
> > This is allocation scenario is called "oscilation".
> > 
> > There is no point in it, because there is already a cached mslab in
> > slab_cache, and it will be used just fine.
> > 
> > Maksim should update the CS comment.
> 
> Text is fine. But we need a test which will clearly show that
> the change will improve something user-visible. If it is not
> then it is refactoring and I doubt we'll take it.

Is this why you accepted transactional DDL without tests and
haven't done any till now?

Nice to see https://github.com/tarantool/tarantool/issues/4349
being shifted around again and again.

It didn't stop you from proclaiming 2.2 stable either...

In the first response you show you don't know what the patch is
about, and in the second top it with double standards for
accepting Mail.Ru and community patches.

Good stewardship indeed...

PS It is of course possible to show that memory fragmentation has
decreased with this patch by allocating a few objects and looking
at memory stats. But such test will be fragile and thus bring more
harm than good. 

-- 
Konstantin Osipov, Moscow, Russia