From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gorcunov@gmail.com>
Return-Path: <gorcunov@gmail.com>
Date: Fri, 22 Feb 2019 13:57:10 +0300
From: Cyrill Gorcunov <gorcunov@gmail.com>
Subject: Re: [tarantool-patches] [RFC] fiber: Increase default stack size
Message-ID: <20190222105710.GX7198@uranus>
References: <20190221212642.GT7198@uranus>
 <20190222104811.pn6lmblrjtdnhi7o@tkn_work_nb>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190222104811.pn6lmblrjtdnhi7o@tkn_work_nb>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: Kirill Yukhin <kyukhin@tarantool.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Georgy Kirichenko <georgy@tarantool.org>, tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Fri, Feb 22, 2019 at 01:48:11PM +0300, Alexander Turenko wrote:
> Hi!
> 
> I look at the patch and found several typos, so wrote this email. See
> below for them.

Thanks!

> Also I wonder whether we can add a metric for a fiber stack size via,
> say, fiber.info()? It'll give us understanding how close we to the
> limits in certain workloads. We already have backtraces there.

Didn't know about fiber.info() feature. I think we can add some stat.

...
> > +
> > +static inline bool
> > +stack_has_wmark(struct fiber *fiber)
> > +{
> > +	void *pos = stack_wmark_pos(fiber);
> > +	return !!!memcmp(pos, stack_wmark, sizeof(stack_wmark));
> 
> Triple negation? Why?

To convert into bool's inverse. IOW, it is a common trick using
double negation to map [0;N] into bools and third negation
for the inverse. Actually on low level it is the same as
return memcmp(...) == 0 but shorter. If it is confusing
I can put traditional comparision.

> > +}
> > +
> > +/*
> > + * Process stack switching: we try to eliminate unneeded
> > + * physical page usage for stack. If fiber never touched
> > + * area after/before the watermark, the OS get ralaxed in
> 
> ralaxed
+1

> > +	 * If fiber ecxeed a watermark, just clear the
> 
> ecxeed

thanks!

> > +
> > +		/*
> > +		 * Set the flag iif've successed,
> 
> iif've

iif is correct, i managed to miss "we" :) thanks