[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