From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 106D970C99; Thu, 2 Sep 2021 22:47:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 106D970C99 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1630612058; bh=rmcq3SHVvzcA4yMieivduNZIdoEwFu3eGLRZnV4zoVA=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=KAkm07CKUGOUNiCcqbWmv4BN7/CDPBwPdTKAv+XzchTPVigRGCtqF+9GQ/NCf44Bd zw18Equnn0YytN+GoEZfEPglWx7SVmr02G/h/72I2QjInrn8LGUMy+URSwabtYlWVQ 2SEB+8ODVolQ1Iw4YdGBYUrCOK3c0awKErigIQNw= Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (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 786EA70C99 for ; Thu, 2 Sep 2021 22:47:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 786EA70C99 Received: by smtp17.mail.ru with esmtpa (envelope-from ) id 1mLsgR-00019p-DE; Thu, 02 Sep 2021 22:47:35 +0300 To: v.shpilevoy@tarantool.org, imun@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: <7ef7fcfec0d2c78721ea17376cbfe0d0f231798c.1630606077.git.babinoleg@mail.ru> Message-ID: <3bca1fac-7305-961c-7e4d-439652bbc815@tarantool.org> Date: Thu, 2 Sep 2021 22:47:34 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <7ef7fcfec0d2c78721ea17376cbfe0d0f231798c.1630606077.git.babinoleg@mail.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D96C1EA41D18F4D5B144C25AA2C86F19FE5C2012F6F3B614182A05F5380850402E16A7D88956B11047ECBFD7487705EC0C2494C0260512EE3257A95E6CC4376D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE71EAE3A63419E5AEDEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E8D1333770DC60CDEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F213BEB0A8AB659918DE02BDD913211CBBCC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC82FFDA4F57982C5F4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C4E7D9683544204AF9735652A29929C6C4AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C32ECB75B566AFDDCCBA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7994FE22CF3C16DE0731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458F0AFF96BAACF4158235E5A14AD4A4A4625E192CAD1D9E79DB8BF922E402C092CB11786249932374B X-C1DE0DAB: 0D63561A33F958A5CC2A0BC42240DA1B165F64584B9410C31F11D1BCD68BE7DED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752546FE575EB473F1410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D1AE09A115117C962F38D0AF7AE5448790F36B7EAA45A1ABCCE15C7855E2347E101D1412C823A0D71D7E09C32AA3244C3E6F6E7EAF9EB4E8EC12196660596D1CF522A1CF68F4BE05FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioja9FuITQvsRrRWPPfSsHxrA== X-Mailru-Sender: 583F1D7ACE8F49BD1042885CEC987B6B72E217648BC9C72C47ECBFD7487705EC52685D71546B8E9E7019711D9D5B048E1458020726E2BC9FD5ECBA0B92C0A936CDC7563AA7CEBD287402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Oleg Babin via Tarantool-patches Reply-To: Oleg Babin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Applied and force-pushed small fix: diff --git a/src/lua/fiber.c b/src/lua/fiber.c index ab0e895eb..f82f4032f 100644 --- a/src/lua/fiber.c +++ b/src/lua/fiber.c @@ -659,7 +659,7 @@ lbox_fiber_storage(struct lua_State *L)  {         struct fiber *f = lbox_checkfiber(L, 1);         int storage_ref = f->storage.lua.storage_ref; -       if (storage_ref <= 0) { +       if (storage_ref == LUA_NOREF) {                 lua_newtable(L); /* create local storage on demand */                 storage_ref = luaL_ref(L, LUA_REGISTRYINDEX);                 f->storage.lua.storage_ref = storage_ref; On 02.09.2021 21:13, Oleg Babin via Tarantool-patches wrote: > From: Oleg Babin > > This patch reworks approach to fiber management in Lua. Before > this patch each action that should return fiber led to new > userdata creation that was quite slow and made GC suffer. This > patch introduces new field in struct fiber to store a reference to > userdata that was created once for a fiber. It allows speedup > operations as fiber.self() and fiber.id(). > Simple benchmark shows that access to fiber storage is faster in > two times, fiber.find() - 2-3 times and fiber.new/create functions > don't have any changes. > --- > src/lib/core/fiber.c | 5 ++ > src/lib/core/fiber.h | 5 ++ > src/lua/fiber.c | 113 +++++++++++++------------------------------ > 3 files changed, 44 insertions(+), 79 deletions(-) > > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c > index 588b78504..39b67f940 100644 > --- a/src/lib/core/fiber.c > +++ b/src/lib/core/fiber.c > @@ -32,6 +32,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -895,6 +896,8 @@ fiber_recycle(struct fiber *fiber) > fiber->f = NULL; > fiber->wait_pad = NULL; > memset(&fiber->storage, 0, sizeof(fiber->storage)); > + fiber->storage.lua.storage_ref = LUA_NOREF; > + fiber->storage.lua.fid_ref = LUA_NOREF; > unregister_fid(fiber); > fiber->fid = 0; > region_free(&fiber->gc); > @@ -1236,6 +1239,8 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr, > return NULL; > } > memset(fiber, 0, sizeof(struct fiber)); > + fiber->storage.lua.storage_ref = LUA_NOREF; > + fiber->storage.lua.fid_ref = LUA_NOREF; > > if (fiber_stack_create(fiber, &cord()->slabc, > fiber_attr->stack_size)) { > diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h > index 593847df7..2d4443544 100644 > --- a/src/lib/core/fiber.h > +++ b/src/lib/core/fiber.h > @@ -623,6 +623,11 @@ struct fiber { > * Should not be used in other fibers. > */ > struct lua_State *stack; > + /** > + * Optional reference to userdata > + * represented current fiber id in Lua. > + */ > + int fid_ref; > /** > * Optional fiber.storage Lua reference. > */ > diff --git a/src/lua/fiber.c b/src/lua/fiber.c > index 5575f2079..ab0e895eb 100644 > --- a/src/lua/fiber.c > +++ b/src/lua/fiber.c > @@ -87,27 +87,28 @@ luaL_testcancel(struct lua_State *L) > static const char *fiberlib_name = "fiber"; > > /** > - * @pre: stack top contains a table > - * @post: sets table field specified by name of the table on top > - * of the stack to a weak kv table and pops that weak table. > + * Trigger invoked when the fiber has stopped execution of its > + * current request. Only purpose - delete storage.lua.fid_ref and > + * storage.lua.storage_ref keeping a reference of Lua > + * fiber and fiber.storage objects. Unlike Lua stack, > + * Lua fiber storage may be created not only for fibers born from > + * Lua land. For example, an IProto request may execute a Lua > + * function, which can create the storage. Trigger guarantees, > + * that even for non-Lua fibers the Lua storage is destroyed. > */ > -static void > -lbox_create_weak_table(struct lua_State *L, const char *name) > +static int > +lbox_fiber_on_stop(struct trigger *trigger, void *event) > { > - lua_newtable(L); > - /* and a metatable */ > - lua_newtable(L); > - /* weak keys and values */ > - lua_pushstring(L, "kv"); > - /* pops 'kv' */ > - lua_setfield(L, -2, "__mode"); > - /* pops the metatable */ > - lua_setmetatable(L, -2); > - /* assigns and pops table */ > - lua_setfield(L, -2, name); > - /* gets memoize back. */ > - lua_getfield(L, -1, name); > - assert(! lua_isnil(L, -1)); > + struct fiber *f = event; > + int storage_ref = f->storage.lua.storage_ref; > + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref); > + f->storage.lua.storage_ref = LUA_NOREF; > + int ref = f->storage.lua.fid_ref; > + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, ref); > + f->storage.lua.fid_ref = LUA_NOREF; > + trigger_clear(trigger); > + free(trigger); > + return 0; > } > > /** > @@ -116,42 +117,26 @@ lbox_create_weak_table(struct lua_State *L, const char *name) > static void > lbox_pushfiber(struct lua_State *L, struct fiber *f) > { > - /* > - * Use 'memoize' pattern and keep a single userdata for > - * the given fiber. This is important to not run __gc > - * twice for a copy of an attached fiber -- __gc should > - * not remove attached fiber's coro prematurely. > - */ > - luaL_getmetatable(L, fiberlib_name); > - lua_getfield(L, -1, "memoize"); > - if (lua_isnil(L, -1)) { > - /* first access - instantiate memoize */ > - /* pop the nil */ > - lua_pop(L, 1); > - /* create memoize table */ > - lbox_create_weak_table(L, "memoize"); > - } > - /* Find out whether the fiber is already in the memoize table. */ > - uint64_t fid = f->fid; > - luaL_pushuint64(L, fid); > - lua_gettable(L, -2); > - if (lua_isnil(L, -1)) { > - /* no userdata for fiber created so far */ > - /* pop the nil */ > - lua_pop(L, 1); > - /* push the key back */ > - luaL_pushuint64(L, fid); > + int ref = f->storage.lua.fid_ref; > + if (ref == LUA_NOREF) { > + struct trigger *t = malloc(sizeof(*t)); > + if (t == NULL) { > + diag_set(OutOfMemory, sizeof(*t), "malloc", "t"); > + luaT_error(L); > + } > + trigger_create(t, lbox_fiber_on_stop, NULL, (trigger_f0)free); > + trigger_add(&f->on_stop, t); > + > + uint64_t fid = f->fid; > /* create a new userdata */ > uint64_t *ptr = lua_newuserdata(L, sizeof(*ptr)); > *ptr = fid; > luaL_getmetatable(L, fiberlib_name); > lua_setmetatable(L, -2); > - /* memoize it */ > - lua_settable(L, -3); > - luaL_pushuint64(L, fid); > - /* get it back */ > - lua_gettable(L, -2); > + ref = luaL_ref(L, LUA_REGISTRYINDEX); > + f->storage.lua.fid_ref = ref; > } > + lua_rawgeti(L, LUA_REGISTRYINDEX, ref); > } > > static struct fiber * > @@ -669,42 +654,12 @@ lbox_fiber_name(struct lua_State *L) > } > } > > -/** > - * Trigger invoked when the fiber has stopped execution of its > - * current request. Only purpose - delete storage.lua.storage_ref keeping > - * a reference of Lua fiber.storage object. Unlike Lua stack, > - * Lua fiber storage may be created not only for fibers born from > - * Lua land. For example, an IProto request may execute a Lua > - * function, which can create the storage. Trigger guarantees, > - * that even for non-Lua fibers the Lua storage is destroyed. > - */ > -static int > -lbox_fiber_on_stop(struct trigger *trigger, void *event) > -{ > - struct fiber *f = (struct fiber *) event; > - int storage_ref = f->storage.lua.storage_ref; > - assert(storage_ref > 0); > - luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref); > - f->storage.lua.storage_ref = LUA_NOREF; > - trigger_clear(trigger); > - free(trigger); > - return 0; > -} > - > static int > lbox_fiber_storage(struct lua_State *L) > { > struct fiber *f = lbox_checkfiber(L, 1); > int storage_ref = f->storage.lua.storage_ref; > if (storage_ref <= 0) { > - struct trigger *t = (struct trigger *) > - malloc(sizeof(*t)); > - if (t == NULL) { > - diag_set(OutOfMemory, sizeof(*t), "malloc", "t"); > - return luaT_error(L); > - } > - trigger_create(t, lbox_fiber_on_stop, NULL, (trigger_f0) free); > - trigger_add(&f->on_stop, t); > lua_newtable(L); /* create local storage on demand */ > storage_ref = luaL_ref(L, LUA_REGISTRYINDEX); > f->storage.lua.storage_ref = storage_ref;