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 712E612292D2; Thu, 13 Feb 2025 14:13:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 712E612292D2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1739445183; bh=J02+PeRGZ7Fr9pwFUviiY311ug0JCNwWOxzyz4XiUZY=; 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=r1JAxNC+GYgDOdOfN6/krsIL1e23dxYJbU0PZQTUtSzJgE7XIWQ/qnQd2CANT+zzg YN4NJehNnOdqfbmCZfL9OOJup2gbgADIkWxjiidTd1pCbd+JCkEitXOLrWEbYctoB5 Z3I8PHprfL0lJPHgW2XCx0kgl+2wO8EMc18A0NiA= Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) (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 3CCB112292D7 for ; Thu, 13 Feb 2025 14:11:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3CCB112292D7 Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-5de51a735acso1394525a12.0 for ; Thu, 13 Feb 2025 03:11:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739445095; x=1740049895; 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=J8RnZz3bRf8w0Xu9FD4VJR3DPO0KI+G2TdMT7avU8Mo=; b=iiLpLIG0uvfhFb0QyhhWzcmaAbmSY96FntUbw6IpKYYdM/FTC0Wz5gar6Ap5wt8C5p aAvpE2LfjJzjvO/gHuOGcvd9gUuR9XSugg+7eQLLmdju3+Vaa4ITIdfIZ77JU7ta3PrH 7EARRXGo1w++EnwtvFmYlT417Ng12N//ze/rJ57t2NKDk56wZEXrmnJZzPd9vUhywRTA Bvajzlmb0jVzQymzKOprG1hDGtVfD00B6wWYyjLbtzYJrLxAOEGBapHaTpgGAvFAnhS1 ciaRIshycrUK5IQq1jHuzUj/DrzMzAkcmv2pTwzM3rd/NhVSyKYLYUXO9++bpgdcc5l2 1hhg== X-Gm-Message-State: AOJu0YwMi3cdSzPBuJ6yKj/UtyEkvoxFHD8BId6m1iQaLtRho2CzwozB S/CTr3Al3XKdIZrUzPlP09lzjhIZkdn0gbfSE7DL+o7F0lPzYL9OJ0LE5+fm X-Gm-Gg: ASbGncvyCU9NgwImwd1w8sqaI9WBJTCpymDnreP5Vy+JkBvSvnR+qE8ee+afNOwt30J AK1ROC+Z9V9CNFb0yl2EjuEDcG66gsM4l5+kghwTYr2w45m3cocEzywFatAJF9YfEWXe6fAzAxb /FkYMOk1k5Po1mYumL8pB5FKZEh6L9yecgTLFB7I2qF6+TZ2/vERJPMInBSkzxlXitAYOLaMH+W 6RDdzLws/oLHJYen5MyRuonzhgQcrDUvMTaIOyxThIT0ndGFEJvUXTp8YUegtRpsir1/Y/9T6MG 0OKO X-Google-Smtp-Source: AGHT+IHPBteHjEW0bBxdtpoq1dgJJ+pBEt/wSV8goi3qokGGAPgwo8p4aCwAODE+Eg1AmcNEcu7eig== X-Received: by 2002:a05:6402:2791:b0:5dc:113c:46c3 with SMTP id 4fb4d7f45d1cf-5dec9ffa138mr1935912a12.21.1739445094895; Thu, 13 Feb 2025 03:11:34 -0800 (PST) Received: from localhost ([5.181.62.98]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5dece1d3619sm996049a12.36.2025.02.13.03.11.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 03:11:34 -0800 (PST) X-Google-Original-From: Sergey Bronnikov To: tarantool-patches@dev.tarantool.org, Sergey Kaplun Date: Thu, 13 Feb 2025 14:10:45 +0300 Message-Id: X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit 4/7][v2] 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 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