Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: tarantool-patches@freelists.org, kostja@tarantool.org
Subject: Re: [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage
Date: Thu, 7 Mar 2019 11:27:58 +0300	[thread overview]
Message-ID: <20190307082758.qywkiey7y5hudcpv@esperanza> (raw)
In-Reply-To: <20190306190349.GD10420@uranus.lan>

On Wed, Mar 06, 2019 at 10:03:49PM +0300, Cyrill Gorcunov wrote:
> On Wed, Mar 06, 2019 at 09:05:04PM +0300, Vladimir Davydov wrote:
> ...
> > > +
> > > +static void
> > > +fiber_wmark_recycle(struct fiber *fiber)
> > > +{
> > > +	static bool overflow_warned = false;
> > > +
> > > +	if (fiber->stack == NULL || fiber->flags & FIBER_CUSTOM_STACK)
> > > +		return;
> > > +
> > > +#ifndef TARGET_OS_DARWIN
> > > +	/*
> > > +	 * On recycle we're trying to shrink stack
> > > +	 * as much as we can until first mark overwrite
> > > +	 * detected, then we simply zap watermark and
> > > +	 * assume the stack is balanced and won't change
> > > +	 * much in future.
> > > +	 */
> > 
> > I don't understand. If the watermark has been exceeded once for a
> > particular fiber, we'll never ever shrink the stack back to normal?
> 
> I guess you meant "extend", not "shrink". But true -- every recycle
> we're trying to shrink stack, ie to make it less. And once we found
> that wmark is vanished we stop further analisys.
> 
> > That is if a task eager for stack is rescheduled on different fibers
> > they will all end up with huge stacks. That's not what we want.
> > We want the stack size to be set dynamically so that the *majority* of
> > fibers don't need to shrink/grow stacks back and forth. Or am I reading
> > the code wrong?
> 
> No, you're right. But it is unclean for me yet how we should reach
> this goal? Track stack usage in context switch?

You can track the ratio of fibers that have overwritten the watermark
recently. If the ratio gets too high (say 90% out of 1000 recently
scheduled fibers), the minimal stack allocation should be increased.

> ...
> > > +
> > > +	if (!stack_has_wmark(fiber->stack_overflow_wmark)) {
> > > +		say_warn("fiber %d seems to overflow the stack soon",
> > > +			 fiber->name, fiber->fid);
> > 
> > No point in printing fiber->name and id - they are printed by the say
> > module, anyway. At the same time I'd prefer to keep max stack size here
> > and print its value whenever we have to grow stack, similarly to how
> > the Linux kernel handles it.
> 
> ok
> 
> > > +	if (stack_direction < 0) {
> > > +		fiber->stack_overflow_wmark  = fiber->stack;
> > > +		fiber->stack_overflow_wmark += wmark_inpage_offset;
> > > +
> > > +		fiber->stack_dynamic_wmark = fiber->stack_overflow_wmark + page_size;
> > > +	} else {
> > > +		fiber->stack_overflow_wmark  = fiber->stack + fiber->stack_size;
> > > +		fiber->stack_overflow_wmark -= page_size;
> > > +		fiber->stack_overflow_wmark -= wmark_inpage_offset;
> > > +
> > > +		fiber->stack_dynamic_wmark = fiber->stack_overflow_wmark - page_size;
> > 
> > I'd rather start with the minimal allowed stack size and grow it on
> > demand, if there are too many fibers that exceed the watermark now and
> > then, not vice versa (imagine we start from 16 MB).
> 
> This won't work. Imagine we put wmark in _first_ page, and then we find
> that the wmark has been overwriten, where to put next mark then? Or
> you propose to catch signals?

Put the next watermark in the second page then. If it gets overwritten,
too, put it in the third page, and so on. Perhaps, we can even double
the minimal stack allocation each time.

> 
> > > +
> > > +	/*
> > > +	 * To increase probability of the stack overflow
> > > +	 * detection we put _first_ mark at random position
> > > +	 * in first 128 bytes range. The rest of the marks
> > > +	 * are put with constant step (because we should not
> > > +	 * pressue random generator much in case of hight
> > > +	 * number of fibers).
> > > +	 */
> > > +	random_bytes((void *)&v, sizeof(v));
> > 
> > Why not rand?
> 
> Because we already have a helper for random numbers :)

It's a helper for filling an array with random bytes. I don't see much
point in using it for generating a single random number.

  reply	other threads:[~2019-03-07  8:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 22:38 [PATCH v3 0/3] core/fiber: Rework stack management Cyrill Gorcunov
2019-03-05 22:38 ` [PATCH 1/3] core/fiber: Increase default stack size Cyrill Gorcunov
2019-03-05 23:43   ` [tarantool-patches] " Alexander Turenko
2019-03-06  7:09     ` Cyrill Gorcunov
2019-03-06  7:30     ` Konstantin Osipov
2019-03-05 22:38 ` [PATCH 2/3] core/fiber: Mark stack as unneeded on creation Cyrill Gorcunov
2019-03-05 22:38 ` [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage Cyrill Gorcunov
2019-03-06 18:05   ` Vladimir Davydov
2019-03-06 19:03     ` Cyrill Gorcunov
2019-03-07  8:27       ` Vladimir Davydov [this message]
2019-03-07  9:04         ` Cyrill Gorcunov
2019-03-07  9:20           ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190307082758.qywkiey7y5hudcpv@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox