[Tarantool-patches] [PATCH] sysprof: change C configuration API
Maxim Kokryashkin
max.kokryashkin at gmail.com
Wed May 11 11:25:18 MSK 2022
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
More information about the Tarantool-patches
mailing list