From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) (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 6D2D646970E for ; Thu, 30 Jan 2020 00:41:24 +0300 (MSK) Received: by mail-lf1-f53.google.com with SMTP id f24so771501lfh.3 for ; Wed, 29 Jan 2020 13:41:24 -0800 (PST) Date: Thu, 30 Jan 2020 00:41:22 +0300 From: Konstantin Osipov Message-ID: <20200129214122.GB31458@atlas> References: <4e734e626aba336b27ec85790747c657d29c0338.1580284383.git.alexander.turenko@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4e734e626aba336b27ec85790747c657d29c0338.1580284383.git.alexander.turenko@tarantool.org> 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/29 11:11]: > This reverts commit 67d7ab44ab09df3356929e3692a03321b31f3ebb. > > The goal of the reverted commit was to fix flaky fails of tarantool > tests that checks amount of memory used by a fiber: > > | fiber.info()[fiber.self().id()].memory.used > > It also attempts to overcome the situation when a fiber holds some > amount of memory, which is not used in any way. The high limit of such > memory is controlled by a threshold in fiber_gc() tarantool's function > (128 KiB at the moment): The commit by Georgy was doing the right thing, it just had a bug in it. I saw Georgy posted a fix for his commit - but I haven't seen it on the list. Why revert this commit rather than fix it? It should be pretty strightforward to fix, no? The fix is to avoid iterating over all the free slabs when we failed to find the right slab size several times, and just get a new slab at once. I'd even say that Georgy's commit was entirely correct, it just didn't account for the "land mine" it is setting for alloc case. Thoughts? -- Konstantin Osipov, Moscow, Russia