From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: v.shpilevoy@tarantool.org, imun@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once Date: Thu, 2 Sep 2021 22:47:34 +0300 [thread overview] Message-ID: <3bca1fac-7305-961c-7e4d-439652bbc815@tarantool.org> (raw) In-Reply-To: <7ef7fcfec0d2c78721ea17376cbfe0d0f231798c.1630606077.git.babinoleg@mail.ru> 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 <babinoleg@mail.ru> > > 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 <trivia/config.h> > #include <trivia/util.h> > +#include <lauxlib.h> > #include <errno.h> > #include <stdio.h> > #include <stdlib.h> > @@ -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;
next prev parent reply other threads:[~2021-09-02 19:47 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-02 18:13 [Tarantool-patches] [PATCH v2 0/3] " Oleg Babin via Tarantool-patches 2021-09-02 18:13 ` [Tarantool-patches] [PATCH v2 1/3] fiber: rename ref to storage_ref Oleg Babin via Tarantool-patches 2021-09-02 18:13 ` [Tarantool-patches] [PATCH v2 2/3] fiber: pass struct fiber into lbox_pushfiber instead of id Oleg Babin via Tarantool-patches 2021-09-02 18:13 ` [Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once Oleg Babin via Tarantool-patches 2021-09-02 19:47 ` Oleg Babin via Tarantool-patches [this message] 2021-09-03 12:23 ` Igor Munkin via Tarantool-patches 2021-09-03 20:48 ` Oleg Babin via Tarantool-patches 2021-09-05 15:07 ` Vladislav Shpilevoy via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=3bca1fac-7305-961c-7e4d-439652bbc815@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imun@tarantool.org \ --cc=olegrok@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox