Skip to content

Commit

Permalink
sysprof: introduce specific errors and default mode
Browse files Browse the repository at this point in the history
sysprof has a number of options and with any incorrect option it
returns `false` and error message "profiler misuse". This message
does not descriptive for sysprof users and make using sysprof
more complicated. The patch splits error `PROFILE_ERRUSE` to four
specific errors: `PROFILE_ERRBADMODE`, `PROFILE_ERRBADINTERVAL`,
`PROFILE_ERRBADPATH` and `PROFILE_ERRBADTABLE`, and use default
profiling mode ("C", callgraph) if it was not passed.
  • Loading branch information
ligurio committed Feb 4, 2025
1 parent 6ff0d57 commit 5dd3f91
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 16 deletions.
47 changes: 36 additions & 11 deletions src/lib_misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 "C"
#define SYSPROF_DEFAULT_OUTPUT "sysprof.bin"

static int set_output_path(const char *path, struct luam_Sysprof_Options *opt) {
Expand All @@ -185,13 +186,16 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
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))
return PROFILE_ERRBADMODE;
mode = strVdata(mode_opt);
if (mode[1] != '\0')
return PROFILE_ERRBADMODE;
}

mode = strVdata(mode_opt);
if (mode[1] != '\0')
return PROFILE_ERRUSE;
if (!mode)
mode = SYSPROF_DEFAULT_MODE;

switch (*mode) {
case 'D':
Expand All @@ -204,7 +208,7 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
opt->mode = LUAM_SYSPROF_CALLGRAPH;
break;
default:
return PROFILE_ERRUSE;
return PROFILE_ERRBADMODE;
}
}

Expand All @@ -215,7 +219,7 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
if (interval && tvisnumber(interval)) {
int32_t signed_interval = numberVint(interval);
if (signed_interval < 1)
return PROFILE_ERRUSE;
return PROFILE_ERRBADINTERVAL;
opt->interval = signed_interval;
}
}
Expand All @@ -231,7 +235,7 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in
if (!pathtv)
path = SYSPROF_DEFAULT_OUTPUT;
else if (!tvisstr(pathtv))
return PROFILE_ERRUSE;
return PROFILE_ERRBADPATH;
else
path = strVdata(pathtv);

Expand All @@ -253,11 +257,12 @@ static int parse_sysprof_opts(lua_State *L, struct luam_Sysprof_Options *opt, in

static int parse_options(lua_State *L, struct luam_Sysprof_Options *opt)
{
if (lua_gettop(L) != 1)
return PROFILE_ERRUSE;
if (lua_gettop(L) != 1) {
lua_createtable(L, 0, 0);
}

if (!lua_istable(L, 1))
return PROFILE_ERRUSE;
return PROFILE_ERRBADTABLE;

return parse_sysprof_opts(L, opt, 1);
}
Expand All @@ -270,6 +275,26 @@ static int sysprof_error(lua_State *L, int status)
lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRBADMODE:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_BADMODE));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRBADINTERVAL:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_BADINTERVAL));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRBADPATH:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_BADPATH));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRBADTABLE:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_BADTABLE));
lua_pushinteger(L, EINVAL);
return 3;
#if LJ_HASSYSPROF
case PROFILE_ERRRUN:
lua_pushnil(L);
Expand Down
4 changes: 4 additions & 0 deletions src/lj_errmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ ERRDEF(FFI_NYICALL, "NYI: cannot call this C function (yet)")

/* Profiler errors. */
ERRDEF(PROF_MISUSE, "profiler misuse")
ERRDEF(PROF_BADMODE, "profiler mode must be 'D', 'L' or 'C'")
ERRDEF(PROF_BADINTERVAL, "profiler interval must be greater than 1")
ERRDEF(PROF_BADPATH, "profiler path does not exist")
ERRDEF(PROF_BADTABLE, "profiler expects a table with parameters")
#if LJ_HASMEMPROF || LJ_HASSYSPROF
ERRDEF(PROF_ISRUNNING, "profiler is running already")
ERRDEF(PROF_NOTRUNNING, "profiler is not running")
Expand Down
5 changes: 5 additions & 0 deletions src/lmisclib.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ struct luam_Sysprof_Options {
#define PROFILE_ERRMEM 3
#define PROFILE_ERRIO 4

#define PROFILE_ERRBADMODE 5
#define PROFILE_ERRBADINTERVAL 6
#define PROFILE_ERRBADPATH 7
#define PROFILE_ERRBADTABLE 8

LUAMISC_API int luaM_sysprof_set_writer(luam_Sysprof_writer writer);

LUAMISC_API int luaM_sysprof_set_on_stop(luam_Sysprof_on_stop on_stop);
Expand Down
45 changes: 40 additions & 5 deletions test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ local test = tap.test("misc-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
Expand Down Expand Up @@ -65,9 +65,25 @@ end

-- Wrong profiling mode.
local res, err, errno = misc.sysprof.start{ mode = "A" }
test:ok(res == nil and err:match("profiler misuse"), "res with no parameters")
test:ok(res == nil, "res with no parameters")
test:ok(err:match("profiler mode must be 'D', 'L' or 'C'"),
"error with no parameters")
test:ok(type(errno) == "number", "errno with no parameters")

-- Missed profiling mode.
res, err, errno = misc.sysprof.start{}
test:is(res, true, "res with missed profiling mode")
test:is(err, nil, "error with missed profiling mode")
test:is(errno, nil, "errno with missed profiling mode")
assert(misc.sysprof.stop(), "sysprof is not stopped")

-- 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)
Expand All @@ -90,10 +106,29 @@ res, err, errno = misc.sysprof.start({ mode = "C", path = BAD_PATH })
test:ok(res == nil and err:match("No such file or directory"), "res 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"), "res and error with bad interval")
test:ok(type(errno) == "number", "errno with bad interval")
test:ok(res == nil, "res 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,
}
test:is(res, true, "res with good interval 1")
test:is(err, nil, "error with good interval 1")
test:is(errno, nil, "errno with good interval 1")
misc.sysprof.stop()

-- DEFAULT MODE

Expand Down

0 comments on commit 5dd3f91

Please sign in to comment.