<!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 the comments! See my replies below.</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 6/4/25 11:35, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:aEAFRYmJ2uad8zU3@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the patch!
LGTM, with minor comments below.
On 16.05.25, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">It is not allowed to call a function `sysprof.report()` without
stopping profiler. However, sometimes it may be useful to analyze
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Typo: s/profiler/the profiler/
</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:aEAFRYmJ2uad8zU3@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">numbers provided by the report without stopping the profiler. The
patch removes the appropriate condition and allows reporting
without stopping.
Resolves tarantool/tarantool#11229
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/ligurio/gh-11229-misc.sysprof.report">https://github.com/tarantool/luajit/tree/ligurio/gh-11229-misc.sysprof.report</a>
Issue: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/11229">https://github.com/tarantool/tarantool/issues/11229</a>
src/lj_sysprof.c | 2 --
.../profilers/misclib-sysprof-lapi.test.lua | 18 ++++++++++++++++--
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 88c7a41b..cf6161a5 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
</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 f316c390..3e774a53 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">@@ -166,13 +166,27 @@ <a class="moz-txt-link-freetext" href="test:is(err">test:is(err</a>, nil, "no error with good interval 1")
<a class="moz-txt-link-freetext" href="test:is(errno">test:is(errno</a>, nil, "no errno with good interval 1")
misc.sysprof.stop()
+-- Intermediate sysprof.report().
+res, err, errno = misc.sysprof.start{
+ mode = "C",
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
I suppose we don't need to run the profiler in "C" mode. Let's use "D"
here.</pre>
</blockquote>
<p>Fixed.</p>
<p>---
a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua<br>
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua<br>
@@ -168,7 +168,7 @@ misc.sysprof.stop()<br>
<br>
-- Intermediate sysprof.report().<br>
res, err, errno = misc.sysprof.start{<br>
- mode = "C",<br>
+ mode = "D",<br>
interval = 1,<br>
path = "/dev/null",<br>
}<br>
<br>
</p>
<blockquote type="cite" cite="mid:aEAFRYmJ2uad8zU3@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ interval = 1,
+ path = "/dev/null",
+}
+test:is(res, true, "res is correct")
+test:is(err, nil, "no error")
+test:is(errno, nil, "no errno")
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
I suppose that 2 last checks are excess. The first one is enough to be
sure that the profiler is started. Also, we may use `assert()` here
instead of `<a class="moz-txt-link-freetext" href="test:is()">test:is()</a>` check, since we don't want to _test_ the starting of
the profiler only to _assert_ that the sysprof has been started.
</pre>
</blockquote>
last two checks were removed and <a class="moz-txt-link-freetext" href="test:is()">test:is()</a> replaced with assert()<br>
<blockquote type="cite" cite="mid:aEAFRYmJ2uad8zU3@root">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+
+local report = misc.sysprof.report()
+test:ok(report.samples == 0, "total number of samples is non-zero")
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
I'm not sure that this will always be true (for example, in coverage
workflow). I suggest increasing the interval dramatically to avoid false
positives here.
</pre>
</blockquote>
Updated.<br>
<blockquote type="cite" cite="mid:aEAFRYmJ2uad8zU3@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+misc.sysprof.stop()
+
</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>