[Tarantool-patches] [PATCH v2 2/3] fiber: destroy fiber.storage created by iproto
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Jan 19 20:32:47 MSK 2020
Hi! Thanks for the review!
>> +/**
>> + * Stop the current fiber after a request is executed to make it
>> + * possible to reuse the fiber for a next request. On_stop
>> + * triggers remove all request-specific data from there.
>> + */
>> +static inline void
>> +tx_fiber_on_stop()
>> +{
>> + fiber_on_stop(fiber());
>> +}
>> +
>> static void
>> tx_process_disconnect(struct cmsg *m)
>> {
>> @@ -1335,6 +1351,7 @@ tx_process_disconnect(struct cmsg *m)
>> if (! rlist_empty(&session_on_disconnect)) {
>> tx_fiber_init(con->session, 0);
>> session_run_on_disconnect_triggers(con->session);
>> + tx_fiber_on_stop();
>> }
>> }
>
> 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.
>> static void
>> fiber_recycle(struct fiber *fiber);
>>
>> @@ -856,8 +869,7 @@ fiber_loop(MAYBE_UNUSED void *data)
>> assert(f != fiber);
>> fiber_wakeup(f);
>> }
>> - if (! rlist_empty(&fiber->on_stop))
>> - trigger_run(&fiber->on_stop, fiber);
>> + fiber_on_stop(fiber);
>
> This was an attempt to optimize a non-inline function call
> for the most common case.
>
> 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");
/*
More information about the Tarantool-patches
mailing list