From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gorcunov@gmail.com>
Return-Path: <gorcunov@gmail.com>
Date: Tue, 5 Mar 2019 11:41:10 +0300
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: Re: [tarantool-patches] [rfc 4/4] core/fiber: Shrink stack when
 recycling
Message-ID: <20190305084110.GM13301@uranus.lan>
References: <20190302125524.279852704@gmail.com>
 <20190302130034.125890957@gmail.com>
 <20190305083048.GM21955@chai>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190305083048.GM21955@chai>
To: Konstantin Osipov <kostja@tarantool.org>
Cc: tarantool-patches@freelists.org, vdavydov.dev@gmail.com
List-ID: <tarantool-patches.dev.tarantool.org>

On Tue, Mar 05, 2019 at 11:30:48AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@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().

Well, surely we can use inlines here, I simply tried to make less code.
Seriously fiber.c looks too bloating. Maybe we should move all this
machinery into fiber-stack.cc?

...
> > +
> > +	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.

Look, the idea is too keep _two_ tests: one when stack is close
to overflow, we do this test every recycle and never move the
mark to other place; this page with overflow mark will be
always present.

In turn overflow mark is shrinked every recycle until first rewrite
detected. Then we simply zap the pointer and never test it again.

If you mean something else, lets talk f2f?