Hi, Igor! Thanks for detailed report, the results are LGTM. As for the measured speedup - I tend to ignore it, since the min-max spread is way bigger. Regards, Sergos  Monday, 21 September 2020, 22:33 +0300 from Igor Munkin : >Sergos, > >Thanks for your review! Please, consider my comments below. > >On 10.07.20, sergos@tarantool.org wrote: >> Hi! >> >> Thanks for the patch and investigation! >> >> >> > On 8 Jul 2020, at 01:24, Igor Munkin < imun@tarantool.org > wrote: >> > >> > Vlad, >> > >> > Thanks for your review! >> > >> > On 01.04.20, Vladislav Shpilevoy wrote: >> >> Hi! Thanks for the patch! >> >> >> >> See 7 comments below. >> >> > > > >> > >> >> >> >> Why can't we call lj_trace_abort() directly? >> > >> > It's the internal API. Its usage complicates a switch between various >> > LuaJIT implementations (we faced several challenges when tried to build >> > Tarantool with uJIT). There is a public API to be used here (though in a >> > bit hacky way). >> >> This hacky way looks fragile, since luaJIT_setmode() may change its behaviour >> in the future and cause some unpredictable result. We have to mention it >> somewhere as a warninig for future LuaJIT updates from upstream. For example, >> introduce a comment inside luaJIT_setmode() that will conflict with plain >> patch. > >We discussed this in the nearby thread[1] with Vlad and finally came to >the solution with . I dropped several comments regarding >the rationale for the fix in v2 version. > >> > > > >> > >> > Looks like this way is slower than the one implemented via triggers. >> >> But does it catch more cases, as Vlad supposed? Do you have an extra >> test for it? > >I provided several benchmarks results in the nearby thread[2]. For the >chosen solution (via internal macro) it has almost no performance >degradation (omitting the noise). > >> >> Also, I would like to see the impact on some ‘real’ test - such as box >> insertion/select or so? > >I tried yours benchmark[3] and got the following numbers: >* Vanilla (insert per second): >| min (15 runs): 809387.28574453 >| median (15 runs): 822854.30884267 >| mean (15 runs): 821996.668288715 >| max (15 runs): 837764.83604149 >* Patched (insert per second): >| min (15 runs): 816005.94236505 >| median (15 runs): 829281.27443029 >| mean (15 runs): 828522.48986598 >| max (15 runs): 839318.90025576 > >Em... It looks like a performance improvement, doesn't it? It seems like >a compiler side-effect (e.g. invalid traces blacklisting), but I didn't >make a deep investigation for this. > >> >> >> Regards, >> Sergos >> > > > >> > >[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019306.html >[2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-September/019521.html >[3]: https://gist.github.com/sergos/feb397ed4d5a5f739ee501f768da31e6 > >-- >Best regards, >IM