[Tarantool-patches] [PATCH luajit 4/7][v2] sysprof: introduce specific errors and default mode
Sergey Bronnikov
estetus at gmail.com
Thu Feb 13 14:10:45 MSK 2025
sysprof has a number of options and with any incorrect option it
returns `false` and error message "profiler misuse". This message
discourage sysprof users and make using sysprof more complicated.
The patch sets default profiling mode ("D", that shows only
virtual machine state counters) if it was not passed and adds
details to the error message with possible reasons of misuse.
---
src/lib_misc.c | 80 +++++++++++++------
src/lj_errmsg.h | 5 ++
.../profilers/misclib-sysprof-lapi.test.lua | 48 +++++++++--
3 files changed, 100 insertions(+), 33 deletions(-)
diff --git a/src/lib_misc.c b/src/lib_misc.c
index 5b7a4b62..d71904e4 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -163,6 +163,7 @@ static int on_stop_cb_default(void *opt, uint8_t *buf)
/* The default profiling interval equals to 10 ms. */
#define SYSPROF_DEFAULT_INTERVAL 10
+#define SYSPROF_DEFAULT_MODE "D"
#define SYSPROF_DEFAULT_OUTPUT "sysprof.bin"
static int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
@@ -177,21 +178,41 @@ static int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
return PROFILE_SUCCESS;
}
-static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, int idx) {
- GCtab *options = lj_lib_checktab(L, idx);
+static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt,
+ const char **err_details) {
+ int n = (int)(L->top - L->base);
+ if (n != 1) {
+ opt->mode = LUAM_SYSPROF_DEFAULT;
+ opt->interval = SYSPROF_DEFAULT_INTERVAL;
+ goto set_path;
+ }
+
+ if (!lua_istable(L, 1)) {
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADTABLE);
+ return PROFILE_ERRUSE;
+ }
+
+ GCtab *options = lj_lib_checktab(L, 1);
/* Get profiling mode. */
{
const char *mode = NULL;
cTValue *mode_opt = lj_tab_getstr(options, lj_str_newlit(L, "mode"));
- if (!mode_opt || !tvisstr(mode_opt)) {
- return PROFILE_ERRUSE;
+ if (mode_opt) {
+ if (!tvisstr(mode_opt)) {
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADMODE);
+ return PROFILE_ERRUSE;
+ }
+ mode = strVdata(mode_opt);
+ if (strlen(mode) > 0 && mode[1] != '\0') {
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADMODE);
+ return PROFILE_ERRUSE;
+ }
}
- mode = strVdata(mode_opt);
- if (mode[1] != '\0')
- return PROFILE_ERRUSE;
+ if (!mode)
+ mode = SYSPROF_DEFAULT_MODE;
switch (*mode) {
case 'D':
@@ -204,6 +225,7 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
opt->mode = LUAM_SYSPROF_CALLGRAPH;
break;
default:
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADMODE);
return PROFILE_ERRUSE;
}
}
@@ -214,12 +236,16 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
opt->interval = SYSPROF_DEFAULT_INTERVAL;
if (interval && tvisnumber(interval)) {
int32_t signed_interval = numberVint(interval);
- if (signed_interval < 1)
+ if (signed_interval < 1) {
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADINTERVAL);
return PROFILE_ERRUSE;
+ }
opt->interval = signed_interval;
}
}
+set_path:
+
/* Get output path. */
if (opt->mode != LUAM_SYSPROF_DEFAULT)
{
@@ -230,8 +256,10 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
cTValue *pathtv = lj_tab_getstr(options, lj_str_newlit(L, "path"));
if (!pathtv)
path = SYSPROF_DEFAULT_OUTPUT;
- else if (!tvisstr(pathtv))
+ else if (!tvisstr(pathtv)) {
+ *err_details = err2msg(LJ_ERR_PROF_DETAILS_BADPATH);
return PROFILE_ERRUSE;
+ }
else
path = strVdata(pathtv);
@@ -251,29 +279,28 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
return PROFILE_SUCCESS;
}
-static int parse_options(lua_State *L, struct luam_Sysprof_Options *opt)
-{
- if (lua_gettop(L) != 1)
- return PROFILE_ERRUSE;
-
- if (!lua_istable(L, 1))
- return PROFILE_ERRUSE;
-
- return parse_sysprof_opts(L, opt, 1);
-}
-
-static int sysprof_error(lua_State *L, int status)
+static int sysprof_error(lua_State *L, int status, const char *err_details)
{
switch (status) {
case PROFILE_ERRUSE:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
+ if (err_details) {
+ lua_pushstring(L, ": ");
+ lua_pushstring(L, err_details);
+ lua_concat(L, 3);
+ }
lua_pushinteger(L, EINVAL);
return 3;
#if LJ_HASSYSPROF
case PROFILE_ERRRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
+ if (err_details) {
+ lua_pushstring(L, ": ");
+ lua_pushstring(L, err_details);
+ lua_concat(L, 3);
+ }
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRIO:
@@ -291,15 +318,16 @@ LJLIB_CF(misc_sysprof_start)
int status = PROFILE_SUCCESS;
struct luam_Sysprof_Options opt = {};
+ const char *err_details = NULL;
- status = parse_options(L, &opt);
+ status = parse_sysprof_opts(L, &opt, &err_details);
if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
- return sysprof_error(L, status);
+ return sysprof_error(L, status, err_details);
status = luaM_sysprof_start(L, &opt);
if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
/* Allocated memory will be freed in on_stop callback. */
- return sysprof_error(L, status);
+ return sysprof_error(L, status, err_details);
lua_pushboolean(L, 1);
return 1;
@@ -310,7 +338,7 @@ LJLIB_CF(misc_sysprof_stop)
{
int status = luaM_sysprof_stop(L);
if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
- return sysprof_error(L, status);
+ return sysprof_error(L, status, NULL);
lua_pushboolean(L, 1);
return 1;
@@ -325,7 +353,7 @@ LJLIB_CF(misc_sysprof_report)
int status = luaM_sysprof_report(&counters);
if (status != PROFILE_SUCCESS)
- return sysprof_error(L, status);
+ return sysprof_error(L, status, NULL);
lua_createtable(L, 0, 3);
data_tab = tabV(L->top - 1);
diff --git a/src/lj_errmsg.h b/src/lj_errmsg.h
index 19c41f0b..b5c3a275 100644
--- a/src/lj_errmsg.h
+++ b/src/lj_errmsg.h
@@ -188,6 +188,11 @@ ERRDEF(PROF_ISRUNNING, "profiler is running already")
ERRDEF(PROF_NOTRUNNING, "profiler is not running")
#endif
+ERRDEF(PROF_DETAILS_BADMODE, "profiler mode must be 'D', 'L' or 'C'")
+ERRDEF(PROF_DETAILS_BADINTERVAL, "profiler interval must be greater than 1")
+ERRDEF(PROF_DETAILS_BADPATH, "profiler path does not exist")
+ERRDEF(PROF_DETAILS_BADTABLE, "profiler expects a table with parameters")
+
#undef ERRDEF
/* Detecting unused error messages:
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 32fa384c..7622323a 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -10,7 +10,7 @@ local test = tap.test("misclib-sysprof-lapi"):skipcond({
["Disabled due to #10803"] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
})
-test:plan(19)
+test:plan(33)
jit.off()
-- XXX: Run JIT tuning functions in a safe frame to avoid errors
@@ -65,10 +65,25 @@ end
-- Wrong profiling mode.
local res, err, errno = misc.sysprof.start{ mode = "A" }
-test:ok(res == nil and err:match("profiler misuse"),
- "result status with wrong profiling mode")
+test:ok(res == nil, "result status with wrong profiling mode")
+test:ok(err:match("profiler mode must be 'D', 'L' or 'C'"),
+ "error with wrong profiling mode")
test:ok(type(errno) == "number", "errno with wrong profiling mode")
+-- Missed profiling mode.
+res, err, errno = misc.sysprof.start{}
+test:is(res, true, "res with missed profiling mode")
+test:is(err, nil, "no error with missed profiling mode")
+test:is(errno, nil, "no errno with missed profiling mode")
+assert(misc.sysprof.stop())
+
+-- Not a table.
+res, err, errno = misc.sysprof.start("NOT A TABLE")
+test:ok(res == nil, "res with not a table")
+test:ok(err:match("profiler expects a table with parameters"),
+ "error with not a table")
+test:ok(type(errno) == "number", "errno with not a table")
+
-- Already running.
res, err = misc.sysprof.start{ mode = "D" }
assert(res, err)
@@ -92,11 +107,30 @@ test:ok(res == nil and err:match("No such file or directory"),
"result status and error with bad path")
test:ok(type(errno) == "number", "errno with bad path")
--- Bad interval.
+-- Bad interval (-1).
res, err, errno = misc.sysprof.start{ mode = "C", interval = -1 }
-test:ok(res == nil and err:match("profiler misuse"),
- "result status and error with bad interval")
-test:ok(type(errno) == "number", "errno with bad interval")
+test:is(res, nil, "result status and error with bad interval -1")
+test:ok(err:match("profiler interval must be greater than 1"),
+ "error with bad interval -1")
+test:ok(type(errno) == "number", "errno with bad interval -1")
+
+-- Bad interval (0).
+res, err, errno = misc.sysprof.start{ mode = "C", interval = 0 }
+test:ok(res == nil, "res with bad interval 0")
+test:ok(err:match("profiler interval must be greater than 1"),
+ "error with bad interval 0")
+test:ok(type(errno) == "number", "errno with bad interval 0")
+
+-- Good interval (1).
+res, err, errno = misc.sysprof.start{
+ mode = "C",
+ interval = 1,
+ path = "/dev/null",
+}
+test:is(res, true, "res with good interval 1")
+test:is(err, nil, "no error with good interval 1")
+test:is(errno, nil, "no errno with good interval 1")
+misc.sysprof.stop()
-- DEFAULT MODE
--
2.34.1
More information about the Tarantool-patches
mailing list