[Tarantool-patches] [PATCH luajit v9 1/2] tools: add cli flag to run profile dump parsers
Maxim Kokryashkin
m.kokryashkin at tarantool.org
Tue Sep 12 23:03:40 MSK 2023
Hi, Sergey!
Thanks for the review!
Fixed all of your comments, except for the one about
`luacheckrc` and _TARANTOOL. I believe, it should be
done in a separate patch.
Here is the diff for other changes:
diff --git a/src/luajit.c b/src/luajit.c
index 84472464..b63c92d1 100644
--- a/src/luajit.c
+++ b/src/luajit.c
@@ -79,6 +79,17 @@ static void print_usage(void)
fflush(stderr);
}
+static void print_tools_usage(void)
+{
+ fputs("usage: ", stderr);
+ fputs(progname, stderr);
+ fputs(" -t<cmd>\n"
+ "Available tools are:\n"
+ " -m [--leak-only] input Memprof profile data parser.\n"
+ " -s input Sysprof profile data parser.\n", stderr);
+ fflush(stderr);
+}
+
static void l_message(const char *pname, const char *msg)
{
if (pname) { fputs(pname, stderr); fputc(':', stderr); fputc(' ', stderr); }
@@ -387,7 +398,7 @@ static int dotoolcmd(lua_State *L, const char *cmd)
case 's':
return runtoolcmd(L, "sysprof");
default:
- l_message(progname, "unknown tool command");
+ print_tools_usage();
break;
}
return -1;
@@ -454,8 +465,6 @@ static int collectargs(char **argv, int *flags)
break;
case 't':
*flags |= FLAGS_TOOL;
- if (argv[i][2] == '\0') return -1;
- if (argv[i + 1] == NULL) return -1;
return i + 1;
case 'e':
*flags |= FLAGS_EXEC;
diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
index ce99535a..8187c594 100644
--- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
@@ -28,12 +28,11 @@ local TABLE_SIZE = 20
local SMOKE_CMD_SET = {
{
cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
- -- luacheck: no global
- like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
+ like = '.+Available tools.+',
},
{
cmd = EXECUTABLE .. ' -ta ' .. BAD_PATH,
- like = '.+unknown tool command',
+ like = '.+Available tools.+',
},
}
--
Best regards,
Maxim Kokryashkin
>Четверг, 7 сентября 2023, 12:41 +03:00 от Sergey Bronnikov <sergeyb at tarantool.org>:
>
>Hi, Max
>
>
>On 8/31/23 14 :35, Maxim Kokryashkin wrote:
>> It is really inconvenient to use a standalone shell script to parse
>> binary dumps from profilers. That is why this commit introduces a
>> CLI flag for tools in the LuaJIT, so now it is possible to parse
>> a memprof dump as simple as:
>> ```
>> luajit -tm memprof.bin
>> luajit -tm --leak-only memprof.bin
>> ```
>>
>> And the sysprof too:
>> ```
>> luajit -ts sysprof.bin
>> luajit -ts --split sysprof.bin
>
>$ ./build/src/luajit -ts --split sysprof.bin
>luajit-parse-sysprof.lua: ERROR: unrecognized option `--split'. Try
>`--help'.
>
>
>> ```
>>
>> The `luajit-parse-memprof` and `luajit-parse-sysprof` are purged
>> as a result of these changes.
>Typo: s/The/The scripts/
>>
>> Closes tarantool/tarantool#5688
>> ---
><snipped>
>> diff --git a/src/luajit.c b/src/luajit.c
>> index 3a3ec247..f57b7e95 100644
>> --- a/src/luajit.c
>> +++ b/src/luajit.c
>> @@ -72,6 +72,7 @@ static void print_usage(void)
>> " -O[opt] Control LuaJIT optimizations.\n"
>> " -i Enter interactive mode after executing " LUA_QL("script") ".\n"
>> " -v Show version information.\n"
>> + " -t<cmd> Execute tool.\n"
>
>
>When I run "luajit -b" without arguments LuaJIT shows me a usage for "-b":
>
>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -b
>Save LuaJIT bytecode: luajit -b[options] input output
> -l Only list bytecode.
> -s Strip debug info (default).
> -g Keep debug info.
> -n name Set module name (default: auto-detect from input name).
> -t type Set output file type (default: auto-detect from output name).
> -a arch Override architecture for object files (default: native).
> -o os Override OS for object files (default: native).
> -e chunk Use chunk string as input.
> -- Stop handling options.
> - Use stdin as input and/or stdout as output.
>
>File types: c h obj o raw (default)
>
>
>When I run "luajit -t" without arguments LuaJIT shows me the same usage,
>that is not helpful:
>
>
>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -t
>usage: ./build/src/luajit [options]... [script [args]...].
>Available options are:
> -e chunk Execute string 'chunk'.
> -l name Require library 'name'.
> -b ... Save or list bytecode.
> -j cmd Perform LuaJIT control command.
> -O[opt] Control LuaJIT optimizations.
> -i Enter interactive mode after executing 'script'.
> -v Show version information.
> -t<cmd> Execute tool.
> -E Ignore environment variables.
> -- Stop handling options.
> - Execute stdin and stop handling options.
>
>Please show a usage with available arguments for "-t":
>
>
>Parse profile dump: luajit -t[options] input
> -m parse memprof dump.
> -s parse sysprof dump (default).
>
> --leak-only ...
>
> --split ...
>
>
>
>> " -E Ignore environment variables.\n"
>> " -- Stop handling options.\n"
>> " - Execute stdin and stop handling options.\n", stderr);
>> @@ -361,6 +362,37 @@ static int dojitcmd(lua_State *L, const char *cmd)
>> return runcmdopt(L, opt ? opt+1 : opt);
>> }
>>
>
><snipped>
>
>
>> diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
>> new file mode 100644
>> index 00000000..4dc4f6a1
>> --- /dev/null
>> +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
>> @@ -0,0 +1,127 @@
>> +local tap = require('tap')
>> +local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
>> + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and
>> + jit.arch ~= 'x64',
>
>indentation
>
>
>> + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
>> + -- XXX: Tarantool integration is required to run this test properly.
>> + -- luacheck: no global
>> + ['No profile tools CLI option integration'] = _TARANTOOL,
>> +})
>> +
>> +test:plan(3)
>> +
>> +jit.off()
>> +jit.flush()
>> +
>> +local table_new = require('table.new')
>> +local utils = require('utils')
>> +
>> +local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin')
>> +local TMP_BINFILE_MEMPROF = utils.tools.profilename('memprofdata.tmp.bin')
>> +local TMP_BINFILE_SYSPROF = utils.tools.profilename('sysprofdata.tmp.bin')
>> +
>> +local EXECUTABLE = utils.exec.luacmd(arg)
>> +local MEMPROF_PARSER = EXECUTABLE .. ' -tm '
>> +local SYSPROF_PARSER = EXECUTABLE .. ' -ts '
>> +
>> +local REDIRECT_OUTPUT = ' 2>&1'
>> +
>> +local TABLE_SIZE = 20
>> +
>> +local SMOKE_CMD_SET = {
>> + {
>> + cmd = EXECUTABLE .. ' -t ' .. BAD_PATH,
>> + -- luacheck: no global
>> + like = _TARANTOOL and '.+unknown tool command' or 'usage:.+',
>
>Let's define this global in .luacheckrc.
>
>I think we will use _TARANTOOL more and this inline suppressions will be
>annoying.
>
>
><no obvious problems, snipped>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20230912/869c446b/attachment.htm>
More information about the Tarantool-patches
mailing list