Hi, Sergey,

See my answers below.

Sergey

On 3/12/26 20:59, Sergey Kaplun wrote:

Hi, Sergey!
See my answers below.

On 12.03.26, Sergey Bronnikov wrote:

Hi, Sergey,

thanks for review! See my comments.

Sergey

On 3/12/26 15:00, Sergey Kaplun via Tarantool-patches wrote:

Hi, Sergey!
Thanks for the patch!
Please consider my comments below.

On 22.01.26, Sergey Bronnikov wrote:

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

Let's rename it to `available` instead. The `enabled` may be interpreted
as `is_running`, and confuse the user then.

I propose using `is_available` instead.

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.

ok, let's rename to "available".

diff --git a/src/lib_misc.c b/src/lib_misc.c
index 6b2278c1..3463445c 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -479,10 +479,10 @@ LUALIB_API int luaopen_misc(struct lua_State *L)
#if !LJ_TARGET_WINDOWS
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
lua_pushboolean(L, LJ_HASMEMPROF);
- lua_setfield(L, -2, "enabled");
+ lua_setfield(L, -2, "available");
LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
lua_pushboolean(L, LJ_HASSYSPROF);
- lua_setfield(L, -2, "enabled");
+ lua_setfield(L, -2, "available");
#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 f867cfc6..2d2d43c1 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-disabled.test.lua
@@ -19,7 +19,8 @@ test:ok(err:match('profiler is disabled'),
'error on stop when memprof is disabled')
test:ok(type(errno) == 'number', 'errno on start when memprof is disabled')

-test:ok(type(misc.memprof.enabled) == 'boolean', 'misc.memprof.enabled exists')
-test:ok(misc.memprof.enabled == false, 'misc.memprof.enabled is false')
+test:ok(type(misc.memprof.available) == 'boolean',
+ 'misc.memprof.available exists')
+test:ok(misc.memprof.available == false, 'misc.memprof.available is false')

test:done(true)
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
index 44ba8b08..a7aad80e 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
@@ -270,7 +270,8 @@ test:test("jit-output", function(subtest)
jit.opt.start(unpack(jit_opt_default))
end)

-test:ok(type(misc.memprof.enabled) == 'boolean', 'misc.memprof.enabled exists')
-test:ok(misc.memprof.enabled == true, 'misc.memprof.enabled is true')
+test:ok(type(misc.memprof.available) == 'boolean',
+ 'misc.memprof.available exists')
+test:ok(misc.memprof.available == true, 'misc.memprof.available is true')

test:done(true)
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 c023d8f1..5b9cc269 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi-disabled.test.lua
@@ -26,7 +26,8 @@ test:ok(err:match('profiler is disabled'),
'error on stop when sysprof is disabled')
test:ok(type(errno) == 'number', 'errno on start when sysprof is disabled')

-test:ok(type(misc.sysprof.enabled) == 'boolean', 'misc.sysprof.enabled exists')
-test:ok(misc.sysprof.enabled == false, 'misc.sysprof.enabled is false')
+test:ok(type(misc.sysprof.available) == 'boolean',
+ 'misc.sysprof.available exists')
+test:ok(misc.sysprof.available == false, 'misc.sysprof.available is false')

test:done(true)
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 41ed43e0..86115b62 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -228,7 +228,8 @@ check_mode("C", 100)

os.remove(TMP_BINFILE)

-test:ok(type(misc.sysprof.enabled) == 'boolean', 'misc.sysprof.enabled exists')
-test:ok(misc.sysprof.enabled == true, 'misc.sysprof.enabled is true')
+test:ok(type(misc.sysprof.available) == 'boolean',
+ 'misc.sysprof.available exists')
+test:ok(misc.sysprof.available == true, 'misc.sysprof.available is true')

test:done(true)

available on platforms supported by profilers (Windows is not
supported).

Closes tarantool/tarantool#12215

Minor: Should be Resolves, since it is closed when we bump LuaJIT in the
Tarantool repository.

Ok, updated.

---
  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");

Is it possible to use standard `LJLIB_PUSH() LJLIB_SET()` machinery
instead?

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?

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

Updated:

diff --git a/src/lib_misc.c b/src/lib_misc.c
index 3463445c..62886242 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -389,6 +389,8 @@ LJLIB_CF(misc_sysprof_report)
#endif /* !LJ_HASSYSPROF */
}

+LJLIB_PUSH(top-2) LJLIB_SET(available)
+
/* ----- misc.memprof module ---------------------------------------------- */

#define LJLIB_MODULE_misc_memprof
@@ -459,6 +461,8 @@ LJLIB_CF(misc_memprof_stop)
}
#endif /* !LJ_TARGET_WINDOWS */

+LJLIB_PUSH(top-2) LJLIB_SET(available)
+
#include "lj_libdef.h"

/* ------------------------------------------------------------------------ */
@@ -477,12 +481,10 @@ 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, "available");
- LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
+ LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof);
lua_pushboolean(L, LJ_HASSYSPROF);
- lua_setfield(L, -2, "available");
+ LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof);
#endif /* !LJ_TARGET_WINDOWS */
return 1;
}

  #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 @@test:ok(err:match('profiler is disabled'),
          'error on stop when memprof is disabled')
  test:ok(type(errno) == 'number', 'errno on start when memprof is disabled')
  
+test:ok(type(misc.memprof.enabled) == 'boolean', 'misc.memprof.enabled exists')

I suppose that
|test:is(misc.memprof.available, false, 'misc.memprof.enabled correct')
is enough.

Same for other tests below.

if "misc.memprof" is not a table the error will be "attempt to index field"

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.

The same sense as with checking both boolean value for pcall and an error message, see for example

your patch in [1]:

+test:ok(not result, 'correct status for recursive call')
+test:like(errmsg, 'stack overflow', 'correct error message for recursive call')

In aforementioned patch you can check only message, but you check both values.

1. https://lists.tarantool.org/tarantool-patches/20260316104853.23901-1-skaplun@tarantool.org/T/#u


Also, `is()` check is more verbose in case of failure -- it prints got
and expected values.

It's much easier for me to read that a type check failed than to figure out why the value is incorrect.

The TAP checks produce completely unreadable message; I never use them.

+test:ok(misc.memprof.enabled == false, 'misc.memprof.enabled is false')
+
  test:done(true)
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

<snipped>

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

<snipped>

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

<snipped>

-- 
2.43.0