* [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
@ 2023-07-10 11:24 Maksim Kokryashkin via Tarantool-patches
2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Maksim Kokryashkin via Tarantool-patches @ 2023-07-10 11:24 UTC (permalink / raw)
To: tarantool-patches, sergeyb, skaplun, m.kokryashkin
From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
It is really inconvenient to use a standalone shell script to parse
memprof dump. That is why this commit introduces a CLI flag for tools
to the LuaJIT, so now it is possible to parse memprof dump
as simple as:
```
luajit -tm memprof.bin
```
Closes tarantool/tarantool#5688
---
Changes in v7:
- Fixed comments as per review by Sergey
Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse
PR: https://github.com/tarantool/tarantool/pull/8002
src/luajit.c | 32 +++++++++-
.../gh-5688-memprof-cli-flag.test.lua | 58 +++++++++++++++++++
2 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
diff --git a/src/luajit.c b/src/luajit.c
index 1ca24301..d335f32b 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"
" -E Ignore environment variables.\n"
" -- Stop handling options.\n"
" - Execute stdin and stop handling options.\n", stderr);
@@ -361,6 +362,25 @@ static int dojitcmd(lua_State *L, const char *cmd)
return runcmdopt(L, opt ? opt+1 : opt);
}
+static int dotoolcmd(lua_State *L, const char *cmd)
+{
+ if (strcmp(cmd, "m") == 0) {
+ lua_getglobal(L, "require");
+ lua_pushliteral(L, "memprof");
+ if (lua_pcall(L, 1, 1, 0)) {
+ const char *msg = lua_tostring(L, -1);
+ if (msg && !strncmp(msg, "module ", 7))
+ l_message(progname,
+ "tools.* modules not installed");
+ return 1;
+ }
+ lua_getglobal(L, "arg");
+ return report(L, lua_pcall(L, 1, 1, 0));
+ }
+ l_message(progname, "invalid tool command");
+ return -1;
+}
+
/* Optimization flags. */
static int dojitopt(lua_State *L, const char *opt)
{
@@ -398,6 +418,7 @@ static int dobytecode(lua_State *L, char **argv)
#define FLAGS_EXEC 4
#define FLAGS_OPTION 8
#define FLAGS_NOENV 16
+#define FLAGS_TOOL 32
static int collectargs(char **argv, int *flags)
{
@@ -419,6 +440,11 @@ static int collectargs(char **argv, int *flags)
notail(argv[i]);
*flags |= FLAGS_VERSION;
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;
case 'j': /* LuaJIT extension */
@@ -474,6 +500,10 @@ static int runargs(lua_State *L, char **argv, int argn)
return 1;
break;
}
+ case 't': { /* Tarantool's fork extension. */
+ const char *cmd = argv[i] + 2;
+ return dotoolcmd(L, cmd) != LUA_OK;
+ }
case 'O': /* LuaJIT extension. */
if (dojitopt(L, argv[i] + 2))
return 1;
@@ -535,7 +565,7 @@ static int pmain(lua_State *L)
luaL_openlibs(L);
lua_gc(L, LUA_GCRESTART, -1);
- createargtable(L, argv, s->argc, argn);
+ createargtable(L, argv, s->argc, (flags & FLAGS_TOOL) ? argn - 1 : argn);
if (!(flags & FLAGS_NOENV)) {
s->status = handle_luainit(L);
diff --git a/test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua b/test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
new file mode 100644
index 00000000..8361781f
--- /dev/null
+++ b/test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
@@ -0,0 +1,58 @@
+local tap = require('tap')
+local test = tap.test('gh-5688-cli-for-memprof-parse'):skipcond({
+ ['Memprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and
+ jit.arch ~= 'x64',
+ ['Memprof is implemented for Linux only'] = jit.os ~= 'Linux',
+-- XXX: The patch is for LuaJIT only, and it doesn't
+-- work on Tarantool.
+-- luacheck: no global
+ ['No memprof CLI option'] = _TARANTOOL,
+})
+
+test:plan(3)
+
+jit.off()
+jit.flush()
+
+local table_new = require('table.new')
+local utils = require('utils')
+
+local TMP_BINFILE = utils.tools.profilename('memprofdata.tmp.bin')
+local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin')
+local EXECUTABLE = utils.exec.luacmd(arg)
+local TABLE_SIZE = 20
+
+local function default_payload()
+ local _ = table_new(TABLE_SIZE, 0)
+ _ = nil
+ collectgarbage()
+end
+
+local function generate_output(filename, payload)
+ -- Clean up all garbage to avoid pollution of free.
+ collectgarbage()
+
+ local res, err = misc.memprof.start(filename)
+ -- Should start succesfully.
+ assert(res, err)
+
+ payload()
+
+ res, err = misc.memprof.stop()
+ -- Should stop succesfully.
+ assert(res, err)
+end
+
+generate_output(TMP_BINFILE, default_payload)
+
+local errcode = os.execute(EXECUTABLE .. ' -tinvalid ' .. TMP_BINFILE)
+test:ok(errcode ~= 0, 'invalid tool flag')
+
+errcode = os.execute(EXECUTABLE .. ' -tm ' .. BAD_PATH)
+test:ok(errcode ~= 0, 'binfile does not exist')
+
+errcode = os.execute(EXECUTABLE .. ' -tm ' .. TMP_BINFILE)
+test:ok(errcode == 0, 'memprof binfile parsing')
+
+os.remove(TMP_BINFILE)
+os.exit(test:check() and 0 or 1)
--
2.39.2 (Apple Git-143)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-10 11:24 [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser Maksim Kokryashkin via Tarantool-patches
@ 2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
2023-07-14 12:41 ` Maxim Kokryashkin via Tarantool-patches
2023-07-15 11:35 ` Sergey Kaplun via Tarantool-patches
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-07-11 13:06 UTC (permalink / raw)
To: Maksim Kokryashkin, tarantool-patches, skaplun, m.kokryashkin
Hello, Max!
Thanks for the patch! See my comments.
Sergey
On 7/10/23 14:24, Maksim Kokryashkin wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
>
> It is really inconvenient to use a standalone shell script to parse
> memprof dump. That is why this commit introduces a CLI flag for tools
> to the LuaJIT, so now it is possible to parse memprof dump
> as simple as:
> ```
> luajit -tm memprof.bin
> ```
>
> Closes tarantool/tarantool#5688
> ---
> Changes in v7:
> - Fixed comments as per review by Sergey
>
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse
> PR: https://github.com/tarantool/tarantool/pull/8002
>
> src/luajit.c | 32 +++++++++-
> .../gh-5688-memprof-cli-flag.test.lua | 58 +++++++++++++++++++
> 2 files changed, 89 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
>
> diff --git a/src/luajit.c b/src/luajit.c
> index 1ca24301..d335f32b 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"
According to usage cmd after -t option is optional. However without cmd
luajit prints usage
that hints about incorrect usage:
[0] ~/sources/MRG/tarantool/third_party/luajit$ ./src/luajit -t
usage: ./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.
<snipped>
> +end
> +
> +generate_output(TMP_BINFILE, default_payload)
> +
> +local errcode = os.execute(EXECUTABLE .. ' -tinvalid ' .. TMP_BINFILE)
> +test:ok(errcode ~= 0, 'invalid tool flag')
> +
> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. BAD_PATH)
> +test:ok(errcode ~= 0, 'binfile does not exist')
> +
> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. TMP_BINFILE)
> +test:ok(errcode == 0, 'memprof binfile parsing')
Nit: probably it is worth adding checks for output.
> +
> +os.remove(TMP_BINFILE)
> +os.exit(test:check() and 0 or 1)
> --
> 2.39.2 (Apple Git-143)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
@ 2023-07-14 12:41 ` Maxim Kokryashkin via Tarantool-patches
2023-07-28 15:30 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 1 reply; 11+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-07-14 12:41 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: Maksim Kokryashkin, tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]
Hi! Thanks for the review.
>
>>Hello, Max!
>>
>>Thanks for the patch! See my comments.
>>
>>
>>Sergey
>>
>>On 7/10/23 14:24, Maksim Kokryashkin wrote:
>>> From: Maxim Kokryashkin < m.kokryashkin@tarantool.org >
>>>
>>> It is really inconvenient to use a standalone shell script to parse
>>> memprof dump. That is why this commit introduces a CLI flag for tools
>>> to the LuaJIT, so now it is possible to parse memprof dump
>>> as simple as:
>>> ```
>>> luajit -tm memprof.bin
>>> ```
>>>
>>> Closes tarantool/tarantool#5688
>>> ---
>>> Changes in v7:
>>> - Fixed comments as per review by Sergey
>>>
>>> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse
>>> PR: https://github.com/tarantool/tarantool/pull/8002
>>>
>>> src/luajit.c | 32 +++++++++-
>>> .../gh-5688-memprof-cli-flag.test.lua | 58 +++++++++++++++++++
>>> 2 files changed, 89 insertions(+), 1 deletion(-)
>>> create mode 100644 test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
>>>
>>> diff --git a/src/luajit.c b/src/luajit.c
>>> index 1ca24301..d335f32b 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"
>>
>>According to usage cmd after -t option is optional. However without cmd
>>luajit prints usage
>>
>>that hints about incorrect usage:
>>
>>
>>[0] ~/sources/MRG/tarantool/third_party/luajit$ ./src/luajit -t
>>usage: ./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.
>Fixed! Branch is force-pushed, here is the diff:
>=========================================================
>diff --git a/src/luajit.c b/src/luajit.c
>index d335f32b..c16138f0 100644
>--- a/src/luajit.c
>+++ b/src/luajit.c
>@@ -72,7 +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"
>+ " -t(cmd) Execute tool.\n"
> " -E Ignore environment variables.\n"
> " -- Stop handling options.\n"
> " - Execute stdin and stop handling options.\n", stderr);
>=========================================================
>>
>><snipped>
>>> +end
>>> +
>>> +generate_output(TMP_BINFILE, default_payload)
>>> +
>>> +local errcode = os.execute(EXECUTABLE .. ' -tinvalid ' .. TMP_BINFILE)
>>> +test:ok(errcode ~= 0, 'invalid tool flag')
>>> +
>>> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. BAD_PATH)
>>> +test:ok(errcode ~= 0, 'binfile does not exist')
>>> +
>>> +errcode = os.execute(EXECUTABLE .. ' -tm ' .. TMP_BINFILE)
>>> +test:ok(errcode == 0, 'memprof binfile parsing')
>>Nit: probably it is worth adding checks for output.
>>> +
>>> +os.remove(TMP_BINFILE)
>>> +os.exit(test:check() and 0 or 1)
>>> --
>>> 2.39.2 (Apple Git-143)
>>>
>--
>Best regards,
>Maxim Kokryashkin
>
[-- Attachment #2: Type: text/html, Size: 5361 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-14 12:41 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-07-28 15:30 ` Sergey Bronnikov via Tarantool-patches
2023-07-29 16:24 ` Igor Munkin via Tarantool-patches
0 siblings, 1 reply; 11+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-07-28 15:30 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: Maksim Kokryashkin, tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 4541 bytes --]
Max, thanks for the <s>cake</s> patches! LGTM
On 7/14/23 15:41, Maxim Kokryashkin wrote:
> Hi! Thanks for the review.
>
> Hello, Max!
>
> Thanks for the patch! See my comments.
>
>
> Sergey
>
> On 7/10/23 14:24, Maksim Kokryashkin wrote:
> > From: Maxim Kokryashkin <m.kokryashkin@tarantool.org
> </compose?To=m.kokryashkin@tarantool.org>>
> >
> > It is really inconvenient to use a standalone shell script
> to parse
> > memprof dump. That is why this commit introduces a CLI flag
> for tools
> > to the LuaJIT, so now it is possible to parse memprof dump
> > as simple as:
> > ```
> > luajit -tm memprof.bin
> > ```
> >
> > Closes tarantool/tarantool#5688
> > ---
> > Changes in v7:
> > - Fixed comments as per review by Sergey
> >
> > Branch:
> https://github.com/tarantool/luajit/tree/fckxorg/gh-5688-cli-for-memprof-parse
> > PR: https://github.com/tarantool/tarantool/pull/8002
> >
> > src/luajit.c | 32 +++++++++-
> > .../gh-5688-memprof-cli-flag.test.lua | 58 +++++++++++++++++++
> > 2 files changed, 89 insertions(+), 1 deletion(-)
> > create mode 100644
> test/tarantool-tests/gh-5688-memprof-cli-flag.test.lua
> >
> > diff --git a/src/luajit.c b/src/luajit.c
> > index 1ca24301..d335f32b 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"
>
> According to usage cmd after -t option is optional. However
> without cmd
> luajit prints usage
>
> that hints about incorrect usage:
>
>
> [0] ~/sources/MRG/tarantool/third_party/luajit$ ./src/luajit -t
> usage: ./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.
>
> Fixed! Branch is force-pushed, here is the diff:
> =========================================================
> diff --git a/src/luajit.c b/src/luajit.c
> index d335f32b..c16138f0 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c
> @@ -72,7 +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"
> + " -t(cmd) Execute tool.\n"
> " -E Ignore environment variables.\n"
> " -- Stop handling options.\n"
> " - Execute stdin and stop handling options.\n", stderr);
> =========================================================
>
>
> <snipped>
> > +end
> > +
> > +generate_output(TMP_BINFILE, default_payload)
> > +
> > +local errcode = os.execute(EXECUTABLE .. ' -tinvalid ' ..
> TMP_BINFILE)
> > +test:ok(errcode ~= 0, 'invalid tool flag')
> > +
> > +errcode = os.execute(EXECUTABLE .. ' -tm ' .. BAD_PATH)
> > +test:ok(errcode ~= 0, 'binfile does not exist')
> > +
> > +errcode = os.execute(EXECUTABLE .. ' -tm ' .. TMP_BINFILE)
> > +test:ok(errcode == 0, 'memprof binfile parsing')
> Nit: probably it is worth adding checks for output.
> > +
> > +os.remove(TMP_BINFILE)
> > +os.exit(test:check() and 0 or 1)
> > --
> > 2.39.2 (Apple Git-143)
> >
>
> --
> Best regards,
> Maxim Kokryashkin
>
[-- Attachment #2: Type: text/html, Size: 8348 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-10 11:24 [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser Maksim Kokryashkin via Tarantool-patches
2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
@ 2023-07-15 11:35 ` Sergey Kaplun via Tarantool-patches
2023-08-01 15:45 ` Igor Munkin via Tarantool-patches
2023-08-02 7:23 ` Igor Munkin via Tarantool-patches
3 siblings, 0 replies; 11+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-07-15 11:35 UTC (permalink / raw)
To: Maksim Kokryashkin; +Cc: tarantool-patches
Hi, Maxim!
Thanks for the fixes!
LGTM!
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-10 11:24 [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser Maksim Kokryashkin via Tarantool-patches
2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
2023-07-15 11:35 ` Sergey Kaplun via Tarantool-patches
@ 2023-08-01 15:45 ` Igor Munkin via Tarantool-patches
2023-08-02 7:23 ` Igor Munkin via Tarantool-patches
3 siblings, 0 replies; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-08-01 15:45 UTC (permalink / raw)
To: Maksim Kokryashkin; +Cc: tarantool-patches
Max,
Thanks for the changes! It would be nice to drop luajit-parse-memprof.in
with the respective CMake parts, since I see no purpose for this now.
--
Best regards,
IM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-07-10 11:24 [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser Maksim Kokryashkin via Tarantool-patches
` (2 preceding siblings ...)
2023-08-01 15:45 ` Igor Munkin via Tarantool-patches
@ 2023-08-02 7:23 ` Igor Munkin via Tarantool-patches
2023-08-02 7:59 ` Maxim Kokryashkin via Tarantool-patches
3 siblings, 1 reply; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-08-02 7:23 UTC (permalink / raw)
To: Maksim Kokryashkin; +Cc: tarantool-patches
Max,
We've discussed with Sergey K. how to run -tm with --leak-only today.
Fortunately, it works fine in LuaJIT, however, I'm afraid such flag
handling in incompatible in Tarantool. Hence, I suggest to glue this
flag with comma to -tm, like jdump does[1]. Thoughts?
[1]: https://github.com/tarantool/luajit/blob/tarantool/master/src/jit/dump.lua#L18
--
Best regards,
IM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-08-02 7:23 ` Igor Munkin via Tarantool-patches
@ 2023-08-02 7:59 ` Maxim Kokryashkin via Tarantool-patches
2023-08-02 8:12 ` Igor Munkin via Tarantool-patches
0 siblings, 1 reply; 11+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-08-02 7:59 UTC (permalink / raw)
To: Igor Munkin; +Cc: Maksim Kokryashkin, tarantool-patches
Hi, Igor!
Well, I see no obstacles with flag handling in Tarantool.
For instance, the -j flag proxy can do exactly what you want.
| review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
| ▶ ./src/tarantool -j on -e 'print(jit.status())'
| true fold cse dce fwd dse narrow loop abc sink fuse
|
| review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
| ▶ ./src/tarantool -j off -e 'print(jit.status())'
| false fold cse dce fwd dse narrow loop abc sink fuse
Tarantool's flag handling is not pleasant to say the least,
but we should try to do the same as with -j.
Best regards,
Maxim Kokryashkin
On Wed, Aug 02, 2023 at 07:23:03AM +0000, Igor Munkin wrote:
> Max,
>
> We've discussed with Sergey K. how to run -tm with --leak-only today.
> Fortunately, it works fine in LuaJIT, however, I'm afraid such flag
> handling in incompatible in Tarantool. Hence, I suggest to glue this
> flag with comma to -tm, like jdump does[1]. Thoughts?
>
> [1]: https://github.com/tarantool/luajit/blob/tarantool/master/src/jit/dump.lua#L18
>
> --
> Best regards,
> IM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-08-02 7:59 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-08-02 8:12 ` Igor Munkin via Tarantool-patches
2023-08-02 8:32 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 1 reply; 11+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-08-02 8:12 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: Maksim Kokryashkin, tarantool-patches
Max,
On 02.08.23, Maxim Kokryashkin wrote:
> Hi, Igor!
>
> Well, I see no obstacles with flag handling in Tarantool.
> For instance, the -j flag proxy can do exactly what you want.
>
> | review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
> | ▶ ./src/tarantool -j on -e 'print(jit.status())'
> | true fold cse dce fwd dse narrow loop abc sink fuse
> |
> | review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
> | ▶ ./src/tarantool -j off -e 'print(jit.status())'
> | false fold cse dce fwd dse narrow loop abc sink fuse
>
My bad, I'm concerted rather about this difference and its effect in
Tarantool flags processing:
| $ ./luajit -bl -e 'print("qq")'
| -- BYTECODE -- "print("qq")":0-1
| 0001 GGET 0 0 ; "print"
| 0002 KSTR 1 1 ; "qq"
| 0003 CALL 0 1 2
| 0004 RET0 0 1
|
| $ cd ../src/tools
| $ ../src/luajit -tm -e 'print("qq")'
| luajit-parse-memprof.lua: ERROR: unrecognized option `-e'. Try `--help'.
|
In other words, I wonder
* whether flags in Tarantool are position independent (i.e. the result
of -j + -e combination equals to -e + -j);
* how --leak-only will be handled in this case.
BTW, I have a thought regarding introducing kinda "mode" for memprof, so
we can use it like -tm=leak-only (as an alternative to the original
proposal in the first message).
Besides, I see no tests for --leak-only (have no idea, why everyone
missed this on review). Could you add them?
> Tarantool's flag handling is not pleasant to say the least,
> but we should try to do the same as with -j.
>
> Best regards,
> Maxim Kokryashkin
>
>
> On Wed, Aug 02, 2023 at 07:23:03AM +0000, Igor Munkin wrote:
> > Max,
> >
> > We've discussed with Sergey K. how to run -tm with --leak-only today.
> > Fortunately, it works fine in LuaJIT, however, I'm afraid such flag
> > handling in incompatible in Tarantool. Hence, I suggest to glue this
> > flag with comma to -tm, like jdump does[1]. Thoughts?
> >
> > [1]: https://github.com/tarantool/luajit/blob/tarantool/master/src/jit/dump.lua#L18
> >
> > --
> > Best regards,
> > IM
--
Best regards,
IM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser
2023-08-02 8:12 ` Igor Munkin via Tarantool-patches
@ 2023-08-02 8:32 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 0 replies; 11+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-08-02 8:32 UTC (permalink / raw)
To: Igor Munkin; +Cc: Maksim Kokryashkin, tarantool-patches
Igor,
On Wed, Aug 02, 2023 at 08:12:50AM +0000, Igor Munkin wrote:
> Max,
>
> On 02.08.23, Maxim Kokryashkin wrote:
> > Hi, Igor!
> >
> > Well, I see no obstacles with flag handling in Tarantool.
> > For instance, the -j flag proxy can do exactly what you want.
> >
> > | review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
> > | ▶ ./src/tarantool -j on -e 'print(jit.status())'
> > | true fold cse dce fwd dse narrow loop abc sink fuse
> > |
> > | review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d16h
> > | ▶ ./src/tarantool -j off -e 'print(jit.status())'
> > | false fold cse dce fwd dse narrow loop abc sink fuse
> >
>
> My bad, I'm concerted rather about this difference and its effect in
> Tarantool flags processing:
> | $ ./luajit -bl -e 'print("qq")'
> | -- BYTECODE -- "print("qq")":0-1
> | 0001 GGET 0 0 ; "print"
> | 0002 KSTR 1 1 ; "qq"
> | 0003 CALL 0 1 2
> | 0004 RET0 0 1
> |
> | $ cd ../src/tools
> | $ ../src/luajit -tm -e 'print("qq")'
> | luajit-parse-memprof.lua: ERROR: unrecognized option `-e'. Try `--help'.
> |
>
> In other words, I wonder
> * whether flags in Tarantool are position independent (i.e. the result
> of -j + -e combination equals to -e + -j);
| review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d17h
| ▶ ./src/tarantool -e 'print(jit.status())' -jon
| false fold cse dce fwd dse narrow loop abc sink fuse
|
| review/tarantool/build fckxorg/gh-5688-cli-for-memprof-parse ✔ 239d17h
| ▶ ./src/tarantool -jon -e 'print(jit.status())'
| true fold cse dce fwd dse narrow loop abc sink fuse
So no, they are not position-independent.
> * how --leak-only will be handled in this case.
I can just pass further args after the -tm right to the memprof parser module then.
>
> BTW, I have a thought regarding introducing kinda "mode" for memprof, so
> we can use it like -tm=leak-only (as an alternative to the original
> proposal in the first message).
>
> Besides, I see no tests for --leak-only (have no idea, why everyone
> missed this on review). Could you add them?
Yep, sure.
>
> > Tarantool's flag handling is not pleasant to say the least,
> > but we should try to do the same as with -j.
> >
> > Best regards,
> > Maxim Kokryashkin
> >
> >
> > On Wed, Aug 02, 2023 at 07:23:03AM +0000, Igor Munkin wrote:
> > > Max,
> > >
> > > We've discussed with Sergey K. how to run -tm with --leak-only today.
> > > Fortunately, it works fine in LuaJIT, however, I'm afraid such flag
> > > handling in incompatible in Tarantool. Hence, I suggest to glue this
> > > flag with comma to -tm, like jdump does[1]. Thoughts?
> > >
> > > [1]: https://github.com/tarantool/luajit/blob/tarantool/master/src/jit/dump.lua#L18
> > >
> > > --
> > > Best regards,
> > > IM
>
> --
> Best regards,
> IM
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-08-02 8:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-10 11:24 [Tarantool-patches] [PATCH luajit v7] memprof: introduce cli flag to run dump parser Maksim Kokryashkin via Tarantool-patches
2023-07-11 13:06 ` Sergey Bronnikov via Tarantool-patches
2023-07-14 12:41 ` Maxim Kokryashkin via Tarantool-patches
2023-07-28 15:30 ` Sergey Bronnikov via Tarantool-patches
2023-07-29 16:24 ` Igor Munkin via Tarantool-patches
2023-07-15 11:35 ` Sergey Kaplun via Tarantool-patches
2023-08-01 15:45 ` Igor Munkin via Tarantool-patches
2023-08-02 7:23 ` Igor Munkin via Tarantool-patches
2023-08-02 7:59 ` Maxim Kokryashkin via Tarantool-patches
2023-08-02 8:12 ` Igor Munkin via Tarantool-patches
2023-08-02 8:32 ` Maxim Kokryashkin via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox