[tarantool-patches] [RFC v2] fiber: Increase default stack size
Cyrill Gorcunov
gorcunov at gmail.com
Mon Feb 25 18:25:30 MSK 2019
On Mon, Feb 25, 2019 at 05:55:16PM +0300, Vladimir Davydov wrote:
...
> > +
> > +static void
> > +stack_recycle(struct fiber *fiber)
> > +{
> > + if (!fiber->stack || (fiber->flags & FIBER_CUSTOM_STACK))
> > + return;
> > +
> > + /*
> > + * If fiber was too eager for memory, just arm
> > + * a watermark back. Maybe on the next reuse
> > + * we will be able to relax RSS pressure.
> > + */
> > + if (!stack_has_wmark(fiber)) {
> > + fiber->flags &= ~FIBER_MADVISED_STACK;
> > + stack_set_wmark(fiber);
> > + return;
> > + }
>
> Hmm, I don't quite understand why you free the stack only if the fiber
> hasn't touched the watermark. See, there may be thousands of fibers out
> there which are chosen randomly to execute a CALL request that needs a
> lot of stack. If this CALL request happens to land on different fibers
> all the time, we will quickly wind up with a lot of memory being used
> for fiber stacks.
>
> That being said, I think we should unconditionally free the stack with
> madvise() on fiber_recycle() if the watermark was overwritten. This
> would also simplify the patch as you won't need to introduce a new fiber
> flag then (FIBER_MADVISED_STACK).
>
> Am I missing something?
Nope, you're not. I'm trying to minimize madvise() call since it is
a quite expencive call. And I don't like the idea of calling it
unconditionally (seriously, pte range invalidation is not cheap
at all, which involves tlb flushes and such). Letme think more...
> > +
> > + /*
> > + * Set the flag iif we've successed,
> > + * otherwise will try on the next round.
> > + */
> > + if (!madvise(fiber->stack, size, MADV_DONTNEED))
>
> This is nitpicking, but in tarantool we use logical negation (!) only
> with bools. We compare integer values with 0 explicitly.
sure, will update.
More information about the Tarantool-patches
mailing list