* [Tarantool-patches] [PATCH luajit 1/8][v3] test: add descriptions to sysprof testcases
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 2/8][v3] test: align test title with test filename Sergey Bronnikov via Tarantool-patches
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
The patch adds descriptions to sysprof testcases to make the TAP
report more usable.
---
.../profilers/misclib-sysprof-lapi.test.lua | 43 ++++++++++++-------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index b2c38c49..237143ad 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -65,34 +65,38 @@ end
-- Wrong profiling mode.
local res, err, errno = misc.sysprof.start{ mode = "A" }
-test:ok(res == nil and err:match("profiler misuse"))
-test:ok(type(errno) == "number")
+test:ok(res == nil and err:match("profiler misuse"),
+ "result status with wrong profiling mode")
+test:ok(type(errno) == "number", "errno with wrong profiling mode")
-- Already running.
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 is running already"))
-test:ok(type(errno) == "number")
+test:ok(res == nil and err:match("profiler is running already"),
+ "ok with already running")
+test:ok(type(errno) == "number", "errno with already running")
res, err = misc.sysprof.stop()
assert(res, err)
-- Not running.
res, err, errno = misc.sysprof.stop()
-test:ok(res == nil and err)
-test:ok(type(errno) == "number")
+test:ok(res == nil and err, "result status and error with not running")
+test:ok(type(errno) == "number", "errno with not running")
-- Bad path.
res, err, errno = misc.sysprof.start({ mode = "C", path = BAD_PATH })
-test:ok(res == nil and err:match("No such file or directory"))
-test:ok(type(errno) == "number")
+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.
res, err, errno = misc.sysprof.start{ mode = "C", interval = -1 }
-test:ok(res == nil and err:match("profiler misuse"))
-test:ok(type(errno) == "number")
+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")
-- DEFAULT MODE
@@ -102,20 +106,26 @@ end
local report = misc.sysprof.report()
--- Check the profile is not empty
-test:ok(report.samples > 0)
+-- Check the profile is not empty.
+test:ok(report.samples > 0,
+ "number of samples is greater than 0 for the default payload")
-- There is a Lua function with FNEW bytecode in it. Hence there
-- are only three possible sample types:
-- * LFUNC -- Lua payload is sampled.
-- * GC -- Lua GC machinery triggered in scope of FNEW bytecode
-- is sampled.
-- * INTERP -- VM is in a specific state when the sample is taken.
-test:ok(report.vmstate.LFUNC + report.vmstate.GC + report.vmstate.INTERP > 0)
+test:ok(report.vmstate.LFUNC + report.vmstate.GC + report.vmstate.INTERP > 0,
+ "total number of LFUNC, GC and INTERP samples is greater than 0 " ..
+ "for the default payload")
-- There is no fast functions and C function in default payload.
-test:ok(report.vmstate.FFUNC + report.vmstate.CFUNC == 0)
+test:ok(report.vmstate.FFUNC + report.vmstate.CFUNC == 0,
+ "total number of FFUNC and CFUNC samples is equal to 0 " ..
+ "for the default payload")
-- Check all JIT-related VM states are not sampled.
+local msg = "total number of VM state %s is equal to 0 for the default payload"
for _, vmstate in pairs({ 'TRACE', 'RECORD', 'OPT', 'ASM', 'EXIT' }) do
- test:ok(report.vmstate[vmstate] == 0)
+ test:ok(report.vmstate[vmstate] == 0, msg:format(vmstate))
end
-- With very big interval.
@@ -124,7 +134,8 @@ if not pcall(generate_output, { mode = "D", interval = 1000 }) then
end
report = misc.sysprof.report()
-test:ok(report.samples == 0)
+test:ok(report.samples == 0, "total number of samples is equal to 0 for " ..
+ "the too big sampling interval")
-- LEAF MODE
check_mode("L", 100)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 2/8][v3] test: align test title with test filename
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 1/8][v3] test: add descriptions to sysprof testcases Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 3/8][v3] sysprof: fix typo in the comment Sergey Bronnikov via Tarantool-patches
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
The patch fixes TAP test title to make it aligned with test
filename. Inconsistency introduced in commit
efd7e1a67de24464e1e83793e16eac6df443e3c0
("test: make skipcond helper more convenient").
---
test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua | 2 +-
test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
index ce41e4d5..077a7e44 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
@@ -1,7 +1,7 @@
-- XXX: This comment is a reminder to reimplement memprof tests
-- assertions to make them more independent to the changes made.
local tap = require("tap")
-local test = tap.test("misc-memprof-lapi"):skipcond({
+local test = tap.test("misclib-memprof-lapi"):skipcond({
['Test requires JIT enabled'] = not jit.status(),
['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 237143ad..32fa384c 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -1,5 +1,5 @@
local tap = require("tap")
-local test = tap.test("misc-sysprof-lapi"):skipcond({
+local test = tap.test("misclib-sysprof-lapi"):skipcond({
["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
jit.arch ~= "x64",
["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 3/8][v3] sysprof: fix typo in the comment
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 1/8][v3] test: add descriptions to sysprof testcases Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 2/8][v3] test: align test title with test filename Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 4/8][v3] sysprof: introduce specific errors and default mode Sergey Bronnikov via Tarantool-patches
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
The patch fixes inconsistency in comment and source code below
the comment. The typo was added in the initial commit
2593a9f83072ca999d5f528e1da481e8ba51d5b8 ("misc: introduce Lua
API for new sampling profiler").
Follows up tarantool/tarantool#781
---
src/lib_misc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lib_misc.c b/src/lib_misc.c
index ca1d1c75..5b7a4b62 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -161,7 +161,7 @@ static int on_stop_cb_default(void *opt, uint8_t *buf)
#define LJLIB_MODULE_misc_sysprof
-/* The default profiling interval equals to 11 ms. */
+/* The default profiling interval equals to 10 ms. */
#define SYSPROF_DEFAULT_INTERVAL 10
#define SYSPROF_DEFAULT_OUTPUT "sysprof.bin"
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 4/8][v3] sysprof: introduce specific errors and default mode
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
` (2 preceding siblings ...)
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 3/8][v3] sysprof: fix typo in the comment Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 5/8][v3] test: introduce flag LUAJIT_DISABLE_MEMPROF Sergey Bronnikov via Tarantool-patches
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
sysprof has a number of options and with any incorrect option it
returns `false` and error message "profiler misuse". This message
discourages sysprof users and makes using sysprof more
complicated. The patch sets the 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 | 77 ++++++++++++-------
src/lj_errmsg.h | 5 ++
.../profilers/misclib-sysprof-lapi.test.lua | 68 ++++++++++++++--
3 files changed, 114 insertions(+), 36 deletions(-)
diff --git a/src/lib_misc.c b/src/lib_misc.c
index 5b7a4b62..1fd06dd1 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -14,6 +14,7 @@
#include "lj_obj.h"
#include "lj_str.h"
+#include "lj_strfmt.h"
#include "lj_tab.h"
#include "lj_lib.h"
#include "lj_gc.h"
@@ -163,6 +164,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 +179,36 @@ 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 == 0) {
+ opt->mode = LUAM_SYSPROF_DEFAULT;
+ opt->interval = SYSPROF_DEFAULT_INTERVAL;
+ return PROFILE_SUCCESS;
+ }
+
+ 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 +221,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,8 +232,10 @@ 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;
}
}
@@ -230,9 +250,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
+ } else
path = strVdata(pathtv);
ctx = lj_mem_new(L, sizeof(*ctx));
@@ -251,29 +272,26 @@ 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) {
+ lj_strfmt_pushf(L, "%s: %s", err2msg(LJ_ERR_PROF_MISUSE), err_details);
+ } else {
+ lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
+ }
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) {
+ lj_strfmt_pushf(L, "%s: %s", err2msg(LJ_ERR_PROF_MISUSE), err_details);
+ } else {
+ lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
+ }
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRIO:
@@ -291,15 +309,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 +329,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 +344,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..9915f565 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(39)
jit.off()
-- XXX: Run JIT tuning functions in a safe frame to avoid errors
@@ -24,6 +24,8 @@ local profilename = require("utils").tools.profilename
local TMP_BINFILE = profilename("sysprofdata.tmp.bin")
local BAD_PATH = profilename("sysprofdata/tmp.bin")
+local BAD_MODE = "A"
+local BAD_INTERVAL = -1
local function payload()
local function fib(n)
@@ -64,11 +66,44 @@ end
-- GENERAL
-- 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")
+local res, err, errno = misc.sysprof.start{ mode = BAD_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 = pcall(misc.sysprof.start, "NOT A TABLE")
+print(res, err, errno)
+test:is(res, false, "res with not a table")
+test:ok(err:match("table expected, got string"),
+ "error with not a table")
+test:is(errno, nil, "errno with not a table")
+
+-- All parameters are invalid.
+res, err, errno = misc.sysprof.start({
+ mode = BAD_MODE, path = BAD_PATH, interval = BAD_INTERVAL })
+test:ok(res == nil, "res with all invalid parameters")
+test:ok(err:match("profiler misuse: profiler mode must be 'D', 'L' or 'C'"),
+ "error with all invalid parameters")
+test:ok(type(errno) == "number", "errno with all invalid parameters")
+
+-- All parameters are invalid, except the first one.
+res, err, errno = misc.sysprof.start({
+ mode = "C", path = BAD_PATH, interval = BAD_INTERVAL })
+test:ok(res == nil, "res with all invalid parameters except the first one")
+test:ok(err:match("profiler misuse: profiler interval must be greater than 1"),
+ "error with all invalid parameters except the first one")
+test:ok(type(errno) == "number",
+ "errno with all invalid parameters except the first one")
+
-- Already running.
res, err = misc.sysprof.start{ mode = "D" }
assert(res, err)
@@ -93,11 +128,30 @@ test:ok(res == nil and err:match("No such file or directory"),
test:ok(type(errno) == "number", "errno with bad path")
-- Bad interval.
-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")
+res, err, errno = misc.sysprof.start{ mode = "C", interval = BAD_INTERVAL }
+test:is(res, nil, "result status and error with bad interval")
+test:ok(err:match("profiler interval must be greater than 1"),
+ "error with bad interval")
test:ok(type(errno) == "number", "errno with bad interval")
+-- 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
if not pcall(generate_output, { mode = "D", interval = 100 }) then
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 5/8][v3] test: introduce flag LUAJIT_DISABLE_MEMPROF
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
` (3 preceding siblings ...)
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 4/8][v3] sysprof: introduce specific errors and default mode Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 6/8][v3] ci: add workflow with disabled profilers Sergey Bronnikov via Tarantool-patches
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
The patch introduce an environment variable LUAJIT_DISABLE_MEMPROF
needed by tests in suite `tarantool-tests` for skip testcases when
memprof is disabled in build.
Needed for the following commit.
---
test/tarantool-tests/CMakeLists.txt | 4 ++++
.../gh-5994-memprof-human-readable.test.lua | 1 +
.../profilers/misclib-memprof-lapi.test.lua | 15 ++++++++-------
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 9bacac88..a6d09dc7 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -97,6 +97,10 @@ if(LUAJIT_DISABLE_SYSPROF)
list(APPEND LUA_TEST_ENV_MORE LUAJIT_DISABLE_SYSPROF=1)
endif()
+if(LUAJIT_DISABLE_MEMPROF)
+ list(APPEND LUA_TEST_ENV_MORE LUAJIT_DISABLE_MEMPROF=1)
+endif()
+
# Needed to skip some long tests or tests using signals.
# See also https://github.com/tarantool/tarantool/issues/10803.
if(LUAJIT_USE_VALGRIND)
diff --git a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
index f3041779..3dc0db6b 100644
--- a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
+++ b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
@@ -7,6 +7,7 @@ local test = tap.test('gh-5994-memprof-human-readable'):skipcond({
['No profile tools CLI option integration'] = _TARANTOOL,
-- See also https://github.com/LuaJIT/LuaJIT/issues/606.
['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
+ ['Memprof is disabled'] = os.getenv('LUAJIT_DISABLE_MEMPROF'),
})
local utils = require('utils')
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
index 077a7e44..cd675864 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
@@ -8,6 +8,7 @@ local test = tap.test("misclib-memprof-lapi"):skipcond({
jit.arch ~= "x64",
-- See also https://github.com/LuaJIT/LuaJIT/issues/606.
["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
+ ["Memprof is disabled"] = os.getenv("LUAJIT_DISABLE_MEMPROF"),
})
test:plan(5)
@@ -164,9 +165,9 @@ test:test("output", function(subtest)
-- one is the number of allocations. 1 event - allocation of
-- table by itself + 1 allocation of array part as far it is
-- bigger than LJ_MAX_COLOSIZE (16).
- subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
+ subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 38 }, 2))
-- 20 strings allocations.
- subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 20))
+ subtest:ok(check_alloc_report(alloc, { line = 45, linedefined = 38 }, 20))
-- Collect all previous allocated objects.
subtest:ok(free.INTERNAL == 22)
@@ -174,8 +175,8 @@ test:test("output", function(subtest)
-- Tests for leak-only option.
-- See also https://github.com/tarantool/tarantool/issues/5812.
local heap_delta = process.form_heap_delta(events)
- local tab_alloc_stats = heap_delta[form_source_line(39)]
- local str_alloc_stats = heap_delta[form_source_line(44)]
+ local tab_alloc_stats = heap_delta[form_source_line(40)]
+ local str_alloc_stats = heap_delta[form_source_line(45)]
subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
subtest:ok(tab_alloc_stats.dbytes == 0)
subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
@@ -260,10 +261,10 @@ test:test("jit-output", function(subtest)
-- 10 allocations in interpreter mode, 1 allocation for a trace
-- recording and assembling and next 9 allocations will happen
-- while running the trace.
- subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 11))
- subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 42 }, 9))
+ subtest:ok(check_alloc_report(alloc, { line = 45, linedefined = 38 }, 11))
+ subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 43 }, 9))
-- See same checks with jit.off().
- subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
+ subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 38 }, 2))
-- Restore default JIT settings.
jit.opt.start(unpack(jit_opt_default))
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 6/8][v3] ci: add workflow with disabled profilers
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
` (4 preceding siblings ...)
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 5/8][v3] test: introduce flag LUAJIT_DISABLE_MEMPROF Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 7/8][v3] misc: specific message for " Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 8/8][v3] memprof: set default path to profiling output file Sergey Bronnikov via Tarantool-patches
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
Needed for the following commit.
---
.github/workflows/exotic-builds-testing.yml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
index 374c879b..0c575404 100644
--- a/.github/workflows/exotic-builds-testing.yml
+++ b/.github/workflows/exotic-builds-testing.yml
@@ -34,7 +34,7 @@ jobs:
BUILDTYPE: [Debug, Release]
ARCH: [ARM64, x86_64]
GC64: [ON, OFF]
- FLAVOR: [checkhook, dualnum, dynamic, gdbjit, nojit, nounwind, tablebump]
+ FLAVOR: [checkhook, dualnum, dynamic, gdbjit, nojit, noprof, nounwind, tablebump]
include:
- BUILDTYPE: Debug
CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
@@ -48,6 +48,8 @@ jobs:
FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
- FLAVOR: nojit
FLAVORFLAGS: -DLUAJIT_DISABLE_JIT=ON
+ - FLAVOR: noprof
+ FLAVORFLAGS: -DLUAJIT_DISABLE_MEMPROF=ON -DLUAJIT_DISABLE_SYSPROF=ON
- FLAVOR: gdbjit
FLAVORFLAGS: -DLUAJIT_USE_GDBJIT=ON
- FLAVOR: nounwind
@@ -66,6 +68,8 @@ jobs:
ARCH: ARM64
- FLAVOR: dynamic
BUILDTYPE: Release
+ - FLAVOR: noprof
+ ARCH: ARM64
# With table bump optimization enabled (and due to our modification
# related to metrics), some offsets in GG_State stop fitting in 12bit
# immediate. Hence, the build failed due to the DASM error
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 7/8][v3] misc: specific message for disabled profilers
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
` (5 preceding siblings ...)
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 6/8][v3] ci: add workflow with disabled profilers Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 8/8][v3] memprof: set default path to profiling output file Sergey Bronnikov via Tarantool-patches
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
sysprof and memprof Lua API functions return an error message
"profiler misuse", when the corresponding profiler is disabled in
the build. It is not possible to easily distinguish whether it is
really misuse or if the profiler was not enabled in the build. The
patch changes error messages, so when profiler is not enabled in
the build, the message is the following: "profiler misuse:
profiler is disabled".
---
src/lib_misc.c | 25 ++++++++++++++--
src/lj_errmsg.h | 1 +
.../misclib-memprof-lapi-disabled.test.lua | 22 ++++++++++++++
.../misclib-sysprof-lapi-disabled.test.lua | 29 +++++++++++++++++++
4 files changed, 75 insertions(+), 2 deletions(-)
create mode 100644 test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
create mode 100644 test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
diff --git a/src/lib_misc.c b/src/lib_misc.c
index 1fd06dd1..d98cf3f0 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -306,10 +306,15 @@ static int sysprof_error(lua_State *L, int status, const char *err_details)
/* local res, err, errno = sysprof.start(options) */
LJLIB_CF(misc_sysprof_start)
{
+ const char *err_details = NULL;
int status = PROFILE_SUCCESS;
struct luam_Sysprof_Options opt = {};
- const char *err_details = NULL;
+
+ if (!LJ_HASSYSPROF) {
+ err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+ return sysprof_error(L, PROFILE_ERRUSE, err_details);
+ }
status = parse_sysprof_opts(L, &opt, &err_details);
if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
@@ -328,6 +333,10 @@ LJLIB_CF(misc_sysprof_start)
LJLIB_CF(misc_sysprof_stop)
{
int status = luaM_sysprof_stop(L);
+ if (!LJ_HASSYSPROF) {
+ const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+ return sysprof_error(L, PROFILE_ERRUSE, err_details);
+ }
if (LJ_UNLIKELY(status != PROFILE_SUCCESS))
return sysprof_error(L, status, NULL);
@@ -341,8 +350,12 @@ LJLIB_CF(misc_sysprof_report)
struct luam_Sysprof_Counters counters = {};
GCtab *data_tab = NULL;
GCtab *count_tab = NULL;
-
int status = luaM_sysprof_report(&counters);
+ if (!LJ_HASSYSPROF) {
+ const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+ return sysprof_error(L, PROFILE_ERRUSE, err_details);
+ }
+
if (status != PROFILE_SUCCESS)
return sysprof_error(L, status, NULL);
@@ -381,6 +394,10 @@ LJLIB_CF(misc_memprof_start)
const char *fname = strdata(lj_lib_checkstr(L, 1));
struct profile_ctx *ctx;
int memprof_status;
+ if (!LJ_HASMEMPROF) {
+ const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+ return sysprof_error(L, PROFILE_ERRUSE, err_details);
+ }
/*
** FIXME: more elegant solution with ctx.
@@ -432,6 +449,10 @@ LJLIB_CF(misc_memprof_start)
LJLIB_CF(misc_memprof_stop)
{
int status = lj_memprof_stop(L);
+ if (!LJ_HASMEMPROF) {
+ const char *err_details = err2msg(LJ_ERR_PROF_DETAILS_DISABLED);
+ return sysprof_error(L, PROFILE_ERRUSE, err_details);
+ }
if (status != PROFILE_SUCCESS) {
switch (status) {
case PROFILE_ERRUSE:
diff --git a/src/lj_errmsg.h b/src/lj_errmsg.h
index b5c3a275..e26f5e38 100644
--- a/src/lj_errmsg.h
+++ b/src/lj_errmsg.h
@@ -193,6 +193,7 @@ 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")
+ERRDEF(PROF_DETAILS_DISABLED, "profiler is disabled")
#undef ERRDEF
/* Detecting unused error messages:
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
new file mode 100644
index 00000000..de0aa136
--- /dev/null
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
@@ -0,0 +1,22 @@
+local tap = require('tap')
+local test = tap.test('misclib-memprof-lapi-disabled'):skipcond({
+ ['Memprof is enabled'] = not os.getenv('LUAJIT_DISABLE_MEMPROF'),
+})
+
+test:plan(6)
+
+-- Attempt to start memprof when it is disabled.
+local res, err, errno = misc.memprof.start()
+test:is(res, nil, 'result status on start when memprof is disabled')
+test:ok(err:match('profiler is disabled'),
+ 'error on start when memprof is disabled')
+test:ok(type(errno) == 'number', 'errno on start when memprof is disabled')
+
+-- Attempt to stop memprof when it is disabled.
+res, err, errno = misc.memprof.stop()
+test:is(res, nil, 'result status on stop when memprof is disabled')
+test:ok(err:match('profiler is disabled'),
+ 'error on stop when memprof is disabled')
+test:ok(type(errno) == 'number', 'errno on start when memprof is disabled')
+
+test:done(true)
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
new file mode 100644
index 00000000..2a9ce796
--- /dev/null
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
@@ -0,0 +1,29 @@
+local tap = require('tap')
+local test = tap.test('misclib-sysprof-lapi-disabled'):skipcond({
+ ['Sysprof is enabled'] = not os.getenv('LUAJIT_DISABLE_SYSPROF'),
+})
+
+test:plan(9)
+
+-- Attempt to start sysprof when sysprof is disabled.
+local res, err, errno = misc.sysprof.start()
+test:is(res, nil, 'result status on start when sysprof is disabled')
+test:ok(err:match('profiler is disabled'),
+ 'error on start when sysprof is disabled')
+test:ok(type(errno) == 'number', 'errno on start when sysprof is disabled')
+
+-- Attempt to stop sysprof when sysprof is disabled.
+res, err, errno = misc.sysprof.stop()
+test:is(res, nil, 'result status on stop when sysprof is disabled')
+test:ok(err:match('profiler is disabled'),
+ 'error on stop when sysprof is disabled')
+test:ok(type(errno) == 'number', 'errno on start when sysprof is disabled')
+
+-- Attempt to report when sysprof is disabled.
+res, err, errno = misc.sysprof.report()
+test:is(res, nil, 'result status on report when sysprof is disabled')
+test:ok(err:match('profiler is disabled'),
+ 'error on stop when sysprof is disabled')
+test:ok(type(errno) == 'number', 'errno on start when sysprof is disabled')
+
+test:done(true)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Tarantool-patches] [PATCH luajit 8/8][v3] memprof: set default path to profiling output file
2025-02-20 11:21 [Tarantool-patches] [PATCH luajit 0/8][v3] Fix profilers issues Sergey Bronnikov via Tarantool-patches
` (6 preceding siblings ...)
2025-02-20 11:21 ` [Tarantool-patches] [PATCH luajit 7/8][v3] misc: specific message for " Sergey Bronnikov via Tarantool-patches
@ 2025-02-20 11:21 ` Sergey Bronnikov via Tarantool-patches
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-02-20 11:21 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, sergos
sysprof has an optional parameter `path`, that sets a path to
the profiling output file. By default the path is `sysprof.bin`.
`misc.memprof.start()` requires to set a path to profiling output
file. The patch fixes this inconsistency by introducing a default
path to the memprof profiling output file - `memprof.bin`.
---
src/lib_misc.c | 5 ++-
...misclib-memprof-lapi-default-file.test.lua | 37 +++++++++++++++++++
2 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
diff --git a/src/lib_misc.c b/src/lib_misc.c
index d98cf3f0..92dc6e2a 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -387,11 +387,14 @@ LJLIB_CF(misc_sysprof_report)
#define LJLIB_MODULE_misc_memprof
+#define MEMPROF_DEFAULT_OUTPUT "memprof.bin"
+
/* local started, err, errno = misc.memprof.start(fname) */
LJLIB_CF(misc_memprof_start)
{
struct lj_memprof_options opt = {0};
- const char *fname = strdata(lj_lib_checkstr(L, 1));
+ GCstr *s = lj_lib_optstr(L, 1);
+ const char *fname = s ? strdata(s) : MEMPROF_DEFAULT_OUTPUT;
struct profile_ctx *ctx;
int memprof_status;
if (!LJ_HASMEMPROF) {
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
new file mode 100644
index 00000000..ae8a73c9
--- /dev/null
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
@@ -0,0 +1,37 @@
+local tap = require('tap')
+local test = tap.test('misc-memprof-lapi-default-file'):skipcond({
+ ['Memprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and
+ jit.arch ~= 'x64',
+ ['Memprof is disabled'] = os.getenv('LUAJIT_DISABLE_MEMPROF'),
+})
+
+test:plan(1)
+
+local tools = require('utils.tools')
+
+test:test('default-output-file', function(subtest)
+
+ subtest:plan(1)
+
+ local default_output_file = 'memprof.bin'
+ os.remove(default_output_file)
+
+ local _, _ = misc.memprof.start()
+
+ local res, err = misc.memprof.stop()
+
+ -- Want to cleanup carefully if something went wrong.
+ if not res then
+ os.remove(default_output_file)
+ error(err)
+ end
+
+ local profile_buf = tools.read_file(default_output_file)
+ subtest:ok(profile_buf ~= nil and #profile_buf ~= 0,
+ 'default output file is not empty')
+
+ -- We don't need it any more.
+ os.remove(default_output_file)
+end)
+
+test:done(true)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread