From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3F76346970E for ; Thu, 30 Jan 2020 11:34:39 +0300 (MSK) Received: by mail-lj1-f196.google.com with SMTP id w1so2376701ljh.5 for ; Thu, 30 Jan 2020 00:34:39 -0800 (PST) Date: Thu, 30 Jan 2020 11:34:37 +0300 From: Konstantin Osipov Message-ID: <20200130083437.GD631@atlas> References: <20200129014816.14248-1-bokuno@picodata.io> <20200129102813.3gsdeo27lmoh2zsi@tarantool.org> <20200129214603.GC31458@atlas> <20200130080223.tyta6lti4xsgwqcg@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200130080223.tyta6lti4xsgwqcg@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] small: unite the oscillation cache of all mempools List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kirill Yukhin Cc: tarantool-patches@dev.tarantool.org * Kirill Yukhin [20/01/30 11:02]: > Hello, > > Thanks a lot for your answer! > > On 30 янв 00:46, Konstantin Osipov wrote: > > * Kirill Yukhin [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