<HTML><BODY><div id="composeWebView_editable_content" data-mailruapp-compose-id="composeWebView_editable_content" style="text-align: left;"><div>Hi, Igor!</div><div><br></div><div>Thanks for detailed report, the results are LGTM.</div><div>As for the measured speedup - I tend to ignore it, since the min-max spread is way bigger.</div><div><br></div><div>Regards,</div><div>Sergos </div><br>Monday, 21 September 2020, 22:33 +0300 from Igor Munkin <imun@tarantool.org>:<br><div id="composeWebView_previouse_content" data-mailruapp-compose-id="composeWebView_previouse_content"><blockquote id="mail-app-auto-quote" style="border-left-width: 1px; border-left-style: solid; border-left-color: rgb(11, 102, 249); margin: 10px 0px 10px 5px; padding: 0px 0px 0px 10px; display: inherit;" data-darkosha-id="1:3"><div class="js-helper js-readmsg-msg" data-darkosha-id="1:4" style="">
<style type="text/css" data-darkosha-id="1:5" style=""></style>
<div data-darkosha-id="1:6" style="">
<base target="_self" href="https://e.mail.ru/" data-darkosha-id="1:7" style="">
<div id="style_16007168341699335483_BODY" data-darkosha-id="1:8" style="">Sergos,<br data-darkosha-id="1:9" style="">
<br data-darkosha-id="1:10" style="">
Thanks for your review! Please, consider my comments below.<br data-darkosha-id="1:11" style="">
<br data-darkosha-id="1:12" style="">
On 10.07.20, <a href="mailto:sergos@tarantool.org" data-darkosha-id="1:13" style="">sergos@tarantool.org</a> wrote:<br data-darkosha-id="1:14" style="">
> Hi!<br data-darkosha-id="1:15" style="">
> <br data-darkosha-id="1:16" style="">
> Thanks for the patch and investigation!<br data-darkosha-id="1:17" style="">
> <br data-darkosha-id="1:18" style="">
> <br data-darkosha-id="1:19" style="">
> > On 8 Jul 2020, at 01:24, Igor Munkin <<a href="mailto:imun@tarantool.org" data-darkosha-id="1:20" style="">imun@tarantool.org</a>> wrote:<br data-darkosha-id="1:21" style="">
> > <br data-darkosha-id="1:22" style="">
> > Vlad,<br data-darkosha-id="1:23" style="">
> > <br data-darkosha-id="1:24" style="">
> > Thanks for your review!<br data-darkosha-id="1:25" style="">
> > <br data-darkosha-id="1:26" style="">
> > On 01.04.20, Vladislav Shpilevoy wrote:<br data-darkosha-id="1:27" style="">
> >> Hi! Thanks for the patch!<br data-darkosha-id="1:28" style="">
> >> <br data-darkosha-id="1:29" style="">
> >> See 7 comments below.<br data-darkosha-id="1:30" style="">
> >> <br data-darkosha-id="1:31" style="">
<br data-darkosha-id="1:32" style="">
<snipped><br data-darkosha-id="1:33" style="">
<br data-darkosha-id="1:34" style="">
> > <br data-darkosha-id="1:35" style="">
> >> <br data-darkosha-id="1:36" style="">
> >> Why can't we call lj_trace_abort() directly?<br data-darkosha-id="1:37" style="">
> > <br data-darkosha-id="1:38" style="">
> > It's the internal API. Its usage complicates a switch between various<br data-darkosha-id="1:39" style="">
> > LuaJIT implementations (we faced several challenges when tried to build<br data-darkosha-id="1:40" style="">
> > Tarantool with uJIT). There is a public API to be used here (though in a<br data-darkosha-id="1:41" style="">
> > bit hacky way).<br data-darkosha-id="1:42" style="">
> <br data-darkosha-id="1:43" style="">
> This hacky way looks fragile, since luaJIT_setmode() may change its behaviour<br data-darkosha-id="1:44" style="">
> in the future and cause some unpredictable result. We have to mention it <br data-darkosha-id="1:45" style="">
> somewhere as a warninig for future LuaJIT updates from upstream. For example,<br data-darkosha-id="1:46" style="">
> introduce a comment inside luaJIT_setmode() that will conflict with plain<br data-darkosha-id="1:47" style="">
> patch.<br data-darkosha-id="1:48" style="">
<br data-darkosha-id="1:49" style="">
We discussed this in the nearby thread[1] with Vlad and finally came to<br data-darkosha-id="1:50" style="">
the solution with <lj_trace_abort>. I dropped several comments regarding<br data-darkosha-id="1:51" style="">
the rationale for the fix in v2 version.<br data-darkosha-id="1:52" style="">
<br data-darkosha-id="1:53" style="">
> <br data-darkosha-id="1:54" style="">
<br data-darkosha-id="1:55" style="">
<snipped><br data-darkosha-id="1:56" style="">
<br data-darkosha-id="1:57" style="">
> > <br data-darkosha-id="1:58" style="">
> > Looks like this way is slower than the one implemented via triggers.<br data-darkosha-id="1:59" style="">
> <br data-darkosha-id="1:60" style="">
> But does it catch more cases, as Vlad supposed? Do you have an extra<br data-darkosha-id="1:61" style="">
> test for it?<br data-darkosha-id="1:62" style="">
<br data-darkosha-id="1:63" style="">
I provided several benchmarks results in the nearby thread[2]. For the<br data-darkosha-id="1:64" style="">
chosen solution (via internal macro) it has almost no performance<br data-darkosha-id="1:65" style="">
degradation (omitting the noise).<br data-darkosha-id="1:66" style="">
<br data-darkosha-id="1:67" style="">
> <br data-darkosha-id="1:68" style="">
> Also, I would like to see the impact on some ‘real’ test - such as box<br data-darkosha-id="1:69" style="">
> insertion/select or so?<br data-darkosha-id="1:70" style="">
<br data-darkosha-id="1:71" style="">
I tried yours benchmark[3] and got the following numbers:<br data-darkosha-id="1:72" style="">
* Vanilla (insert per second):<br data-darkosha-id="1:73" style="">
| min (15 runs): <span class="js-phone-number" data-darkosha-id="1:74" style="">809387.28574453</span><br data-darkosha-id="1:75" style="">
| median (15 runs): <span class="js-phone-number" data-darkosha-id="1:76" style="">822854.30884267</span><br data-darkosha-id="1:77" style="">
| mean (15 runs): <span class="js-phone-number" data-darkosha-id="1:78" style="">821996.668288715</span><br data-darkosha-id="1:79" style="">
| max (15 runs): <span class="js-phone-number" data-darkosha-id="1:80" style="">837764.83604149</span><br data-darkosha-id="1:81" style="">
* Patched (insert per second):<br data-darkosha-id="1:82" style="">
| min (15 runs): <span class="js-phone-number" data-darkosha-id="1:83" style="">816005.94236505</span><br data-darkosha-id="1:84" style="">
| median (15 runs): <span class="js-phone-number" data-darkosha-id="1:85" style="">829281.27443029</span><br data-darkosha-id="1:86" style="">
| mean (15 runs): <span class="js-phone-number" data-darkosha-id="1:87" style="">828522.48986598</span><br data-darkosha-id="1:88" style="">
| max (15 runs): <span class="js-phone-number" data-darkosha-id="1:89" style="">839318.90025576</span><br data-darkosha-id="1:90" style="">
<br data-darkosha-id="1:91" style="">
Em... It looks like a performance improvement, doesn't it? It seems like<br data-darkosha-id="1:92" style="">
a compiler side-effect (e.g. invalid traces blacklisting), but I didn't<br data-darkosha-id="1:93" style="">
make a deep investigation for this.<br data-darkosha-id="1:94" style="">
<br data-darkosha-id="1:95" style="">
> <br data-darkosha-id="1:96" style="">
> <br data-darkosha-id="1:97" style="">
> Regards,<br data-darkosha-id="1:98" style="">
> Sergos<br data-darkosha-id="1:99" style="">
> <br data-darkosha-id="1:100" style="">
<br data-darkosha-id="1:101" style="">
<snipped><br data-darkosha-id="1:102" style="">
<br data-darkosha-id="1:103" style="">
> <br data-darkosha-id="1:104" style="">
<br data-darkosha-id="1:105" style="">
[1]: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019306.html" target="_blank" data-darkosha-id="1:106" style="">https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019306.html</a><br data-darkosha-id="1:107" style="">
[2]: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019521.html" target="_blank" data-darkosha-id="1:108" style="">https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019521.html</a><br data-darkosha-id="1:109" style="">
[3]: <a href="https://gist.github.com/sergos/feb397ed4d5a5f739ee501f768da31e6" target="_blank" data-darkosha-id="1:110" style="">https://gist.github.com/sergos/feb397ed4d5a5f739ee501f768da31e6</a><br data-darkosha-id="1:111" style="">
<br data-darkosha-id="1:112" style="">
-- <br data-darkosha-id="1:113" style="">
Best regards,<br data-darkosha-id="1:114" style="">
IM<br data-darkosha-id="1:115" style="">
</div>
<base target="_self" href="https://e.mail.ru/" data-darkosha-id="1:116" style="">
</div>
</div></blockquote></div></div></BODY></HTML>