Tarantool development patches archive
 help / color / mirror / Atom feed
From: Egor Elchinov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: gorcunov@tarantool.org, alyapunov@tarantool.org
Cc: tarantool-patches@dev.tarantool.org,
	Egor Elchinov <elchinov.es@gmail.com>
Subject: [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace
Date: Fri,  9 Jul 2021 14:03:51 +0300	[thread overview]
Message-ID: <fdc0e2385d01eff0baf2239f1b40d54b4fbb52de.1625827535.git.elchinov.es@gmail.com> (raw)
In-Reply-To: <cover.1625827535.git.elchinov.es@gmail.com>

From: Egor Elchinov <elchinov.es@gmail.com>

For now Lua backtrace is fully collected
at fiber creation moment and stored in a
dynamically allocated buffer.

This patch also adds option enabling to turn on and off
the collection of backtraces of parent fibers when
creating new fibers.

Needed for: #4002

@TarantoolBot document
Title: new option for fiber parent backtrace

`fiber.parent_bt_enable()` and `fiber.parent_bt_disable()`
turn on and off the ability to trace fiber creation stacks.
If present this option may cause up to 40 per cent
slowdown of fiber creation.
By default parent backtrace is disabled.

Note that fibers created while this option was disabled
lack their parent backtraces in `fiber.info()` forever.
---
 src/lib/core/fiber.c                          |  29 ++-
 src/lib/core/fiber.h                          |  18 ++
 src/lua/fiber.c                               | 200 ++++++++++++++++--
 src/lua/fiber.h                               |  19 ++
 .../gh-4002-fiber-creation-backtrace.result   |  21 ++
 .../gh-4002-fiber-creation-backtrace.test.lua |   7 +
 6 files changed, 280 insertions(+), 14 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 924ff3c82..dd26e2f13 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -220,6 +220,10 @@ fiber_mprotect(void *addr, size_t len, int prot)
 static __thread bool fiber_top_enabled = false;
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+static __thread bool fiber_parent_bt_enabled = false;
+#endif /* ENABLE_BACKTRACE */
+
 /**
  * An action performed each time a context switch happens.
  * Used to count each fiber's processing time.
@@ -1265,7 +1269,10 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
 	fiber->fid = cord->next_fid;
 	fiber_set_name(fiber, name);
 #if ENABLE_BACKTRACE
-	backtrace_collect_ip(fiber->parent_bt_ip_buf, FIBER_PARENT_BT_MAX);
+	if (fiber_parent_bt_enabled) {
+		backtrace_collect_ip(fiber->parent_bt_ip_buf,
+				     FIBER_PARENT_BT_MAX);
+	}
 #endif /* ENABLE_BACKTRACE */
 	register_fid(fiber);
 	fiber->csw = 0;
@@ -1414,6 +1421,26 @@ fiber_top_disable(void)
 }
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+bool
+fiber_parent_bt_is_enabled(void)
+{
+	return fiber_parent_bt_enabled;
+}
+
+void
+fiber_parent_bt_enable(void)
+{
+	fiber_parent_bt_enabled = true;
+}
+
+void
+fiber_parent_bt_disable(void)
+{
+	fiber_parent_bt_enabled = false;
+}
+#endif /* ENABLE_BACKTRACE */
+
 size_t
 box_region_used(void)
 {
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index beed58866..477b68e4c 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -528,6 +528,7 @@ struct txn;
 struct credentials;
 struct lua_State;
 struct ipc_wait_pad;
+struct parent_bt_lua;
 
 struct fiber {
 	coro_context ctx;
@@ -634,6 +635,12 @@ struct fiber {
 			 * Optional fiber.storage Lua reference.
 			 */
 			int ref;
+#if ENABLE_BACKTRACE
+			/**
+			 * Lua parent backtrace (may be NULL)
+			 */
+			struct parent_bt_lua *parent_bt;
+#endif /* ENABLE_BACKTRACE */
 		} lua;
 		/**
 		 * Iproto sync.
@@ -871,6 +878,17 @@ void
 fiber_top_disable(void);
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+bool
+fiber_parent_bt_is_enabled(void);
+
+void
+fiber_parent_bt_enable(void);
+
+void
+fiber_parent_bt_disable(void);
+#endif /* ENABLE_BACKTRACE */
+
 /** Useful for C unit tests */
 static inline int
 fiber_c_invoke(fiber_func f, va_list ap)
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 7b21361d4..8e96059b4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -193,6 +193,18 @@ struct lua_fiber_tb_ctx {
 	int tb_frame;
 };
 
+/**
+ * Lua parent traceback context.
+ */
+struct lua_parent_tb_ctx {
+	/* Lua stack to push values. */
+	struct lua_State *L;
+	/* Lua parent backtrace. */
+	struct parent_bt_lua *bt;
+	/* Count of traced frames (both C and Lua). */
+	int tb_frame;
+};
+
 #ifdef ENABLE_BACKTRACE
 static void
 dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
@@ -209,6 +221,22 @@ dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
 	lua_settable(L, -3);
 }
 
+static void
+dump_parent_lua_frame(struct lua_State *L, const char *name, const char *src,
+		      int currentline, int tb_frame)
+{
+	char buf[512];
+	snprintf(buf, sizeof(buf), "%s in %s at line %i",
+		 name != NULL ? name : "(unnamed)",
+		 src, currentline);
+	lua_pushnumber(L, tb_frame);
+	lua_newtable(L);
+	lua_pushstring(L, "L");
+	lua_pushstring(L, buf);
+	lua_settable(L, -3);
+	lua_settable(L, -3);
+}
+
 static int
 fiber_backtrace_cb(int frameno, void *frameret, const char *func, size_t offset, void *cb_ctx)
 {
@@ -260,7 +288,115 @@ fiber_backtrace_cb(int frameno, void *frameret, const char *func, size_t offset,
 	lua_settable(L, -3);
 	return 0;
 }
-#endif
+
+static int
+fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
+			  size_t offset, void *cb_ctx)
+{
+	int lua_frame = 0;
+	struct lua_parent_tb_ctx *tb_ctx = (struct lua_parent_tb_ctx *)cb_ctx;
+	struct parent_bt_lua *bt = tb_ctx->bt;
+	struct lua_State *L = tb_ctx->L;
+	/*
+	 * There is impossible to get func == NULL until
+	 * https://github.com/tarantool/tarantool/issues/5326
+	 * will not resolved, but is possible afterwards.
+	 */
+	if (bt != NULL && func != NULL && strstr(func, "lj_BC_FUNCC") == func) {
+		/* We are in the LUA vm. */
+		while (lua_frame < bt->cnt) {
+			tb_ctx->tb_frame++;
+			dump_parent_lua_frame(L, bt->names[lua_frame],
+					      bt->sources[lua_frame],
+					      bt->lines[lua_frame],
+					      tb_ctx->tb_frame);
+			lua_frame++;
+		}
+	}
+	char buf[512];
+	int l = snprintf(buf, sizeof(buf), "#%-2d %p in ", frameno, frameret);
+	if (func)
+		snprintf(buf + l, sizeof(buf) - l, "%s+%zu", func, offset);
+	else
+		snprintf(buf + l, sizeof(buf) - l, "?");
+	tb_ctx->tb_frame++;
+	lua_pushnumber(L, tb_ctx->tb_frame);
+	lua_newtable(L);
+	lua_pushstring(L, "C");
+	lua_pushstring(L, buf);
+	lua_settable(L, -3);
+	lua_settable(L, -3);
+	return 0;
+}
+
+static int
+fiber_parent_bt_init(struct fiber *f, struct lua_State *L)
+{
+	int lua_frame = 0, tb_frame = 0;
+	lua_Debug ar;
+	struct parent_bt_lua *bt = NULL;
+
+	bt = (struct parent_bt_lua *)malloc(sizeof(*bt));
+	if (bt == NULL){
+		diag_set(OutOfMemory, sizeof(*bt), "malloc", "bt");
+		return 1;
+	}
+
+	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
+	       lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Skip all following C-frames. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what != 'C')
+			break;
+		if (ar.name != NULL) {
+			/* Dump frame if it is a C built-in call. */
+			bt->lines[tb_frame] = ar.currentline;
+			memset(bt->names[tb_frame], 0,
+			       PARENT_BT_LUA_NAME_MAX);
+			strncpy(bt->names[tb_frame], ar.name,
+				PARENT_BT_LUA_NAME_MAX - 1);
+			memset(bt->sources[tb_frame], 0,
+			       PARENT_BT_LUA_NAME_MAX);
+			strncpy(bt->sources[tb_frame], ar.source,
+				PARENT_BT_LUA_NAME_MAX - 1);
+			tb_frame++;
+		}
+		lua_frame++;
+	}
+	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
+	       lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Trace Lua frame. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what == 'C')
+			break;
+		bt->lines[tb_frame] = ar.currentline;
+		memset(bt->names[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
+		if (ar.name != NULL) {
+			strncpy(bt->names[tb_frame], ar.name,
+				PARENT_BT_LUA_NAME_MAX - 1);
+		} else {
+			strncpy(bt->names[tb_frame], "(unnamed)",
+				PARENT_BT_LUA_NAME_MAX - 1);
+		}
+		memset(bt->sources[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
+		strncpy(bt->sources[tb_frame], ar.source,
+			PARENT_BT_LUA_NAME_MAX - 1);
+		tb_frame++;
+		lua_frame++;
+	}
+
+	bt->cnt = tb_frame;
+	f->storage.lua.parent_bt = bt;
+
+	return 0;
+}
+
+static void
+fiber_parent_bt_free(struct fiber *f)
+{
+	free(f->storage.lua.parent_bt);
+}
+#endif /* ENABLE_BACKTRACE */
 
 static int
 lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
@@ -299,6 +435,7 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 	if (backtrace) {
 #ifdef ENABLE_BACKTRACE
 		struct lua_fiber_tb_ctx tb_ctx;
+		struct lua_parent_tb_ctx parent_tb_ctx;
 		tb_ctx.L = L;
 		tb_ctx.R = f->storage.lua.stack;
 		tb_ctx.lua_frame = 0;
@@ -309,15 +446,18 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 				  f != fiber() ? &f->ctx : NULL, &tb_ctx);
 		lua_settable(L, -3);
 
-		tb_ctx.lua_frame = 0;
-		tb_ctx.tb_frame = 0;
-		tb_ctx.R = NULL;
-		lua_pushstring(L, "backtrace_parent");
-		lua_newtable(L);
-		backtrace_foreach_ip(fiber_backtrace_cb,
-				     f->parent_bt_ip_buf,
-				     FIBER_PARENT_BT_MAX, &tb_ctx);
-		lua_settable(L, -3);
+		if (fiber_parent_bt_is_enabled()) {
+			parent_tb_ctx.L = L;
+			parent_tb_ctx.bt = f->storage.lua.parent_bt;
+			parent_tb_ctx.tb_frame = 0;
+			lua_pushstring(L, "backtrace_parent");
+			lua_newtable(L);
+			backtrace_foreach_ip(fiber_parent_backtrace_cb,
+					     f->parent_bt_ip_buf,
+					     FIBER_PARENT_BT_MAX,
+					     &parent_tb_ctx);
+			lua_settable(L, -3);
+		}
 #endif /* ENABLE_BACKTRACE */
 	}
 	return 0;
@@ -441,6 +581,22 @@ lbox_do_backtrace(struct lua_State *L)
 	}
 	return true;
 }
+
+static int
+lbox_fiber_parent_bt_enable(struct lua_State *L)
+{
+	(void) L;
+	fiber_parent_bt_enable();
+	return 0;
+}
+
+static int
+lbox_fiber_parent_bt_disable(struct lua_State *L)
+{
+	(void) L;
+	fiber_parent_bt_disable();
+	return 0;
+}
 #endif /* ENABLE_BACKTRACE */
 
 /**
@@ -481,10 +637,14 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap)
 	 * We can unref child stack here,
 	 * otherwise we have to unref child stack in join
 	 */
-	if (f->flags & FIBER_IS_JOINABLE)
+	if (f->flags & FIBER_IS_JOINABLE) {
 		lua_pushinteger(L, coro_ref);
-	else
+	} else {
+#ifdef ENABLE_BACKTRACE
+		fiber_parent_bt_free(f);
+#endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	}
 
 	return result;
 }
@@ -506,6 +666,12 @@ fiber_create(struct lua_State *L)
 		luaT_error(L);
 	}
 
+#ifdef ENABLE_BACKTRACE
+	// TODO: error handling
+	if (fiber_parent_bt_is_enabled())
+		fiber_parent_bt_init(f, L);
+#endif
+
 	/* Move the arguments to the new coro */
 	lua_xmove(L, child_L, lua_gettop(L));
 	/* XXX: 'fiber' is leaked if this throws a Lua error. */
@@ -886,8 +1052,12 @@ lbox_fiber_join(struct lua_State *L)
 			lua_xmove(child_L, L, num_ret);
 		}
 	}
-	if (child_L != NULL)
+	if (child_L != NULL) {
+#ifdef ENABLE_BACKTRACE
+		fiber_parent_bt_free(fiber);
+#endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	}
 	return num_ret + 1;
 }
 
@@ -941,6 +1111,10 @@ static const struct luaL_Reg fiberlib[] = {
 	{"top_enable", lbox_fiber_top_enable},
 	{"top_disable", lbox_fiber_top_disable},
 #endif /* ENABLE_FIBER_TOP */
+#if ENABLE_BACKTRACE
+	{"parent_bt_enable", lbox_fiber_parent_bt_enable},
+	{"parent_bt_disable", lbox_fiber_parent_bt_disable},
+#endif /* ENABLE_BACKTRACE */
 	{"sleep", lbox_fiber_sleep},
 	{"yield", lbox_fiber_yield},
 	{"self", lbox_fiber_self},
diff --git a/src/lua/fiber.h b/src/lua/fiber.h
index e29898706..450840fb0 100644
--- a/src/lua/fiber.h
+++ b/src/lua/fiber.h
@@ -36,6 +36,25 @@ extern "C" {
 
 struct lua_State;
 
+/**
+ * Maximal name length (including '\0')
+ * and backtrace length.
+ */
+enum {
+	PARENT_BT_LUA_NAME_MAX = 64,
+	PARENT_BT_LUA_LEN_MAX = 8
+};
+
+/**
+ * Stores lua parent backtrace for fiber.
+ */
+struct parent_bt_lua {
+	int cnt;
+	char names[PARENT_BT_LUA_LEN_MAX][PARENT_BT_LUA_NAME_MAX];
+	char sources[PARENT_BT_LUA_LEN_MAX][PARENT_BT_LUA_NAME_MAX];
+	int lines[PARENT_BT_LUA_LEN_MAX];
+};
+
 /**
 * Initialize box.fiber system
 */
diff --git a/test/app/gh-4002-fiber-creation-backtrace.result b/test/app/gh-4002-fiber-creation-backtrace.result
index 4934b82d6..6a075911d 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.result
+++ b/test/app/gh-4002-fiber-creation-backtrace.result
@@ -47,6 +47,17 @@ baz = function(n) bar(n) end
  | ---
  | ...
 
+baz(10)
+ | ---
+ | ...
+assert(parent_stack_len == -1)
+ | ---
+ | - true
+ | ...
+
+if stack_len ~= -1 then fiber:parent_bt_enable() end
+ | ---
+ | ...
 baz(10)
  | ---
  | ...
@@ -55,3 +66,13 @@ assert(parent_stack_len > 0 or stack_len == -1)
  | - true
  | ...
 
+if stack_len ~= -1 then fiber:parent_bt_disable() end
+ | ---
+ | ...
+baz(10)
+ | ---
+ | ...
+assert(parent_stack_len == -1)
+ | ---
+ | - true
+ | ...
diff --git a/test/app/gh-4002-fiber-creation-backtrace.test.lua b/test/app/gh-4002-fiber-creation-backtrace.test.lua
index 24d41a860..79a516860 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.test.lua
+++ b/test/app/gh-4002-fiber-creation-backtrace.test.lua
@@ -22,6 +22,13 @@ local bar,baz
 bar = function(n) if n ~= 0 then baz(n-1) else fiber.create(foo) end end
 baz = function(n) bar(n) end
 
+baz(10)
+assert(parent_stack_len == -1)
+
+if stack_len ~= -1 then fiber:parent_bt_enable() end
 baz(10)
 assert(parent_stack_len > 0 or stack_len == -1)
 
+if stack_len ~= -1 then fiber:parent_bt_disable() end
+baz(10)
+assert(parent_stack_len == -1)
-- 
2.31.1


  parent reply	other threads:[~2021-07-09 11:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 11:03 [Tarantool-patches] [PATCH v2 0/4] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
2021-07-09 11:03 ` [Tarantool-patches] [PATCH v2 1/4] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
2021-07-12 11:42   ` Cyrill Gorcunov via Tarantool-patches
2021-07-09 11:03 ` Egor Elchinov via Tarantool-patches [this message]
2021-07-12 12:09   ` [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace Cyrill Gorcunov via Tarantool-patches
2021-07-14  9:29     ` Egor Elchinov via Tarantool-patches
2021-07-09 11:03 ` [Tarantool-patches] [PATCH v2 3/4] fiber: refactor lua backtrace routines Egor Elchinov via Tarantool-patches
2021-07-12 12:13   ` Cyrill Gorcunov via Tarantool-patches
2021-07-14  9:42     ` Egor Elchinov via Tarantool-patches
2021-07-09 11:03 ` [Tarantool-patches] [PATCH v2 4/4] fiber: refactor C backtrace and add changelog Egor Elchinov via Tarantool-patches
2021-07-12 12:35   ` Cyrill Gorcunov via Tarantool-patches
2021-07-14  9:42     ` Egor Elchinov 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=fdc0e2385d01eff0baf2239f1b40d54b4fbb52de.1625827535.git.elchinov.es@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alyapunov@tarantool.org \
    --cc=eelchinov@tarantool.org \
    --cc=elchinov.es@gmail.com \
    --cc=gorcunov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace' \
    /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