<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p>thanks for review! See my comments.</p>
<p>Sergey</p>
<div class="moz-cite-prefix">On 3/12/26 15:00, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:abKq8Jd2tHow9Efs@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
On 22.01.26, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">The patch introduce flags in module "misc" with support status
for sysprof and memprof: `misc.sysprof.enabled` and
`misc.memprof.enabled`. Both flags are boolean and always
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Let's rename it to `available` instead. The `enabled` may be interpreted
as `is_running`, and confuse the user then.</pre>
</blockquote>
I propose using `is_available` instead.
<blockquote type="cite" cite="mid:abKq8Jd2tHow9Efs@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">available on platforms supported by profilers (Windows is not
supported).
Closes tarantool/tarantool#12215
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Minor: Should be Resolves, since it is closed when we bump LuaJIT in the
Tarantool repository.</pre>
</blockquote>
Ok, updated.
<blockquote type="cite" cite="mid:abKq8Jd2tHow9Efs@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">---
src/lib_misc.c | 4 ++++
.../profilers/misclib-memprof-lapi-disabled.test.lua | 5 ++++-
test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua | 5 ++++-
.../profilers/misclib-sysprof-lapi-disabled.test.lua | 5 ++++-
test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 5 ++++-
5 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/lib_misc.c b/src/lib_misc.c
index 034ff878..6b2278c1 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -478,7 +478,11 @@ LUALIB_API int luaopen_misc(struct lua_State *L)
LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc);
#if !LJ_TARGET_WINDOWS
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
+ lua_pushboolean(L, LJ_HASMEMPROF);
+ lua_setfield(L, -2, "enabled");
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
+ lua_pushboolean(L, LJ_HASSYSPROF);
+ lua_setfield(L, -2, "enabled");
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Is it possible to use standard `LJLIB_PUSH() LJLIB_SET()` machinery
instead?</pre>
</blockquote>
<p>I didn't get what is a macros `LJLIB_SET`.</p>
<p>Also, <span class="HwtZe" lang="en"><span class="jCAhz"><span
class="ryNqvb">what are the benefits with using mentioned
macros instead more standard Lua API functions?</span></span></span></p>
<p><br>
</p>
<blockquote type="cite" cite="mid:abKq8Jd2tHow9Efs@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre"> #endif /* !LJ_TARGET_WINDOWS */
return 1;
}
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
index de0aa136..f867cfc6 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
@@ -3,7 +3,7 @@ local test = tap.test('misclib-memprof-lapi-disabled'):skipcond({
['Memprof is enabled'] = not os.getenv('LUAJIT_DISABLE_MEMPROF'),
})
-test:plan(6)
+test:plan(8)
-- Attempt to start memprof when it is disabled.
local res, err, errno = misc.memprof.start()
@@ -19,4 +19,7 @@ <a class="moz-txt-link-freetext" href="test:ok(err:match('profiler">test:ok(err:match('profiler</a> is disabled'),
'error on stop when memprof is disabled')
<a class="moz-txt-link-freetext" href="test:ok(type(errno)">test:ok(type(errno)</a> == 'number', 'errno on start when memprof is disabled')
+test:ok(type(misc.memprof.enabled) == 'boolean', 'misc.memprof.enabled exists')
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
I suppose that
| <a class="moz-txt-link-freetext" href="test:is(misc.memprof.available">test:is(misc.memprof.available</a>, false, 'misc.memprof.enabled correct')
is enough.
Same for other tests below.</pre>
</blockquote>
if "misc.memprof" is not a table the error will be "attempt to index
field"
<blockquote type="cite" cite="mid:abKq8Jd2tHow9Efs@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+test:ok(misc.memprof.enabled == false, 'misc.memprof.enabled is false')
+
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a>
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
index cd675864..44ba8b08 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
index 2a9ce796..c023d8f1 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 701d58e4..41ed43e0 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">--
2.43.0
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>