[Tarantool-patches] [PATCH] region: take allocated region size instead of used one

[Tarantool-patches] [PATCH] region: take allocated region size instead of used one

[Georgy Kirichenko]

As region has cached but not used slabs take the full region size
in account while fiber gc.

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;
 	}
-- 
2.25.0

Re: [Tarantool-patches] [PATCH] region: take allocated region size instead of used one

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

Sorry, I forgot an issue and a branch.
Issue: https://github.com/tarantool/tarantool/issues/4736
Branch: https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-4736-region-size-for-gc
Also the branch was repushed because of wrong rebase.
On Monday, 27 January 2020 22:33:20 MSK Georgy Kirichenko wrote:
> As region has cached but not used slabs take the full region size
> in account while fiber gc.
> 
> 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;
>  	}


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Tarantool-patches] [PATCH] region: take allocated region size instead of used one

[Konstantin Osipov]

* Georgy Kirichenko <georgy@tarantool.org> [20/01/27 22:34]:
> As region has cached but not used slabs take the full region size
> in account while fiber gc.

I prefer pushing the fix for linear scan and keeping this logic
intact, since fixing it may impact a lot of workloads which
require large amounts of region memory per request.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH] region: take allocated region size instead of used one

[Vladislav Shpilevoy]

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;
>  	}
>