From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 6EC5112EB02B; Thu, 20 Feb 2025 14:24:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6EC5112EB02B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1740050647; bh=6y83GwXXBwpj2b9DTjK9xxAReu45mqbfaNpyKSn0uZM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=e/4chXnypEleKjFJVjEeNgFHu/w2OZTJLY2uEujCHv8KjJsfKmZu8QumZjki1fe5n 8VBqnVR6SK8dJoQ1+iVVfvYapBXTjKXe1esDnblYHRdKm0p7JpemFTlhBShHkd23G8 K1VlfFc/NRHOzhcXCFR1fb2y2UQUsL51pddHpPVs= Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A99F812EB019 for ; Thu, 20 Feb 2025 14:22:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A99F812EB019 Received: by mail-ej1-f48.google.com with SMTP id a640c23a62f3a-ab78e6edb99so115521566b.2 for ; Thu, 20 Feb 2025 03:22:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740050552; x=1740655352; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=B4CvGH3gyjNyYnVcfoCuQBiXb3gq2gh1acJHRGJdCKc=; b=U9py1wcyx78pJVYbSOzMW1OskFs5jc2oFvI5QLHGTKy2TzerFQW+kr65FwLxW5u/6U zAJIAU28uMakRT+0XMvGS1ETmJtpGpFmOfkJg+Ri2bx2/mUD5JfVr/bRgfvxJHBfX9Wp srZ5ikB2+016g3Ae00NNWPxRZRXi5XXy48JqFzNr+TDTXWkH/3nwMXgdZmuJ+jVt1qrl U9XQzCn5VNamgCsA+JK51xGliZJv/GXxwKgzlMAq12TJub1UAqTj5LFrSiVR2RhVOjtE wiiOOrhPmvdWDqBKEds90L+U/tnLHrjZ1mLw7AXD7ua9wYZiCdJ9J/A8k5AWWzYa0XDk l7tQ== X-Gm-Message-State: AOJu0Yx8JxyAjfdbWug66R9IZNRR+KyFuyX7edaAD6k/0Wo5eBsBtOcB 7RjoG4Or0TYfW1A0oVpq9abFDUEbhLHd4ueLNbl5IbZHtwoPduCZp1npQ5rT X-Gm-Gg: ASbGncs+zhiJ+VU+9oKkCep5dUtHLxO+aLLfrP6pQloQ+LYDvgNeMLp6ZLDYTumZUpz dTOiQWszqNVk4fFkIzHsQ4zjrZ/mftER3RuPsnQiVt0xe4IPzNdFop4ya3a9j54igQx08ORnRcG ZL1RFm1RQIUXvqOEtd6CLoh1SOBh1m/ZV+0QvZRT3/lHwDI8hpwGgdEGgKQWoSd0kLwe4q/u/am T9Y2P/Kgy2ORy8nOxMmZJ4NUJYAxhjCX24HDISNF4XuldWjPVtEiF0fcJ48c0BHorRnpbzenBGm WN9iwg== X-Google-Smtp-Source: AGHT+IES9oz4yIocMFRMIARpBYmT7AYlgh+6W+m+BllX7Ee+u89kxB6N5vUS5FKs+UP7U3nsdTwjRw== X-Received: by 2002:a17:907:3ea7:b0:abb:b2ad:4d3d with SMTP id a640c23a62f3a-abbcccd15a1mr764371266b.8.1740050551480; Thu, 20 Feb 2025 03:22:31 -0800 (PST) Received: from localhost ([5.181.62.98]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-abbac3242dcsm624358866b.107.2025.02.20.03.22.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Feb 2025 03:22:31 -0800 (PST) X-Google-Original-From: Sergey Bronnikov To: tarantool-patches@dev.tarantool.org, Sergey Kaplun , sergos@tarantool.org Date: Thu, 20 Feb 2025 14:21:48 +0300 Message-ID: <881980705ead69f58c0474d5f49c00603e4c6e72.1740050074.git.sergeyb@tarantool.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit 4/8][v3] sysprof: introduce specific errors and default mode X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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