From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 20B2A46970E
 for <tarantool-patches@dev.tarantool.org>;
 Wed,  5 Feb 2020 00:25:56 +0300 (MSK)
References: <20200127193320.33773-1-georgy@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <2dccd4ca-9f29-c71d-f49c-da6722a5e743@tarantool.org>
Date: Tue, 4 Feb 2020 22:25:54 +0100
MIME-Version: 1.0
In-Reply-To: <20200127193320.33773-1-georgy@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH] region: take allocated region size
 instead of used one
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Georgy Kirichenko <georgy@tarantool.org>, tarantool-patches@dev.tarantool.org

Hi! Thanks for the patch!

On 27/01/2020 20:33, Georgy Kirichenko wrote:
> As region has cached but not used slabs take the full region size
> in account while fiber gc.

Please, describe the problem you are trying to solve. The
text above just narrates the code.

If this is related to the perf degradation, then there is
another hint for you: region_reset() looks like this:

	static inline void
	region_reset(struct region *region)
	{
		struct rslab *slab;
		rlist_foreach_entry(slab, &region->slabs.slabs, slab.next_in_list) {
			region->slabs.stats.used -= slab->used;
			slab->used = 0;
		}
	}

But in fact you can stop when you see first 'used == 0', because
all the next slabs anyway have used 0 too (from what I see in the code).
So as not to add an 'if' here, we could in struct region keep a
pointer at the last slab having used != 0. And iterate till it.

> Fixes #4736
> 
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index 00ae8cded..f795dc566 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -774,7 +774,7 @@ fiber_self()
>  void
>  fiber_gc(void)
>  {
> -	if (region_used(&fiber()->gc) < 128 * 1024) {
> +	if (region_total(&fiber()->gc) < 128 * 1024) {
>  		region_reset(&fiber()->gc);
>  		return;
>  	}
>