From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34])
 (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 B5A6246970E
 for <tarantool-patches@dev.tarantool.org>;
 Wed,  5 Feb 2020 00:25:16 +0300 (MSK)
References: <20200127193907.34019-1-georgy@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <1a8f199e-4fd6-004e-4d91-bc7a909b242d@tarantool.org>
Date: Tue, 4 Feb 2020 22:25:14 +0100
MIME-Version: 1.0
In-Reply-To: <20200127193907.34019-1-georgy@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH] do not keep a slab in cache if new
 allocation does not fit
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:39, Georgy Kirichenko wrote:
> Do not allocate a new slab if there is a free slab in a region cache.
> If a cached slab is not big enough then free it.

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

> Follow up 67d7ab44ab09df3356929e3692a03321b31f3ebb
> 
> Fixes #12

Well, the slabs still are not freed in case there was a spike
which allocated lots of slabs, and then only 1-2 are used, and
all allocations fit into the existing slabs.

> 
> Issue: https://github.com/tarantool/small/issues/12
> Branch: https://github.com/tarantool/small/tree/g.kirichenko/gh-12-region-cache
> 
> diff --git a/small/region.h b/small/region.h
> index d9be176..58481b0 100644
> --- a/small/region.h
> +++ b/small/region.h
> @@ -157,14 +157,20 @@ region_reserve(struct region *region, size_t size)
>  		if (size <= rslab_unused(slab))
>  			return (char *) rslab_data(slab) + slab->used;
>  		/* Try to get a slab from the region cache. */
> -		slab = rlist_last_entry(&region->slabs.slabs,
> -					struct rslab,
> -					slab.next_in_list);
> -		if (slab->used == 0 && size <= rslab_unused(slab)) {
> -			/* Move this slab to the head. */
> +		while ((slab = rlist_last_entry(&region->slabs.slabs,
> +						struct rslab,
> +						slab.next_in_list))->used == 0) {
>  			slab_list_del(&region->slabs, &slab->slab, next_in_list);
> -			slab_list_add(&region->slabs, &slab->slab, next_in_list);
> -			return (char *) rslab_data(slab);
> +			if (size <= rslab_unused(slab)) {
> +				/* Move this slab to the head. */
> +				slab_list_add(&region->slabs, &slab->slab, next_in_list);
> +				return (char *) rslab_data(slab);
> +			}
> +			/*
> +			 * This cached slab could not be used so free it
> +			 * and try to use the next one.
> +			 */
> +			slab_put(region->cache, (struct slab *)slab);
>  		}
>  	}
>  	return region_reserve_slow(region, size);
>