[tarantool-patches] [rfc 4/4] core/fiber: Shrink stack when recycling

Konstantin Osipov kostja at tarantool.org
Tue Mar 5 11:30:48 MSK 2019


* Cyrill Gorcunov <gorcunov at gmail.com> [19/03/03 23:25]:
> +#define wmark_freeze(_pp) do { *((uintptr_t *)(_pp)) |= (uintptr_t)1; } while (0)
> +#define wmark_frozen(_p) ((uintptr_t)(_p) & (uintptr_t)1)

Why not static inline? A comment would help. Usually the name for
function returning true/false contains "is", e.g.
wmark_is_frozen().


>  
>  static void
> +stack_shrink(struct fiber *fiber)
> +{
> +	void *hi, *lo;
> +
> +	if (stack_direction < 0) {
> +		hi = fiber->stack + fiber->stack_size;
> +		lo = fiber->stack_wmark_ofl + page_size;
> +	} else {
> +		hi = fiber->stack_wmark_ofl - page_size;
> +		lo = fiber->stack - fiber->stack_size;
> +	}
> +
> +	if (fiber->stack_wmark <= lo ||
> +	    fiber->stack_wmark >= hi)
> +		return;
> +
> +	madvise(fiber->stack_wmark, page_size, MADV_DONTNEED);
> +	if (stack_direction < 0)
> +		fiber->stack_wmark += page_size;
> +	else
> +		fiber->stack_wmark -= page_size;
> +	stack_put_wmark(fiber);
> +}
> +
> +static void
>  fiber_wmark_recycle(struct fiber *fiber)
>  {
>  	static bool overflow_warned = false;
> @@ -772,20 +800,30 @@ fiber_wmark_recycle(struct fiber *fiber)
>  
>  	/*
>  	 * We are watching for stack overflow in one shot way:
> -	 * firstly to not spam a user with messages and secondly
> -	 * to relax system from additional operations while our
> -	 * watermark is not dynamic.
> +	 * to not spam a user with messages.
>  	 */
> -	if (overflow_warned)
> -		return;
> -
> -	if (!stack_has_wmark(fiber->stack_wmark_ofl)) {
> -		if (!overflow_warned) {
> +	if (!overflow_warned) {
> +		if (!stack_has_wmark(fiber->stack_wmark_ofl)) {
>  			say_warn("fiber %d is close to a stack limit",
>  				 fiber->name, fiber->fid);
>  			overflow_warned = true;
>  		}
>  	}
> +
> +	if (wmark_frozen(fiber->stack_wmark))
> +		return;
> +
> +	/*
> +	 * On recycle we're trying to shrink stack
> +	 * as much as we can until first mark overwrite
> +	 * detected, then we simply freeze watermark and
> +	 * assume the stack is balanced and won't change
> +	 * much in future.
> +	 */
> +	if (!stack_has_wmark(fiber->stack_wmark))
> +		wmark_freeze(&fiber->stack_wmark);

I don't see how "frozen" watermark concept is more powerful than
already existing "overflow warned" flag. I think the idea is to
have some sort of smart threshold after which we stop trying to
shrink the stack.

Right now this threshold is not too smart (which is OK), but
complicated (both overflow_warned and wmark_frozen() are
considered), which doesn't make too much sense to me.

> +	else
> +		stack_shrink(fiber);

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov



More information about the Tarantool-patches mailing list