Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org,
	Alexander Turenko <alexander.turenko@tarantool.org>
Subject: [Tarantool-patches] [PATCH 3/3] lua: expose temporary Lua state for iproto calls
Date: Mon,  1 Jun 2020 21:10:13 +0300	[thread overview]
Message-ID: <353b8524edba38a68642a10d5b6bd6b033477e33.1591028838.git.alexander.turenko@tarantool.org> (raw)
In-Reply-To: <cover.1591028838.git.alexander.turenko@tarantool.org>

There is code that may save some time and resources for creating a new
Lua state when it is present in the fiber storage of a current fiber.
There are not so much of them: running a Lua trigger and construction of
a next tuple in a merge source.

Before the patch, fiber->storage.lua.stack is filled only for the main
fiber and fibers created from Lua using fiber.create() or fiber.new(),
but not for background fibers (which serve binary protocol requests).

This patch fills fiber->storage.lua.stack for background fibers that
serve a Lua call or eval: we already have this state and nothing prevent
us from exposing it via the fiber storage.

Follows up #4954
---
 src/box/lua/call.c   | 27 +++++++++++++++++++++++++++
 src/lib/core/fiber.h |  8 ++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 6588ec2fa..f69d1adbe 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -537,12 +537,39 @@ box_process_lua(lua_CFunction handler, struct execute_lua_ctx *ctx,
 	port_lua_create(ret, L);
 	((struct port_lua *) ret)->ref = coro_ref;
 
+	/*
+	 * A code that need a temporary fiber-local Lua state may
+	 * save some time and resources for creating a new state
+	 * and use this one.
+	 */
+	bool has_lua_stack = fiber()->storage.lua.stack != NULL;
+	if (! has_lua_stack)
+		fiber()->storage.lua.stack = L;
+
 	lua_pushcfunction(L, handler);
 	lua_pushlightuserdata(L, ctx);
 	if (luaT_call(L, 1, LUA_MULTRET) != 0) {
+		if (! has_lua_stack)
+			fiber()->storage.lua.stack = NULL;
 		port_lua_destroy(ret);
 		return -1;
 	}
+
+	/*
+	 * Since this field is optional we're not obligated to
+	 * keep it until the Lua state will be unreferenced in
+	 * port_lua_destroy().
+	 *
+	 * There is no much sense to keep it beyond the Lua call,
+	 * so let's zap now.
+	 *
+	 * But: keep the stack if it was present before the call,
+	 * because it would be counter-intuitive if the existing
+	 * state pointer would be zapped after this function call.
+	 */
+	if (! has_lua_stack)
+		fiber()->storage.lua.stack = NULL;
+
 	return 0;
 }
 
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index cd9346a55..db68fb47e 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -496,8 +496,12 @@ struct fiber {
 		struct credentials *credentials;
 		struct txn *txn;
 		/**
-		 * Lua stack and the optional
-		 * fiber.storage Lua reference.
+		 * Optional Lua state (may be NULL). Useful as a
+		 * temporary Lua state to save time and resources
+		 * on creating it. Should not be used in other
+		 * fibers.
+		 *
+		 * Optional fiber.storage Lua reference.
 		 */
 		struct {
 			struct lua_State *stack;
-- 
2.25.0

  parent reply	other threads:[~2020-06-01 18:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 18:10 [Tarantool-patches] [PATCH 0/3] Merger's NULL defererence Alexander Turenko
2020-06-01 18:10 ` [Tarantool-patches] [PATCH 1/3] merger: drop luaL prefix where contract allows it Alexander Turenko
2020-06-02 22:47   ` Vladislav Shpilevoy
2020-06-07 16:57     ` Alexander Turenko
2020-06-11 16:17       ` Vladislav Shpilevoy
2020-06-16 11:59       ` Igor Munkin
2020-06-17 17:53         ` Alexander Turenko
2020-06-01 18:10 ` [Tarantool-patches] [PATCH 2/3] merger: fix NULL dereference when called via iproto Alexander Turenko
2020-06-02 22:48   ` Vladislav Shpilevoy
2020-06-07 16:58     ` Alexander Turenko
2020-06-11 16:18       ` Vladislav Shpilevoy
2020-06-17 17:53         ` Alexander Turenko
2020-06-18 22:47           ` Vladislav Shpilevoy
2020-06-01 18:10 ` Alexander Turenko [this message]
2020-06-02 22:48   ` [Tarantool-patches] [PATCH 3/3] lua: expose temporary Lua state for iproto calls Vladislav Shpilevoy
2020-06-07 16:58     ` Alexander Turenko
2020-06-02 22:47 ` [Tarantool-patches] [PATCH 0/3] Merger's NULL defererence Vladislav Shpilevoy
2020-06-07 17:17   ` Alexander Turenko
2020-06-07 16:58 ` [Tarantool-patches] [PATCH 2.5/3] merger: clean fiber-local Lua stack after next() Alexander Turenko
2020-06-11 16:20   ` Vladislav Shpilevoy
2020-06-17 17:53     ` Alexander Turenko
2020-06-18 22:48       ` Vladislav Shpilevoy
2020-06-19  7:41         ` Alexander Turenko
2020-06-17 17:54 ` [Tarantool-patches] [PATCH 0/3] Merger's NULL defererence Alexander Turenko

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=353b8524edba38a68642a10d5b6bd6b033477e33.1591028838.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 3/3] lua: expose temporary Lua state for iproto calls' \
    /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