<!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>See my answers below.</p>
<p>Sergey</p>
<div class="moz-cite-prefix">On 3/12/26 20:59, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
See my answers below.
On 12.03.26, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">Hi, Sergey,
thanks for review! See my comments.
Sergey
On 3/12/26 15:00, Sergey Kaplun via Tarantool-patches wrote:
</pre>
<blockquote type="cite">
<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>
<pre wrap="" class="moz-quote-pre">I propose using `is_available` instead.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
The Lua API has 0 functions in the snake_case. Even `funcinfo`,
`traceexitstub` from lib_jit.c have this naming style. Lets be
consistent with it, especially, since `is_` prefix doesn't change the
semantics.
</pre>
</blockquote>
<p>ok, let's rename to "available".</p>
<p>diff --git a/src/lib_misc.c b/src/lib_misc.c<br>
index 6b2278c1..3463445c 100644<br>
--- a/src/lib_misc.c<br>
+++ b/src/lib_misc.c<br>
@@ -479,10 +479,10 @@ LUALIB_API int luaopen_misc(struct lua_State
*L)<br>
#if !LJ_TARGET_WINDOWS<br>
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);<br>
lua_pushboolean(L, LJ_HASMEMPROF);<br>
- lua_setfield(L, -2, "enabled");<br>
+ lua_setfield(L, -2, "available");<br>
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);<br>
lua_pushboolean(L, LJ_HASSYSPROF);<br>
- lua_setfield(L, -2, "enabled");<br>
+ lua_setfield(L, -2, "available");<br>
#endif /* !LJ_TARGET_WINDOWS */<br>
return 1;<br>
}<br>
diff --git
a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua<br>
index f867cfc6..2d2d43c1 100644<br>
---
a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua<br>
+++
b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua<br>
@@ -19,7 +19,8 @@ <a class="moz-txt-link-freetext" href="test:ok(err:match('profiler">test:ok(err:match('profiler</a> is disabled'),<br>
'error on stop when memprof is disabled')<br>
<a class="moz-txt-link-freetext" href="test:ok(type(errno)">test:ok(type(errno)</a> == 'number', 'errno on start when memprof is
disabled')<br>
<br>
-test:ok(type(misc.memprof.enabled) == 'boolean',
'misc.memprof.enabled exists')<br>
-test:ok(misc.memprof.enabled == false, 'misc.memprof.enabled is
false')<br>
+test:ok(type(misc.memprof.available) == 'boolean',<br>
+ 'misc.memprof.available exists')<br>
+test:ok(misc.memprof.available == false, 'misc.memprof.available
is false')<br>
<br>
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a><br>
diff --git
a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua<br>
index 44ba8b08..a7aad80e 100644<br>
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua<br>
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua<br>
@@ -270,7 +270,8 @@ <a class="moz-txt-link-freetext" href="test:test(">test:test(</a>"jit-output", function(subtest)<br>
jit.opt.start(unpack(jit_opt_default))<br>
end)<br>
<br>
-test:ok(type(misc.memprof.enabled) == 'boolean',
'misc.memprof.enabled exists')<br>
-test:ok(misc.memprof.enabled == true, 'misc.memprof.enabled is
true')<br>
+test:ok(type(misc.memprof.available) == 'boolean',<br>
+ 'misc.memprof.available exists')<br>
+test:ok(misc.memprof.available == true, 'misc.memprof.available
is true')<br>
<br>
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a><br>
diff --git
a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua<br>
index c023d8f1..5b9cc269 100644<br>
---
a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua<br>
+++
b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua<br>
@@ -26,7 +26,8 @@ <a class="moz-txt-link-freetext" href="test:ok(err:match('profiler">test:ok(err:match('profiler</a> is disabled'),<br>
'error on stop when sysprof is disabled')<br>
<a class="moz-txt-link-freetext" href="test:ok(type(errno)">test:ok(type(errno)</a> == 'number', 'errno on start when sysprof is
disabled')<br>
<br>
-test:ok(type(misc.sysprof.enabled) == 'boolean',
'misc.sysprof.enabled exists')<br>
-test:ok(misc.sysprof.enabled == false, 'misc.sysprof.enabled is
false')<br>
+test:ok(type(misc.sysprof.available) == 'boolean',<br>
+ 'misc.sysprof.available exists')<br>
+test:ok(misc.sysprof.available == false, 'misc.sysprof.available
is false')<br>
<br>
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a><br>
diff --git
a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua<br>
index 41ed43e0..86115b62 100644<br>
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua<br>
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua<br>
@@ -228,7 +228,8 @@ check_mode("C", 100)<br>
<br>
os.remove(TMP_BINFILE)<br>
<br>
-test:ok(type(misc.sysprof.enabled) == 'boolean',
'misc.sysprof.enabled exists')<br>
-test:ok(misc.sysprof.enabled == true, 'misc.sysprof.enabled is
true')<br>
+test:ok(type(misc.sysprof.available) == 'boolean',<br>
+ 'misc.sysprof.available exists')<br>
+test:ok(misc.sysprof.available == true, 'misc.sysprof.available
is true')<br>
<br>
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a><br>
<br>
</p>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<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>
<pre wrap="" class="moz-quote-pre">Ok, updated.
</pre>
<blockquote type="cite">
<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>
<pre wrap="" class="moz-quote-pre">
I didn't get what is a macros `LJLIB_SET`.
Also, what are the benefits with using mentioned macros instead more
standard Lua API functions?
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
It is more consistent with the rest of the code base.
Also, it helps to avoid table rehasing on library initialization.
You may see details in buildvm_lib.c
</pre>
</blockquote>
<p>Updated:</p>
<p>diff --git a/src/lib_misc.c b/src/lib_misc.c<br>
index 3463445c..62886242 100644<br>
--- a/src/lib_misc.c<br>
+++ b/src/lib_misc.c<br>
@@ -389,6 +389,8 @@ LJLIB_CF(misc_sysprof_report)<br>
#endif /* !LJ_HASSYSPROF */<br>
}<br>
<br>
+LJLIB_PUSH(top-2) LJLIB_SET(available)<br>
+<br>
/* ----- misc.memprof module
---------------------------------------------- */<br>
<br>
#define LJLIB_MODULE_misc_memprof<br>
@@ -459,6 +461,8 @@ LJLIB_CF(misc_memprof_stop)<br>
}<br>
#endif /* !LJ_TARGET_WINDOWS */<br>
<br>
+LJLIB_PUSH(top-2) LJLIB_SET(available)<br>
+<br>
#include "lj_libdef.h"<br>
<br>
/*
------------------------------------------------------------------------
*/<br>
@@ -477,12 +481,10 @@ LUALIB_API int luaopen_misc(struct lua_State
*L)<br>
<br>
LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc);<br>
#if !LJ_TARGET_WINDOWS<br>
- LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);<br>
lua_pushboolean(L, LJ_HASMEMPROF);<br>
- lua_setfield(L, -2, "available");<br>
- LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);<br>
+ LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);<br>
lua_pushboolean(L, LJ_HASSYSPROF);<br>
- lua_setfield(L, -2, "available");<br>
+ LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);<br>
#endif /* !LJ_TARGET_WINDOWS */<br>
return 1;<br>
}<br>
<br>
</p>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<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>
<pre wrap="" class="moz-quote-pre">if "misc.memprof" is not a table the error will be "attempt to index field"
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Don't get the point here:
1) We will see this error much earlier in that case.
2) We will see with your approach as well.
I don't get the reason for the `type()` call if the check below will
fail with a different type as well (since Lua checks types first). I see
no reason in double-checking that means nothing.</pre>
</blockquote>
<br>
<p>The same sense as with checking both boolean value for pcall and
an error message, see for example</p>
<p>your patch in [1]:</p>
<p>+test:ok(not result, 'correct status for recursive call')<br>
+test:like(errmsg, 'stack overflow', 'correct error message for
recursive call')</p>
<p>In aforementioned patch you can check only message, but you check
both values.</p>
<p>1.
<a class="moz-txt-link-freetext" href="https://lists.tarantool.org/tarantool-patches/20260316104853.23901-1-skaplun@tarantool.org/T/#u">https://lists.tarantool.org/tarantool-patches/20260316104853.23901-1-skaplun@tarantool.org/T/#u</a></p>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<pre wrap="" class="moz-quote-pre"></pre>
</blockquote>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<pre wrap="" class="moz-quote-pre">
Also, `is()` check is more verbose in case of failure -- it prints got
and expected values.</pre>
</blockquote>
<p><span class="HwtZe" lang="en"><span class="jCAhz ChMk0b"><span
class="ryNqvb">It's much easier for me to read that a type
check failed than to figure out why the value is incorrect.</span></span></span></p>
<p><span class="HwtZe" lang="en"><span class="jCAhz ChMk0b"><span
class="ryNqvb">The TAP checks produce completely unreadable
message; I never use them.</span></span></span></p>
<blockquote type="cite" cite="mid:abL-9P1aepENHzMl@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<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>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>