[Tarantool-patches] [PATCH v2 3/3] fiber: keep reference to userdata if fiber created once
Oleg Babin
olegrok at tarantool.org
Thu Sep 2 22:47:34 MSK 2021
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 at 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;
More information about the Tarantool-patches
mailing list