[Tarantool-patches] [PATCH] sysprof: add `LUAJIT_DISABLE_SYSPROF` to Makefile

[Tarantool-patches] [PATCH] sysprof: add `LUAJIT_DISABLE_SYSPROF` to Makefile

[Maxim Kokryashkin via Tarantool-patches]

The Makefile.original was not properly updated for sysporf. This
patch fixes this issue by adding the `LUAJIT_DISABLE_SYSPROF`
option to it.

Part of tarantool/tarantool#781
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci

 src/Makefile.original | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/Makefile.original b/src/Makefile.original
index 50b84bfe..dd1c6a76 100644
--- a/src/Makefile.original
+++ b/src/Makefile.original
@@ -116,6 +116,8 @@ XCFLAGS=
 # Disable the memory profiler.
 #XCFLAGS+= -DLUAJIT_DISABLE_MEMPROF
 #
+# Disable the system profiler.
+#XCFLAGS+= -DLUAJIT_DISABLE_SYSPROF
 ##############################################################################
 
 ##############################################################################
-- 
2.35.1

[Tarantool-patches] [PATCH] sysprof: change C configuration API

[Maxim Kokryashkin via Tarantool-patches]

This patch replaces the `lj_sysprof_configure` with individual
configuration handles for each config entry. Also, it chnages the
internal sysprof checks to let users run sysprof in default mode without
configuratons.

Part of tarantool/tarantool#781
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
PR with Tarantool integration:
https://github.com/tarantool/tarantool/pull/7123

 src/lib_misc.c                                | 20 +---
 src/lj_mapi.c                                 | 13 ++-
 src/lj_sysprof.c                              | 91 +++++++++++++------
 src/lj_sysprof.h                              |  6 +-
 src/lmisclib.h                                | 54 +++++------
 .../misclib-sysprof-capi/testsysprof.c        | 17 +---
 .../misclib-sysprof-lapi.test.lua             |  2 +-
 7 files changed, 116 insertions(+), 87 deletions(-)

diff --git a/src/lib_misc.c b/src/lib_misc.c
index e07f1b32..370307a7 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -288,23 +288,10 @@ LJLIB_CF(misc_sysprof_start)
   int status = PROFILE_SUCCESS;
 
   struct luam_Sysprof_Options opt = {};
-  struct luam_Sysprof_Config conf = {};
-
 
   status = parse_options(L, &opt);
-  if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) {
-    return sysprof_error(L, status);
-  }
-
-  conf.writer = buffer_writer_default;
-  conf.on_stop = on_stop_cb_default;
-  conf.backtracer = NULL;
-
-  status = luaM_sysprof_configure(&conf);
-  if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) {
-    on_stop_cb_default(opt.ctx, opt.buf);
+  if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
     return sysprof_error(L, status);
-  }
 
   status = luaM_sysprof_start(L, &opt);
   if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
@@ -454,9 +441,12 @@ LJLIB_CF(misc_memprof_stop)
 
 LUALIB_API int luaopen_misc(struct lua_State *L)
 {
+  luaM_sysprof_set_writer(buffer_writer_default);
+  luaM_sysprof_set_on_stop(on_stop_cb_default);
+  luaM_sysprof_set_backtracer(NULL);
+
   LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc);
   LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
   LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
-
   return 1;
 }
diff --git a/src/lj_mapi.c b/src/lj_mapi.c
index 73b5dc74..b327d2ca 100644
--- a/src/lj_mapi.c
+++ b/src/lj_mapi.c
@@ -67,10 +67,19 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics)
 }
 
 /* --- Platform and Lua profiler ------------------------------------------ */
+LUAMISC_API int luaM_sysprof_set_writer(sp_writer writer)
+{
+  return lj_sysprof_set_writer(writer);
+}
+
+LUAMISC_API int luaM_sysprof_set_on_stop(sp_on_stop on_stop)
+{
+  return lj_sysprof_set_on_stop(on_stop);
+}
 
-LUAMISC_API int luaM_sysprof_configure(const struct luam_Sysprof_Config *config)
+LUAMISC_API int luaM_sysprof_set_backtracer(sp_backtracer backtracer)
 {
-  return lj_sysprof_configure(config);
+  return lj_sysprof_set_backtracer(backtracer);
 }
 
 LUAMISC_API int luaM_sysprof_start(lua_State *L,
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 23947315..3088036f 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -54,8 +54,6 @@
 #define vmstfit4(st) ((st & ~(uint32_t)((1 << 4) - 1)) == 0)
 
 enum sysprof_state {
-  /* Profiler needs to be configured. */
-  SPS_UNCONFIGURED,
   /* Profiler is not running. */
   SPS_IDLE,
   /* Profiler is running. */
@@ -74,7 +72,9 @@ struct sysprof {
   struct lj_wbuf out; /* Output accumulator. */
   struct luam_Sysprof_Counters counters; /* Profiling counters. */
   struct luam_Sysprof_Options opt; /* Profiling options. */
-  struct luam_Sysprof_Config config; /* Profiler configurations. */
+  sp_writer writer; /* Writer function for profile events. */
+  sp_on_stop on_stop; /* Callback on profiling stopping. */
+  sp_backtracer backtracer; /* Backtracing function for the host stack. */
   lj_profile_timer timer; /* Profiling timer. */
   int saved_errno; /* Saved errno when profiler failed. */
   uint32_t lib_adds; /* Number of libs loaded. Monotonic. */
@@ -95,6 +95,11 @@ static int stream_is_needed(struct sysprof *sp)
   return sp->opt.mode != LUAM_SYSPROF_DEFAULT;
 }
 
+static int is_unconfigured(struct sysprof *sp)
+{
+  return sp->backtracer == NULL || sp->on_stop == NULL || sp->writer == NULL;
+}
+
 static void stream_prologue(struct sysprof *sp)
 {
   lj_symtab_dump(&sp->out, sp->g, &sp->lib_adds);
@@ -206,8 +211,8 @@ static void default_backtrace_host(void *(writer)(int frame_no, void *addr))
 
 static void stream_backtrace_host(struct sysprof *sp)
 {
-  lua_assert(sp->config.backtracer != NULL);
-  sp->config.backtracer(stream_frame_host);
+  lua_assert(sp->backtracer != NULL);
+  sp->backtracer(stream_frame_host);
   lj_wbuf_addu64(&sp->out, (uintptr_t)LJP_FRAME_HOST_LAST);
 }
 
@@ -325,15 +330,11 @@ static int sysprof_validate(struct sysprof *sp,
                             const struct luam_Sysprof_Options *opt)
 {
   switch (sp->state) {
-    case SPS_UNCONFIGURED:
-      return PROFILE_ERRUSE;
-
     case SPS_IDLE:
       if (opt->mode > LUAM_SYSPROF_CALLGRAPH) {
         return PROFILE_ERRUSE;
       } else if (opt->mode != LUAM_SYSPROF_DEFAULT &&
-                 (opt->buf == NULL || opt->len == 0 ||
-                  sp->config.writer == NULL || sp->config.on_stop == NULL)) {
+                 (opt->buf == NULL || opt->len == 0 || is_unconfigured(sp))) {
         return PROFILE_ERRUSE;
       } else if (opt->interval == 0) {
         return PROFILE_ERRUSE;
@@ -373,24 +374,46 @@ static int sysprof_init(struct sysprof *sp, lua_State *L,
   sp->saved_errno = 0;
 
   if (stream_is_needed(sp))
-    lj_wbuf_init(&sp->out, sp->config.writer, opt->ctx, opt->buf, opt->len);
+    lj_wbuf_init(&sp->out, sp->writer, opt->ctx, opt->buf, opt->len);
 
   return PROFILE_SUCCESS;
 }
 
 /* -- Public profiling API ------------------------------------------------ */
 
-int lj_sysprof_configure(const struct luam_Sysprof_Config *config)
-{
+int lj_sysprof_set_writer(sp_writer writer) {
+  struct sysprof *sp = &sysprof;
+
+  if (sp->state != SPS_IDLE || writer == NULL)
+    return PROFILE_ERRUSE;
+
+  sp->writer = writer;
+  if (!is_unconfigured(sp)) {
+    sp->state = SPS_IDLE;
+  }
+  return PROFILE_SUCCESS;
+}
+
+int lj_sysprof_set_on_stop(sp_on_stop on_stop) {
   struct sysprof *sp = &sysprof;
-  lua_assert(config != NULL);
-  if (sp->state != SPS_UNCONFIGURED && sp->state != SPS_IDLE)
+
+  if (sp->state != SPS_IDLE || on_stop == NULL)
     return PROFILE_ERRUSE;
 
-  memcpy(&sp->config, config, sizeof(*config));
+  sp->on_stop = on_stop;
+  if (!is_unconfigured(sp)) {
+    sp->state = SPS_IDLE;
+  }
+  return PROFILE_SUCCESS;
+}
+
+int lj_sysprof_set_backtracer(sp_backtracer backtracer) {
+  struct sysprof *sp = &sysprof;
 
-  if (sp->config.backtracer == NULL) {
-    sp->config.backtracer = default_backtrace_host;
+  if (sp->state != SPS_IDLE)
+    return PROFILE_ERRUSE;
+  if (backtracer == NULL) {
+    sp->backtracer = default_backtrace_host;
     /*
     ** XXX: `backtrace` is not signal-safe, according to man,
     ** because it is lazy loaded on the first call, which triggers
@@ -400,9 +423,12 @@ int lj_sysprof_configure(const struct luam_Sysprof_Config *config)
     void *dummy = NULL;
     backtrace(&dummy, 1);
   }
-
-  sp->state = SPS_IDLE;
-
+  else {
+    sp->backtracer = backtracer;
+  }
+  if (!is_unconfigured(sp)) {
+    sp->state = SPS_IDLE;
+  }
   return PROFILE_SUCCESS;
 }
 
@@ -412,12 +438,12 @@ int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt)
 
   int status = sysprof_init(sp, L, opt);
   if (PROFILE_SUCCESS != status) {
-    if (NULL != sp->config.on_stop) {
+    if (NULL != sp->on_stop) {
       /*
       ** Initialization may fail in case of unconfigured sysprof,
       ** so we cannot guarantee cleaning up resources in this case.
       */
-      sp->config.on_stop(opt->ctx, opt->buf);
+      sp->on_stop(opt->ctx, opt->buf);
     }
     return status;
   }
@@ -430,7 +456,7 @@ int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt)
       /* on_stop call may change errno value. */
       const int saved_errno = lj_wbuf_errno(&sp->out);
       /* Ignore possible errors. mp->out.buf may be NULL here. */
-      sp->config.on_stop(opt->ctx, sp->out.buf);
+      sp->on_stop(opt->ctx, sp->out.buf);
       lj_wbuf_terminate(&sp->out);
       sp->state = SPS_IDLE;
       errno = saved_errno;
@@ -473,7 +499,7 @@ int lj_sysprof_stop(lua_State *L)
     stream_epilogue(sp);
     lj_wbuf_flush(out);
 
-    cb_status = sp->config.on_stop(sp->opt.ctx, out->buf);
+    cb_status = sp->on_stop(sp->opt.ctx, out->buf);
     if (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_ERRIO | STREAM_STOP)) ||
         cb_status != 0) {
       errno = lj_wbuf_errno(out);
@@ -530,9 +556,18 @@ void lj_sysprof_add_trace(const struct GCtrace *tr)
 
 #else /* LJ_HASSYSPROF */
 
-int lj_sysprof_configure(const struct luam_Sysprof_Config *config)
-{
-  UNUSED(config);
+int lj_sysprof_set_writer(sp_writer writer) {
+  UNUSED(writer);
+  return PROFILE_ERRUSE;
+}
+
+int lj_sysprof_set_on_stop(sp_on_stop on_stop) {
+  UNUSED(on_stop);
+  return PROFILE_ERRUSE;
+}
+
+int lj_sysprof_set_backtracer(sp_backtracer backtracer) {
+  UNUSED(backtracer);
   return PROFILE_ERRUSE;
 }
 
diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h
index c31d61d8..d869013e 100644
--- a/src/lj_sysprof.h
+++ b/src/lj_sysprof.h
@@ -86,7 +86,11 @@
 #define LJP_SYMTAB_TRACE_EVENT ((uint8_t)12)
 #define LJP_EPILOGUE_BYTE 0x80
 
-int lj_sysprof_configure(const struct luam_Sysprof_Config *config);
+int lj_sysprof_set_writer(sp_writer writer);
+
+int lj_sysprof_set_on_stop(sp_on_stop on_stop);
+
+int lj_sysprof_set_backtracer(sp_backtracer backtracer);
 
 int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt);
 
diff --git a/src/lmisclib.h b/src/lmisclib.h
index b41f7f59..3ebf18ff 100644
--- a/src/lmisclib.h
+++ b/src/lmisclib.h
@@ -63,31 +63,29 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics);
 /* --- Sysprof - platform and lua profiler -------------------------------- */
 
 /* Profiler configurations. */
-struct luam_Sysprof_Config {
-  /*
-  ** Writer function for profile events. Must be async-safe, see also
-  ** `man 7 signal-safety`.
-  ** Should return amount of written bytes on success or zero in case of error.
-  ** Setting *data to NULL means end of profiling.
-  ** For details see <lj_wbuf.h>.
-  */
-  size_t (*writer)(const void **data, size_t len, void *ctx);
-  /*
-  ** Callback on profiler stopping. Required for correctly cleaning
-  ** at VM finalization when profiler is still running.
-  ** Returns zero on success.
-  */
-  int (*on_stop)(void *ctx, uint8_t *buf);
-  /*
-  ** Backtracing function for the host stack. Should call `frame_writer` on
-  ** each frame in the stack in the order from the stack top to the stack
-  ** bottom. The `frame_writer` function is implemented inside the sysprof
-  ** and will be passed to the `backtracer` function. If `frame_writer` returns
-  ** NULL, backtracing should be stopped. If `frame_writer` returns not NULL,
-  ** the backtracing should be continued if there are frames left.
-  */
-  void (*backtracer)(void *(*frame_writer)(int frame_no, void *addr));
-};
+/*
+** Writer function for profile events. Must be async-safe, see also
+** `man 7 signal-safety`.
+** Should return amount of written bytes on success or zero in case of error.
+** Setting *data to NULL means end of profiling.
+** For details see <lj_wbuf.h>.
+*/
+typedef size_t (*sp_writer)(const void **data, size_t len, void *ctx);
+/*
+** Callback on profiler stopping. Required for correctly cleaning
+** at VM finalization when profiler is still running.
+** Returns zero on success.
+*/
+typedef int (*sp_on_stop)(void *ctx, uint8_t *buf);
+/*
+** Backtracing function for the host stack. Should call `frame_writer` on
+** each frame in the stack in the order from the stack top to the stack
+** bottom. The `frame_writer` function is implemented inside the sysprof
+** and will be passed to the `backtracer` function. If `frame_writer` returns
+** NULL, backtracing should be stopped. If `frame_writer` returns not NULL,
+** the backtracing should be continued if there are frames left.
+*/
+typedef void (*sp_backtracer)(void *(*frame_writer)(int frame_no, void *addr));
 
 /*
 ** DEFAULT mode collects only data for luam_sysprof_counters, which is stored
@@ -144,7 +142,11 @@ struct luam_Sysprof_Options {
 #define PROFILE_ERRMEM  3
 #define PROFILE_ERRIO   4
 
-LUAMISC_API int luaM_sysprof_configure(const struct luam_Sysprof_Config *config);
+LUAMISC_API int luaM_sysprof_set_writer(sp_writer writer);
+
+LUAMISC_API int luaM_sysprof_set_on_stop(sp_on_stop on_stop);
+
+LUAMISC_API int luaM_sysprof_set_backtracer(sp_backtracer backtracer);
 
 LUAMISC_API int luaM_sysprof_start(lua_State *L,
                                    const struct luam_Sysprof_Options *opt);
diff --git a/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c b/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c
index 4b258766..984fc121 100644
--- a/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c
+++ b/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c
@@ -122,14 +122,9 @@ static int c_payload(lua_State *L)
 
 static int base(lua_State *L)
 {
-	struct luam_Sysprof_Config config = {};
 	struct luam_Sysprof_Options opt = {};
 	struct luam_Sysprof_Counters cnt = {};
 
-	(void)config.writer;
-	(void)config.on_stop;
-	(void)config.backtracer;
-
 	(void)opt.interval;
 	(void)opt.mode;
 	(void)opt.ctx;
@@ -156,13 +151,9 @@ static int base(lua_State *L)
 
 static int validation(lua_State *L)
 {
-	struct luam_Sysprof_Config config = {};
 	struct luam_Sysprof_Options opt = {};
 	int status = PROFILE_SUCCESS;
 
-	status = luaM_sysprof_configure(&config);
-	assert(PROFILE_SUCCESS == status);
-
 	/* Unknown mode. */
 	opt.mode = 0x40;
 	status = luaM_sysprof_start(L, &opt);
@@ -204,7 +195,6 @@ static int validation(lua_State *L)
 
 static int profile_func(lua_State *L)
 {
-	struct luam_Sysprof_Config config = {};
 	struct luam_Sysprof_Options opt = {};
 	struct luam_Sysprof_Counters cnt = {};
 	int status = PROFILE_ERRUSE;
@@ -222,10 +212,9 @@ static int profile_func(lua_State *L)
 	opt.interval = SYSPROF_INTERVAL_DEFAULT;
 	stream_init(&opt);
 
-	config.on_stop = on_stop_cb_default;
-	config.writer = buffer_writer_default;
-	status = luaM_sysprof_configure(&config);
-	assert(PROFILE_SUCCESS == status);
+  assert(luaM_sysprof_set_writer(buffer_writer_default) == PROFILE_SUCCESS);
+  assert(luaM_sysprof_set_on_stop(on_stop_cb_default) == PROFILE_SUCCESS);
+  assert(luaM_sysprof_set_backtracer(NULL) == PROFILE_SUCCESS);
 
 	status = luaM_sysprof_start(L, &opt);
 	assert(PROFILE_SUCCESS == status);
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index bb0e065d..2f53d771 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -70,7 +70,7 @@ res, err = misc.sysprof.start{ mode = "D" }
 assert(res, err)
 
 res, err, errno = misc.sysprof.start{ mode = "D" }
-test:ok(res == nil and err:match("profiler misuse"))
+test:ok(res == nil and err:match("profiler is running already"))
 test:ok(type(errno) == "number")
 
 res, err = misc.sysprof.stop()
-- 
2.35.1

Re: [Tarantool-patches] [PATCH] sysprof: change C configuration API

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the patch! See no critical flaws, but please consider my
comments below.

On 11.05.22, Maxim Kokryashkin wrote:
> This patch replaces the `lj_sysprof_configure` with individual
> configuration handles for each config entry. Also, it chnages the

Typo: s/chnages/changes/.

> internal sysprof checks to let users run sysprof in default mode without
> configuratons.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> PR with Tarantool integration:
> https://github.com/tarantool/tarantool/pull/7123
> 
>  src/lib_misc.c                                | 20 +---
>  src/lj_mapi.c                                 | 13 ++-
>  src/lj_sysprof.c                              | 91 +++++++++++++------
>  src/lj_sysprof.h                              |  6 +-
>  src/lmisclib.h                                | 54 +++++------
>  .../misclib-sysprof-capi/testsysprof.c        | 17 +---
>  .../misclib-sysprof-lapi.test.lua             |  2 +-
>  7 files changed, 116 insertions(+), 87 deletions(-)
> 
> diff --git a/src/lib_misc.c b/src/lib_misc.c
> index e07f1b32..370307a7 100644
> --- a/src/lib_misc.c
> +++ b/src/lib_misc.c

<snipped>

> @@ -454,9 +441,12 @@ LJLIB_CF(misc_memprof_stop)
>  
>  LUALIB_API int luaopen_misc(struct lua_State *L)
>  {
> +  luaM_sysprof_set_writer(buffer_writer_default);
> +  luaM_sysprof_set_on_stop(on_stop_cb_default);
> +  luaM_sysprof_set_backtracer(NULL);

Why default backtracer is set to NULL here?

Besides, why do you need to use public API if you can use the same
internal interfaces here? Otherwise, what is the reason to have these
lj_sysprof_* functions?

> +
>    LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc);
>    LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
>    LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
> -
>    return 1;
>  }

<snipped>

> diff --git a/src/lmisclib.h b/src/lmisclib.h
> index b41f7f59..3ebf18ff 100644
> --- a/src/lmisclib.h
> +++ b/src/lmisclib.h
> @@ -63,31 +63,29 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics);
>  /* --- Sysprof - platform and lua profiler -------------------------------- */
>  
>  /* Profiler configurations. */

<snipped>

> +/*
> +** Writer function for profile events. Must be async-safe, see also
> +** `man 7 signal-safety`.
> +** Should return amount of written bytes on success or zero in case of error.
> +** Setting *data to NULL means end of profiling.
> +** For details see <lj_wbuf.h>.
> +*/
> +typedef size_t (*sp_writer)(const void **data, size_t len, void *ctx);

Since this is a public type, please use a proper prefix for it.

> +/*
> +** Callback on profiler stopping. Required for correctly cleaning
> +** at VM finalization when profiler is still running.
> +** Returns zero on success.
> +*/
> +typedef int (*sp_on_stop)(void *ctx, uint8_t *buf);

Ditto.

> +/*
> +** Backtracing function for the host stack. Should call `frame_writer` on
> +** each frame in the stack in the order from the stack top to the stack
> +** bottom. The `frame_writer` function is implemented inside the sysprof
> +** and will be passed to the `backtracer` function. If `frame_writer` returns
> +** NULL, backtracing should be stopped. If `frame_writer` returns not NULL,
> +** the backtracing should be continued if there are frames left.
> +*/
> +typedef void (*sp_backtracer)(void *(*frame_writer)(int frame_no, void *addr));

Ditto.

>  
>  /*
>  ** DEFAULT mode collects only data for luam_sysprof_counters, which is stored

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] sysprof: change C configuration API

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!

Thanks for the patch!

LGTM, after fixes mentioned by Igor below.

On 18.05.22, Igor Munkin wrote:
> Max,
> 
> Thanks for the patch! See no critical flaws, but please consider my
> comments below.
> 
> On 11.05.22, Maxim Kokryashkin wrote:
> > This patch replaces the `lj_sysprof_configure` with individual
> > configuration handles for each config entry. Also, it chnages the
> 
> Typo: s/chnages/changes/.
> 
> > internal sysprof checks to let users run sysprof in default mode without
> > configuratons.
> > 
> > Part of tarantool/tarantool#781
> > ---
> > Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> > PR with Tarantool integration:
> > https://github.com/tarantool/tarantool/pull/7123
> > 
> >  src/lib_misc.c                                | 20 +---
> >  src/lj_mapi.c                                 | 13 ++-
> >  src/lj_sysprof.c                              | 91 +++++++++++++------
> >  src/lj_sysprof.h                              |  6 +-
> >  src/lmisclib.h                                | 54 +++++------
> >  .../misclib-sysprof-capi/testsysprof.c        | 17 +---
> >  .../misclib-sysprof-lapi.test.lua             |  2 +-
> >  7 files changed, 116 insertions(+), 87 deletions(-)
> > 
> > diff --git a/src/lib_misc.c b/src/lib_misc.c
> > index e07f1b32..370307a7 100644
> > --- a/src/lib_misc.c
> > +++ b/src/lib_misc.c
> 
> <snipped>
> 
> > @@ -454,9 +441,12 @@ LJLIB_CF(misc_memprof_stop)
> >  
> >  LUALIB_API int luaopen_misc(struct lua_State *L)
> >  {
> > +  luaM_sysprof_set_writer(buffer_writer_default);
> > +  luaM_sysprof_set_on_stop(on_stop_cb_default);
> > +  luaM_sysprof_set_backtracer(NULL);
> 
> Why default backtracer is set to NULL here?
> 
> Besides, why do you need to use public API if you can use the same
> internal interfaces here? Otherwise, what is the reason to have these
> lj_sysprof_* functions?
> 
> > +
> >    LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc);
> >    LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
> >    LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
> > -
> >    return 1;
> >  }
> 
> <snipped>
> 
> > diff --git a/src/lmisclib.h b/src/lmisclib.h
> > index b41f7f59..3ebf18ff 100644
> > --- a/src/lmisclib.h
> > +++ b/src/lmisclib.h
> > @@ -63,31 +63,29 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics);
> >  /* --- Sysprof - platform and lua profiler -------------------------------- */
> >  
> >  /* Profiler configurations. */
> 
> <snipped>
> 
> > +/*
> > +** Writer function for profile events. Must be async-safe, see also
> > +** `man 7 signal-safety`.
> > +** Should return amount of written bytes on success or zero in case of error.
> > +** Setting *data to NULL means end of profiling.
> > +** For details see <lj_wbuf.h>.
> > +*/
> > +typedef size_t (*sp_writer)(const void **data, size_t len, void *ctx);
> 
> Since this is a public type, please use a proper prefix for it.
> 
> > +/*
> > +** Callback on profiler stopping. Required for correctly cleaning
> > +** at VM finalization when profiler is still running.
> > +** Returns zero on success.
> > +*/
> > +typedef int (*sp_on_stop)(void *ctx, uint8_t *buf);
> 
> Ditto.
> 
> > +/*
> > +** Backtracing function for the host stack. Should call `frame_writer` on
> > +** each frame in the stack in the order from the stack top to the stack
> > +** bottom. The `frame_writer` function is implemented inside the sysprof
> > +** and will be passed to the `backtracer` function. If `frame_writer` returns
> > +** NULL, backtracing should be stopped. If `frame_writer` returns not NULL,
> > +** the backtracing should be continued if there are frames left.
> > +*/
> > +typedef void (*sp_backtracer)(void *(*frame_writer)(int frame_no, void *addr));
> 
> Ditto.
> 
> >  
> >  /*
> >  ** DEFAULT mode collects only data for luam_sysprof_counters, which is stored
> 
> <snipped>
> 
> > -- 
> > 2.35.1
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH] sysprof: enrich symtab on a new trace or a proto

[Maxim Kokryashkin via Tarantool-patches]

This commit adds functionality introduced
in 0847e71abf7db2559e2bfa35f147ccf0112035e3 ('memprof: enrich symtab when
meeting new prototype') and 0243fb72b05c7c63481c50151c65bef6b04e3372 ('memprof:
enrich symtab when new trace is compiled') to sysprof.

Both the proto and the trace symtab extensions cannot be run from the
sysprof's signal handler, so it is required to prevent sysprof from
dumping anything in its signal handler during the process to keep the
event stream intact. That is achieved with setting the sysprof's internal state
to `SPS_IDLE`.

Part of tarantool/tarantool#781
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci

 src/lj_bcread.c         |  7 +++++++
 src/lj_parse.c          |  6 ++++++
 src/lj_sysprof.c        | 44 ++++++++++++++++++++++++++++++++++++++++-
 src/lj_sysprof.h        |  9 ++++++++-
 src/lj_trace.c          |  7 +++++++
 tools/sysprof/parse.lua | 18 ++++++++++++-----
 6 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/src/lj_bcread.c b/src/lj_bcread.c
index cb08599d..f6c7ad25 100644
--- a/src/lj_bcread.c
+++ b/src/lj_bcread.c
@@ -25,6 +25,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* Reuse some lexer fields for our own purposes. */
 #define bcread_flags(ls)	ls->level
@@ -390,6 +393,10 @@ GCproto *lj_bcread_proto(LexState *ls)
   lj_memprof_add_proto(pt);
 #endif
 
+#if LJ_HASSYSPROF
+  lj_sysprof_add_proto(pt);
+#endif
+
   return pt;
 }
 
diff --git a/src/lj_parse.c b/src/lj_parse.c
index 30b0caa0..af0dc53f 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -30,6 +30,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* -- Parser structures and definitions ----------------------------------- */
 
@@ -1598,6 +1601,9 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
 #if LJ_HASMEMPROF
   lj_memprof_add_proto(pt);
 #endif
+#if LJ_HASSYSPROF
+  lj_sysprof_add_proto(pt);
+#endif
 
   L->top--;  /* Pop table of constants. */
   ls->vtop = fs->vbase;  /* Reset variable stack. */
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 28d7d229..23947315 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -238,7 +238,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate)
 static void stream_host(struct sysprof *sp, uint32_t vmstate)
 {
   struct lua_State *L = gco2th(gcref(sp->g->cur_L));
-  lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_EVENT, L);
+  lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L);
   lj_wbuf_addbyte(&sp->out, (uint8_t)vmstate);
   stream_backtrace_host(sp);
 }
@@ -496,6 +496,38 @@ int lj_sysprof_report(struct luam_Sysprof_Counters *counters)
   return PROFILE_SUCCESS;
 }
 
+void lj_sysprof_add_proto(const struct GCproto *pt)
+{
+  struct sysprof *sp = &sysprof;
+
+  if (sp->state != SPS_PROFILE)
+    return;
+
+  /*
+  ** XXX: Avoid sampling during the symtab extension. That shouldn't have any
+  ** significant effect on profile precision, but if it does, it's better to
+  ** implement an async-safe queue for the symtab events.
+  */
+  sp->state = SPS_IDLE;
+  lj_wbuf_addbyte(&sp->out, LJP_SYMTAB_LFUNC_EVENT);
+  lj_symtab_dump_proto(&sp->out, pt);
+  sp->state = SPS_PROFILE;
+}
+
+void lj_sysprof_add_trace(const struct GCtrace *tr)
+{
+  struct sysprof *sp = &sysprof;
+
+  if (sp->state != SPS_PROFILE)
+    return;
+
+  /* See the comment about the sysprof state above. */
+  sp->state = SPS_IDLE;
+  lj_wbuf_addbyte(&sp->out, LJP_SYMTAB_TRACE_EVENT);
+  lj_symtab_dump_trace(&sp->out, tr);
+  sp->state = SPS_PROFILE;
+}
+
 #else /* LJ_HASSYSPROF */
 
 int lj_sysprof_configure(const struct luam_Sysprof_Config *config)
@@ -522,4 +554,14 @@ int lj_sysprof_report(struct luam_Sysprof_Counters *counters)
   return PROFILE_ERRUSE;
 }
 
+void lj_sysprof_add_proto(const struct GCproto *pt)
+{
+  UNUSED(pt);
+}
+
+void lj_sysprof_add_trace(const struct GCtrace *tr)
+{
+  UNUSED(tr);
+}
+
 #endif /* LJ_HASSYSPROF */
diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h
index 2978bbd8..c31d61d8 100644
--- a/src/lj_sysprof.h
+++ b/src/lj_sysprof.h
@@ -15,6 +15,7 @@
 #ifndef _LJ_SYSPROF_H
 #define _LJ_SYSPROF_H
 
+#include "lj_jit.h"
 #include "lj_obj.h"
 #include "lmisclib.h"
 
@@ -80,7 +81,9 @@
 #define LJP_FRAME_LUA_LAST  0x80
 #define LJP_FRAME_HOST_LAST NULL
 
-#define LJP_SYMTAB_EVENT ((uint8_t)10)
+#define LJP_SYMTAB_LFUNC_EVENT ((uint8_t)10)
+#define LJP_SYMTAB_CFUNC_EVENT ((uint8_t)11)
+#define LJP_SYMTAB_TRACE_EVENT ((uint8_t)12)
 #define LJP_EPILOGUE_BYTE 0x80
 
 int lj_sysprof_configure(const struct luam_Sysprof_Config *config);
@@ -91,4 +94,8 @@ int lj_sysprof_stop(lua_State *L);
 
 int lj_sysprof_report(struct luam_Sysprof_Counters *counters);
 
+void lj_sysprof_add_proto(const struct GCproto *pt);
+
+void lj_sysprof_add_trace(const struct GCtrace *tr);
+
 #endif
diff --git a/src/lj_trace.c b/src/lj_trace.c
index 84b957c6..d7a78d4d 100644
--- a/src/lj_trace.c
+++ b/src/lj_trace.c
@@ -33,6 +33,9 @@
 #if LJ_HASMEMPROF
 #include "lj_memprof.h"
 #endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
 
 /* -- Error handling ------------------------------------------------------ */
 
@@ -171,6 +174,10 @@ static void trace_save(jit_State *J, GCtrace *T)
 #if LJ_HASMEMPROF
   lj_memprof_add_trace(T);
 #endif
+
+#if LJ_HASSYSPROF
+  lj_sysprof_add_trace(T);
+#endif
 }
 
 void LJ_FASTCALL lj_trace_free(global_State *g, GCtrace *T)
diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
index cb271784..555b6b3b 100755
--- a/tools/sysprof/parse.lua
+++ b/tools/sysprof/parse.lua
@@ -33,7 +33,9 @@ M.FRAME = {
 }
 
 local STREAM_END = 0x80
-local SYMTAB_EVENT = 10
+local SYMTAB_LFUNC_EVENT = 10
+local SYMTAB_CFUNC_EVENT = 11
+local SYMTAB_TRACE_EVENT = 12
 
 local function new_event()
   return {
@@ -128,8 +130,14 @@ local function parse_trace(reader, event, symbols)
   -- parse_lua_callchain(reader, event)
 end
 
-local function parse_symtab(reader, symbols)
-  symtab.parse_sym_cfunc(reader, symbols)
+local function parse_symtab(reader, symbols, vmstate)
+  if vmstate == SYMTAB_LFUNC_EVENT then
+    symtab.parse_sym_lfunc(reader, symbols)
+  elseif vmstate == SYMTAB_CFUNC_EVENT then
+    symtab.parse_sym_cfunc(reader, symbols)
+  elseif vmstate == SYMTAB_TRACE_EVENT then
+    symtab.parse_sym_trace(reader, symbols)
+  end
 end
 
 local event_parsers = {
@@ -152,8 +160,8 @@ local function parse_event(reader, events, symbols)
   if vmstate == STREAM_END then
     -- TODO: samples & overruns
     return false
-  elseif vmstate == SYMTAB_EVENT then
-    parse_symtab(reader, symbols)
+  elseif SYMTAB_LFUNC_EVENT <= vmstate and vmstate <= SYMTAB_TRACE_EVENT then
+    parse_symtab(reader, symbols, vmstate)
     return true
   end
 
-- 
2.35.1

Re: [Tarantool-patches] [PATCH] sysprof: enrich symtab on a new trace or a proto

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!

Thanks for the patch!
LGTM, except a few nits.

On 11.05.22, Maxim Kokryashkin wrote:
> This commit adds functionality introduced
> in 0847e71abf7db2559e2bfa35f147ccf0112035e3 ('memprof: enrich symtab when
> meeting new prototype') and 0243fb72b05c7c63481c50151c65bef6b04e3372 ('memprof:
> enrich symtab when new trace is compiled') to sysprof.
> 
> Both the proto and the trace symtab extensions cannot be run from the
> sysprof's signal handler, so it is required to prevent sysprof from
> dumping anything in its signal handler during the process to keep the
> event stream intact. That is achieved with setting the sysprof's internal state

Typo? s/achieved with/achieved by/

> to `SPS_IDLE`.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/lj_bcread.c         |  7 +++++++
>  src/lj_parse.c          |  6 ++++++
>  src/lj_sysprof.c        | 44 ++++++++++++++++++++++++++++++++++++++++-
>  src/lj_sysprof.h        |  9 ++++++++-
>  src/lj_trace.c          |  7 +++++++
>  tools/sysprof/parse.lua | 18 ++++++++++++-----

Nit: please update Makefile.dep.original as well.

>  6 files changed, 84 insertions(+), 7 deletions(-)
> 
> diff --git a/src/lj_bcread.c b/src/lj_bcread.c
> index cb08599d..f6c7ad25 100644
> --- a/src/lj_bcread.c
> +++ b/src/lj_bcread.c

<snipped>

> diff --git a/src/lj_parse.c b/src/lj_parse.c
> index 30b0caa0..af0dc53f 100644
> --- a/src/lj_parse.c
> +++ b/src/lj_parse.c

<snipped>

> diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
> index 28d7d229..23947315 100644
> --- a/src/lj_sysprof.c
> +++ b/src/lj_sysprof.c
> @@ -238,7 +238,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate)

<snipped>

> diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h
> index 2978bbd8..c31d61d8 100644
> --- a/src/lj_sysprof.h
> +++ b/src/lj_sysprof.h
> @@ -15,6 +15,7 @@
>  #ifndef _LJ_SYSPROF_H
>  #define _LJ_SYSPROF_H
>  

Nit: We need to check that LuaJIT is build with JIT here.
| #if LJ_HASJIT

> +#include "lj_jit.h"
>  #include "lj_obj.h"
>  #include "lmisclib.h"
>  
> @@ -80,7 +81,9 @@
>  #define LJP_FRAME_LUA_LAST  0x80
>  #define LJP_FRAME_HOST_LAST NULL
>  
> -#define LJP_SYMTAB_EVENT ((uint8_t)10)
> +#define LJP_SYMTAB_LFUNC_EVENT ((uint8_t)10)
> +#define LJP_SYMTAB_CFUNC_EVENT ((uint8_t)11)
> +#define LJP_SYMTAB_TRACE_EVENT ((uint8_t)12)

This means we should increase LJP_FORMAT_VERSION as well.

>  #define LJP_EPILOGUE_BYTE 0x80
>  
>  int lj_sysprof_configure(const struct luam_Sysprof_Config *config);
> @@ -91,4 +94,8 @@ int lj_sysprof_stop(lua_State *L);
>  
>  int lj_sysprof_report(struct luam_Sysprof_Counters *counters);
>  
> +void lj_sysprof_add_proto(const struct GCproto *pt);
> +
> +void lj_sysprof_add_trace(const struct GCtrace *tr);

Nit: We need to check that LuaJIT is build with JIT here.
| #if LJ_HASJIT

> +
>  #endif
> diff --git a/src/lj_trace.c b/src/lj_trace.c
> index 84b957c6..d7a78d4d 100644
> --- a/src/lj_trace.c
> +++ b/src/lj_trace.c

<snipped>

> diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> index cb271784..555b6b3b 100755
> --- a/tools/sysprof/parse.lua
> +++ b/tools/sysprof/parse.lua
> @@ -33,7 +33,9 @@ M.FRAME = {

<snipped>

> @@ -128,8 +130,14 @@ local function parse_trace(reader, event, symbols)
>    -- parse_lua_callchain(reader, event)
>  end
>  
> -local function parse_symtab(reader, symbols)
> -  symtab.parse_sym_cfunc(reader, symbols)
> +local function parse_symtab(reader, symbols, vmstate)
> +  if vmstate == SYMTAB_LFUNC_EVENT then
> +    symtab.parse_sym_lfunc(reader, symbols)
> +  elseif vmstate == SYMTAB_CFUNC_EVENT then
> +    symtab.parse_sym_cfunc(reader, symbols)
> +  elseif vmstate == SYMTAB_TRACE_EVENT then
> +    symtab.parse_sym_trace(reader, symbols)
> +  end

Nit: may be we should add
| else
|   error('Unknown symtab event')
| end
check here.
Feel free to ignore.

>  end
>  
>  local event_parsers = {
> @@ -152,8 +160,8 @@ local function parse_event(reader, events, symbols)
>    if vmstate == STREAM_END then
>      -- TODO: samples & overruns
>      return false
> -  elseif vmstate == SYMTAB_EVENT then
> -    parse_symtab(reader, symbols)
> +  elseif SYMTAB_LFUNC_EVENT <= vmstate and vmstate <= SYMTAB_TRACE_EVENT then
> +    parse_symtab(reader, symbols, vmstate)
>      return true
>    end
>  
> -- 
> 2.35.1
> 

-- 
Best regards,
Sergey Kaplun

[Tarantool-patches] [PATCH] sysprof: fix `SYSPROF_HANDLER_STACK_DEPTH`

[Maxim Kokryashkin via Tarantool-patches]

This commit fixes the `SYSPROF_HANDLER_STACK_DEPTH`, by chnaging it to 6.
The `writer` function is called right after `backtrace`, so it will not
appear in the gathered backtrace.

Part of tarantool/tarantool#781
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci

 src/lj_sysprof.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 3088036f..55cea203 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -32,22 +32,20 @@
 ** Number of profiler frames we need to omit during stack
 ** unwinding.
 **   +------------------------+
-** 0 |    stream_frame_host   |
+** 0 | default_backtrace_host |
 **   +------------------------+
-** 1 | default_backtrace_host |
+** 1 | stream_backtrace_host  |
 **   +------------------------+
-** 2 | stream_backtrace_host  |
+** 2 |  stream_{guest/host}   |
 **   +------------------------+
-** 3 |  stream_{guest/host}   |
+** 3 |      stream_event      |
 **   +------------------------+
-** 4 |      stream_event      |
+** 4 | sysprof_record_sample  |
 **   +------------------------+
-** 5 | sysprof_record_sample  |
-**   +------------------------+
-** 6 | sysprof_signal_handler |
+** 5 | sysprof_signal_handler |
 **   +------------------------+
 */
-#define SYSPROF_HANDLER_STACK_DEPTH 7
+#define SYSPROF_HANDLER_STACK_DEPTH 6
 #define SYSPROF_BACKTRACE_FRAME_MAX 512
 
 /* Check that vmstate fits in 4 bits (see streaming format) */
-- 
2.35.1

Re: [Tarantool-patches] [PATCH] sysprof: fix `SYSPROF_HANDLER_STACK_DEPTH`

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the patch! LGTM as trivial.

On 11.05.22, Maxim Kokryashkin wrote:
> This commit fixes the `SYSPROF_HANDLER_STACK_DEPTH`, by chnaging it to 6.
> The `writer` function is called right after `backtrace`, so it will not
> appear in the gathered backtrace.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/lj_sysprof.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] sysprof: fix `SYSPROF_HANDLER_STACK_DEPTH`

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!

Thanks for the patch!
LGTM.

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH] sysprof: fix `SYSPROF_HANDLER_STACK_DEPTH`

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into tarantool branch in tarantool/luajit and
bumped a new version in master.

On 11.05.22, Maxim Kokryashkin wrote:
> This commit fixes the `SYSPROF_HANDLER_STACK_DEPTH`, by chnaging it to 6.
> The `writer` function is called right after `backtrace`, so it will not
> appear in the gathered backtrace.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/lj_sysprof.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM

[Tarantool-patches] [PATCH] sysprof: make sysprof internal API funcs static

[Maxim Kokryashkin via Tarantool-patches]

Make sysprof internal Lua API functions static to avoid their exposure.

Part of tarantool/tarantool#781
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci

 src/lib_misc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lib_misc.c b/src/lib_misc.c
index 370307a7..01dc1bea 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -162,7 +162,7 @@ static int on_stop_cb_default(void *opt, uint8_t *buf)
 #define SYSPROF_DEFAULT_INTERVAL 10
 #define SYSPROF_DEFAULT_OUTPUT "sysprof.bin"
 
-int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
+static int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
   struct profile_ctx *ctx = opt->ctx;
   int fd = 0;
   lua_assert(path != NULL);
@@ -174,7 +174,7 @@ int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
   return PROFILE_SUCCESS;
 }
 
-int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, int idx) {
+static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, int idx) {
   GCtab *options = lj_lib_checktab(L, idx);
 
   /* Get profiling mode. */
@@ -248,7 +248,7 @@ int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, int idx)
   return PROFILE_SUCCESS;
 }
 
-int parse_options(lua_State *L, struct luam_Sysprof_Options *opt)
+static int parse_options(lua_State *L, struct luam_Sysprof_Options *opt)
 {
   if (lua_gettop(L) != 1)
     return PROFILE_ERRUSE;
@@ -259,7 +259,7 @@ int parse_options(lua_State *L, struct luam_Sysprof_Options *opt)
   return parse_sysprof_opts(L, opt, 1);
 }
 
-int sysprof_error(lua_State *L, int status)
+static int sysprof_error(lua_State *L, int status)
 {
   switch (status) {
     case PROFILE_ERRUSE:
-- 
2.35.1

Re: [Tarantool-patches] [PATCH] sysprof: make sysprof internal API funcs static

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!

Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH] sysprof: make sysprof internal API funcs static

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the patch! LGTM as trivial.

On 11.05.22, Maxim Kokryashkin wrote:
> Make sysprof internal Lua API functions static to avoid their exposure.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/lib_misc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] sysprof: make sysprof internal API funcs static

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into tarantool branch in tarantool/luajit and
bumped a new version in master.

On 11.05.22, Maxim Kokryashkin wrote:
> Make sysprof internal Lua API functions static to avoid their exposure.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/lib_misc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] sysprof: add `LUAJIT_DISABLE_SYSPROF` to Makefile

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM!

Side note: You can use the following command to generate correct thread
format patchset:
| $ git format-patch --cover-letter --thread=shallow --subject-prefix="PATCH luajit" HEAD~5

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH] sysprof: add `LUAJIT_DISABLE_SYSPROF` to Makefile

[Igor Munkin via Tarantool-patches]

Max,

Thanks for the patch! LGTM as trivial.

On 11.05.22, Maxim Kokryashkin wrote:
> The Makefile.original was not properly updated for sysporf. This
> patch fixes this issue by adding the `LUAJIT_DISABLE_SYSPROF`
> option to it.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/Makefile.original | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/Makefile.original b/src/Makefile.original
> index 50b84bfe..dd1c6a76 100644
> --- a/src/Makefile.original
> +++ b/src/Makefile.original
> @@ -116,6 +116,8 @@ XCFLAGS=
>  # Disable the memory profiler.
>  #XCFLAGS+= -DLUAJIT_DISABLE_MEMPROF
>  #
> +# Disable the system profiler.
> +#XCFLAGS+= -DLUAJIT_DISABLE_SYSPROF
>  ##############################################################################
>  
>  ##############################################################################
> -- 
> 2.35.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] sysprof: add `LUAJIT_DISABLE_SYSPROF` to Makefile

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into tarantool branch in tarantool/luajit and
bumped a new version in master.

On 11.05.22, Maxim Kokryashkin wrote:
> The Makefile.original was not properly updated for sysporf. This
> patch fixes this issue by adding the `LUAJIT_DISABLE_SYSPROF`
> option to it.
> 
> Part of tarantool/tarantool#781
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci
> 
>  src/Makefile.original | 2 ++
>  1 file changed, 2 insertions(+)
> 

<snipped>

> -- 
> 2.35.1
> 

-- 
Best regards,
IM