<!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 valuable comments!</p>
<p>Sergey<br>
</p>
<div class="moz-cite-prefix">On 10.02.2025 17:52, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:Z6oSpEu4oHels0u5@root">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
On 04.02.25, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">The function `misc.sysprof.stop()` reports that profiler is
already running:
| $ ./build/src/luajit -e 'print(misc.sysprof.stop())'
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Minor: You may omit a ./build here, but this is a matter of taste, so
feel free to ignore.</pre>
</blockquote>
Removed 'build'.<br>
<blockquote type="cite" cite="mid:Z6oSpEu4oHels0u5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">| nil profiler is running already 22
The patch fixes that.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Unfortunatelly not:
| $ git --no-pager log --oneline -n1 --no-decorate && ./src/luajit -e 'print(misc.sysprof.stop())'
| b49c5fac sysprof: fix a message with stop without run
| nil profiler is running already 22
See my comments below.
Please add the follow-up to the tarantool/tarantool#781.</pre>
</blockquote>
Added.<br>
<blockquote type="cite" cite="mid:Z6oSpEu4oHels0u5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">---
src/lj_sysprof.c | 2 +-
test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 88c7a41b..b76f503c 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -493,7 +493,7 @@ int lj_sysprof_stop(lua_State *L)
global_State *g = sp->g;
struct lj_wbuf *out = &sp->out;
- if (SPS_IDLE == sp->state)
+ if (SPS_PROFILE != sp->state)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This check is correct. It checks that the profiler has no error or is
not running (i.e., that it is stopped). With the new check, a case with
an error during profiling will be masked.
It looks like we should add this case in the `misc.sysprof.stop()` (it
uses `sysprof_error()` which handles start cases).</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:Z6oSpEu4oHels0u5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return PROFILE_ERRRUN;
else if (G(L) != g)
return PROFILE_ERRUSE;
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 68a4b72f..91f9ca5c 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -10,7 +10,7 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
["Disabled due to #10803"] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
})
-test:plan(33)
+test:plan(34)
jit.off()
-- XXX: Run JIT tuning functions in a safe frame to avoid errors
@@ -98,7 +98,8 @@ assert(res, err)
-- Not running.
res, err, errno = misc.sysprof.stop()
-test:ok(res == nil and err, "res and error with not running")
+test:is(res, nil, "res with not running")
+test:ok(err:match("profiler is running already"), "error with not running")
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I suppose there is bad copy-pasting here. It should be: 'profiler is not
running'. This is why the test passes.</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:Z6oSpEu4oHels0u5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> test:ok(type(errno) == "number", "errno with not running")
-- Bad path.
--
2.34.1
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>