From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D336846970E for ; Mon, 20 Jan 2020 22:15:51 +0300 (MSK) From: Georgy Kirichenko Date: Mon, 20 Jan 2020 22:15:50 +0300 Message-ID: <4533948.31r3eYUQgx@localhost> In-Reply-To: <20200120072234.GB19835@atlas> References: <20200120072234.GB19835@atlas> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart4208948.LvFx2qVVIh"; micalg="pgp-sha256"; protocol="application/pgp-signature" Subject: Re: [Tarantool-patches] [PATCH v2 2/3] fiber: destroy fiber.storage created by iproto List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov , Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org --nextPart4208948.LvFx2qVVIh Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" On Monday, 20 January 2020 10:22:34 MSK Konstantin Osipov wrote: > * Vladislav Shpilevoy [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! --nextPart4208948.LvFx2qVVIh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEECXG+Yw5ArYcP8x5wDVHWG5PoUw4FAl4l/GYACgkQDVHWG5Po Uw7IYggAlP18vG8FjGs/c/SqzoKz0Q0s5D3ZDe1b9Zu2t09Ntfz++LoUXe1Fyfub zcj2E6AFjD+xFTmZKkC+oguP4yET7EgdUfGKsz4hVuZ++quhAOYYr8zXRkLNNRt8 z0QF33muLRzQseS0LdHItO0z2Cp30rAwuksn87WxpU8PP4h6zG15linyPEGS2kCB a4YsOqB8kpSZ71afRVv9Nlo5jGCujB79fikO6BaYsp9HtjJyqyyIbSxK5qhhhFoq oJlyEbIXH/DaUas18Cn/vh3dNu2mqVvSgfXPhJuoJQUf49Qh9LioFah5AsOWNT3u Lcb8lB4yIACdtQpYnHpqnKW1tUC5uw== =1veR -----END PGP SIGNATURE----- --nextPart4208948.LvFx2qVVIh--