Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tml <tarantool-patches@freelists.org>
Subject: Re: [PATCH 1/3] lib/core/fiber: Increase default stack size
Date: Wed, 13 Mar 2019 16:53:07 +0300	[thread overview]
Message-ID: <20190313135307.GN10420@uranus.lan> (raw)
In-Reply-To: <20190313134253.iihpk6ej7dx7a5rx@esperanza>

On Wed, Mar 13, 2019 at 04:42:53PM +0300, Vladimir Davydov wrote:
> On Wed, Mar 13, 2019 at 01:47:19AM +0300, Cyrill Gorcunov wrote:
> > The default 64K stack size used for years become too small
> > for modern distors (Fedora 29 and etc) where third party libraries
> > (such as ncurses) started to use 64K for own buffers and we get
> > SIGSGV early without reaching interactive console phase.
> > 
> > Thus we increase default size up to 512K which should fit
> 
> May be, better use 1 MB, just to be sure?

Well, you know iirc 512 should fit even on current MacOS.
Also 1M is not guarantee us anything we have to make
this value configurable on early init. And I'm gonna
make it so.

> > for common case. Later we will make this value configurable
> > to address arbitrary stack sizes without a need to rebuild
> > the whole code.
> > 
> > Note the values are switched to 4K page granularity for sake
> > of future modifications -- we gonna manipulate pages to
> > relax rss usage if OS allows.
> > 
> > Closes #3418
> 
> Should be "Part of #3418". We use "Closes" only with the last patch in
> the series (it makes GitHub close the issue).

Strictly speaking it closes this particular issue. The things
we gonna implement on top is rather an optimization on memory
usage. So i would rather open another issue to address it.

> > ---
> >  src/lib/core/fiber.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> > index abd6c6b11..f16ac873f 100644
> > --- a/src/lib/core/fiber.c
> > +++ b/src/lib/core/fiber.c
> > @@ -93,9 +93,9 @@ static int stack_direction;
> >  
> >  enum {
> >  	/* The minimum allowable fiber stack size in bytes */
> > -	FIBER_STACK_SIZE_MINIMAL = 16384,
> > +	FIBER_STACK_SIZE_MINIMAL = 4 << 12,
> >  	/* Default fiber stack size in bytes */
> > -	FIBER_STACK_SIZE_DEFAULT = 65536
> > +	FIBER_STACK_SIZE_DEFAULT = 128 << 12
> 
> I don't like using '<< 12' for stack sizes. Why 12? Because the page
> size is typically 4K. I wouldn't count on that - it's userspace so it
> looks confusing.

Because madvise works with 4K pages and such record force
a code reader to pay attention that we're counting by
pages. Actually once I make this values configurable
I'll use "nr_page * page_size" record.

Volodya, if these are only nits you worries then fine
I can easily fix them :)

  reply	other threads:[~2019-03-13 13:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-12 22:47 [PATCH v5 0/2] " Cyrill Gorcunov
2019-03-12 22:47 ` [PATCH 1/3] " Cyrill Gorcunov
2019-03-13 13:42   ` Vladimir Davydov
2019-03-13 13:53     ` Cyrill Gorcunov [this message]
2019-03-13 14:13       ` Vladimir Davydov
2019-03-12 22:47 ` [PATCH 2/3] lib/core/fiber: Mark stack as unneeded on creation Cyrill Gorcunov
2019-03-12 22:47 ` [PATCH 3/3] lib/core/fiber: Put watermarks into stack to track its usage Cyrill Gorcunov
2019-03-13 13:52   ` Vladimir Davydov
2019-03-13 14:00     ` Cyrill Gorcunov
2019-03-13 14:17       ` Vladimir Davydov
2019-03-13 16:15         ` Cyrill Gorcunov

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=20190313135307.GN10420@uranus.lan \
    --to=gorcunov@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 1/3] lib/core/fiber: Increase default stack size' \
    /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