[Tarantool-patches] [PATCH v2 2/3] fiber: destroy fiber.storage created by iproto
Georgy Kirichenko
georgy at tarantool.org
Mon Jan 20 22:15:50 MSK 2020
On Monday, 20 January 2020 10:22:34 MSK Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy at tarantool.org> [20/01/20 09:59]:
> > > Why did you have to add so many invocation points to
> > > fiber_on_stop() rather than simply adding fiber_on_stop invocation to
> > > fiber_pool.c?
> >
> > We already discussed that in the previous patch version. We decided
> > to move cleanup to iproto.cc, because it depends on when a request
> > ends. Fiber pool knows nothing about requests. Iproto.cc is request
> > processing layer, and this is the right place for request data
> > destruction.
>
> True, but since you abstracted out the destruction via an opaque
> trigger, why not move the invocation of the trigger to fiber pool?
> fiber pool has most knowledge about fiber life cycle, so it seems
> natural to invoke the triggers in it - it will tie the *timing* to
> fiber pool, but not what's going on inside the trigger.
>
> Thoughts?
I agree with Kostja's comment, if a fiber pool is only an optimization then
there should not be any visible difference between code invocation inside a
standalone fiber and a fiber pool member and this point includes fiber pool
clearance.
>
> > > I would move this !rlist_empty check to fiber_on_stop and add a
> > > comment why we explicitly check for the list first.
> >
> > I doubt it really helps, but ok.
> >
> > ==========================================================================
> > ======
> >
> > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> > index 634b3d1b0..354749549 100644
> > --- a/src/lib/core/fiber.c
> > +++ b/src/lib/core/fiber.c
> > @@ -328,6 +328,12 @@ fiber_attr_getstacksize(struct fiber_attr
> > *fiber_attr)
> >
> > void
> > fiber_on_stop(struct fiber *f)
> > {
> >
> > + /*
> > + * The most common case is when the list is empty. Do an
> > + * inlined check before calling trigger_run().
> > + */
> > + if (rlist_empty(&f->on_stop))
> > + return;
> >
> > if (trigger_run(&f->on_stop, f) != 0)
> >
> > panic("On_stop triggers can't fail");
> >
> > /*
>
> thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200120/ca138e72/attachment.sig>
More information about the Tarantool-patches
mailing list