From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) (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 7B6C046970E for ; Thu, 30 Jan 2020 11:21:17 +0300 (MSK) Received: by mail-lf1-f54.google.com with SMTP id f24so1690589lfh.3 for ; Thu, 30 Jan 2020 00:21:17 -0800 (PST) Date: Thu, 30 Jan 2020 11:21:15 +0300 From: Konstantin Osipov Message-ID: <20200130082115.GC631@atlas> References: <4e734e626aba336b27ec85790747c657d29c0338.1580284383.git.alexander.turenko@tarantool.org> <20200129214122.GB31458@atlas> <20200130072318.zxze72h2proksji5@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200130072318.zxze72h2proksji5@tkn_work_nb> Subject: Re: [Tarantool-patches] [small] Revert "Free all slabs on region reset" List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org * Alexander Turenko [20/01/30 10:27]: > 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. > I think that reserve/reset should give predictable performance and so > should be constant until region_free() will be called. Ideally it should > doing something like Georgy made: put one or several unnecessary slabs > to a slab_cache. Even better would be spread deallocations across > allocation calls like LuaJIT does: perform more deallocations if there > are more garbage. 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. There is no need to complicate it beyond that. It worked fine for 10 years, after all. > > BTW, #4750 is only about tests and will be closed soon. If you have > thoughts about gains we can get from changing of region_*() algorithms, > please, file a separate issue and describe them. > The fix that Georgy made in May was not only about tests. It was improving region memory usage (and thus indirectly performance), too. -- Konstantin Osipov, Moscow, Russia