<HTML><BODY><div>Hi, Igor!</div><div>Thanks for the patch!</div><div>LGTM, except for the single nit Sergey has already mentioned.</div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16699273741007865851_BODY">To test whether <jit_snap_restore> metric is counted right for side<br>exits <minstitch> JIT engine parameter is tighted to prevent side trace<br>compilation (considering <math.fmod> being used for the corresponding<br>branch of execution, the side trace will try to compile with stitching).<br>However, if test is run on the platform configured with the option<br>LUAJIT_ENABLE_CHECKHOOK enabled, the resulting trace exceeds the<br>hardcoded <minstitch> limit and the metric is miscounted due to this new<br>trace. To resolve this issue <minstitch> limit is set to the maximum<br>amount of IRRef in the trace, preventing trace compilation for any<br>LuaJIT configuration.<br><br>Relates to tarantool/tarantool#7762<br><br>Signed-off-by: Igor Munkin <<a href="/compose?To=imun@tarantool.org">imun@tarantool.org</a>><br>---<br> test/tarantool-tests/misclib-getmetrics-capi.test.lua | 7 +++++--<br> test/tarantool-tests/misclib-getmetrics-lapi.test.lua | 7 +++++--<br> test/tarantool-tests/utils.lua | 9 +++++++++<br> 3 files changed, 19 insertions(+), 4 deletions(-)<br><br>diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua<br>index 0cbc6cae..42a9adf9 100644<br>--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua<br>+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua<br>@@ -1,5 +1,7 @@<br>+local utils = require('utils')<br>+<br> -- Disabled on *BSD due to #4819.<br>-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')<br>+utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')<br> <br> local path = arg[0]:gsub('%.test%.lua', '')<br> local suffix = package.cpath:match('?.(%a+);')<br>@@ -94,7 +96,8 @@ end))<br> <br> -- Compiled loop with a side exit which does not get compiled.<br> test:ok(testgetmetrics.snap_restores(function()<br>- jit.opt.start(0, "hotloop=1", "hotexit=2", "minstitch=15")<br>+ jit.opt.start(0, "hotloop=1", "hotexit=2",<br>+ ("minstitch=%d"):format(utils.const.maxnins))<br> <br>     local function foo(i)<br>         -- math.fmod is not yet compiled!<br>diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>index 222e7d01..19dfd199 100644<br>--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>@@ -2,8 +2,10 @@<br> -- Major portions taken verbatim or adapted from the LuaVela testing suite.<br> -- Copyright (C) 2015-2019 IPONWEB Ltd.<br> <br>+local utils = require('utils')<br>+<br> -- Disabled on *BSD due to #4819.<br>-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')<br>+utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')<br> <br> local tap = require('tap')<br> <br>@@ -279,7 +281,8 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest)<br>     -- Compiled loop with a side exit which does not get compiled.<br>     subtest:plan(1)<br> <br>- jit.opt.start(0, "hotloop=1", "hotexit=2", "minstitch=15")<br>+ jit.opt.start(0, "hotloop=1", "hotexit=2",<br>+ ("minstitch=%d"):format(utils.const.maxnins))<br> <br>     local function foo(i)<br>         -- math.fmod is not yet compiled!<br>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua<br>index 87f7ff15..eb11d40d 100644<br>--- a/test/tarantool-tests/utils.lua<br>+++ b/test/tarantool-tests/utils.lua<br>@@ -135,4 +135,13 @@ function M.profilename(name)<br>   return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern))<br> end<br> <br>+M.const = {<br>+ -- XXX: Max nins is limited by max IRRef, that equals to<br>+ -- REF_DROP - REF_BIAS. Unfortunately, these constants are not<br>+ -- provided to Lua space, so we ought to make some math:<br>+ -- * REF_DROP = 0xffff<br>+ -- * REF_BIAS = 0x8000<br>+ maxnins = 0xffff - 0x8000,<br>+}<br>+<br> return M<br>--<br>2.34.0</div></div></div></div></blockquote><div> </div></div></blockquote></BODY></HTML>