Tarantool development patches archive
 help / color / mirror / Atom feed
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: [Tarantool-patches] [PATCH v4 4/4] fiber: keep reference to userdata if fiber created once
Date: Wed,  8 Sep 2021 21:20:02 +0300	[thread overview]
Message-ID: <2277f244b5bed55c1a4302310889c7e54ac0872a.1631124536.git.babinoleg@mail.ru> (raw)
In-Reply-To: <cover.1631124536.git.babinoleg@mail.ru>

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 |   2 +
 src/lib/core/fiber.h |   5 ++
 src/lua/fiber.c      | 110 +++++++++++++------------------------------
 3 files changed, 39 insertions(+), 78 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index b7699372a..3e19f131b 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -896,6 +896,7 @@ fiber_recycle(struct fiber *fiber)
 	fiber->wait_pad = NULL;
 	memset(&fiber->storage, 0, sizeof(fiber->storage));
 	fiber->storage.lua.storage_ref = FIBER_LUA_NOREF;
+	fiber->storage.lua.fid_ref = FIBER_LUA_NOREF;
 	unregister_fid(fiber);
 	fiber->fid = 0;
 	region_free(&fiber->gc);
@@ -1238,6 +1239,7 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
 		}
 		memset(fiber, 0, sizeof(struct fiber));
 		fiber->storage.lua.storage_ref = FIBER_LUA_NOREF;
+		fiber->storage.lua.fid_ref = FIBER_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 9584b1611..9ac0cbb9f 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -625,6 +625,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 96aa73b19..12836da69 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -87,27 +87,26 @@ 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;
+	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.storage_ref);
+	f->storage.lua.storage_ref = FIBER_LUA_NOREF;
+	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, f->storage.lua.fid_ref);
+	f->storage.lua.fid_ref = FIBER_LUA_NOREF;
+	trigger_clear(trigger);
+	free(trigger);
+	return 0;
 }
 
 /**
@@ -116,42 +115,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 fid_ref = f->storage.lua.fid_ref;
+	if (fid_ref == FIBER_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);
+		fid_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+		f->storage.lua.fid_ref = fid_ref;
 	}
+	lua_rawgeti(L, LUA_REGISTRYINDEX, fid_ref);
 }
 
 static struct fiber *
@@ -669,41 +652,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;
-	luaL_unref(tarantool_L, LUA_REGISTRYINDEX, storage_ref);
-	f->storage.lua.storage_ref = FIBER_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 == FIBER_LUA_NOREF) {
-		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;
-- 
2.32.0


  parent reply	other threads:[~2021-09-08 18:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 18:19 [Tarantool-patches] [PATCH v4 0/4] " Oleg Babin via Tarantool-patches
2021-09-08 18:19 ` [Tarantool-patches] [PATCH v4 1/4] fiber: rename ref to storage_ref Oleg Babin via Tarantool-patches
2021-09-08 18:20 ` [Tarantool-patches] [PATCH v4 2/4] fiber: pass struct fiber into lbox_pushfiber instead of id Oleg Babin via Tarantool-patches
2021-09-08 18:20 ` [Tarantool-patches] [PATCH v4 3/4] fiber: correctly initialize storage_ref value Oleg Babin via Tarantool-patches
2021-09-08 18:20 ` Oleg Babin via Tarantool-patches [this message]
2021-09-08 20:45 ` [Tarantool-patches] [PATCH v4 0/4] fiber: keep reference to userdata if fiber created once Vladislav Shpilevoy via Tarantool-patches
2021-09-09 10:21 ` Vladimir Davydov via Tarantool-patches
2021-09-09 10:45   ` Oleg Babin 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=2277f244b5bed55c1a4302310889c7e54ac0872a.1631124536.git.babinoleg@mail.ru \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v4 4/4] 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