Re: [Tarantool-patches] [PATCH luajit] Save trace recorder state around VM event call.\

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim!

Thanks for the review!

On 29.10.22, Maxim Kokryashkin wrote:
> 
> Hi!
> Thanks for the patch!
> Please consider my comment below.
> > 
> >>Hi, Sergos!
> >>
> >>Thanks for the review!
> >>
> >>On 25.10.22, sergos wrote:
> >>> Hi!
> >>>
> >>> Thanks for the patch!
> >>>
> >>> Some comments on the commit message and the test showed flaky behavior,
> >>> it failed only 2 times out of 10 runs using the
> >>> Tarantool 2.11.0-entrypoint-637-gdd7d46af3 on a
> >>> Darwin s-ostanevich2 22.1.0 Darwin Kernel Version 22.1.0
> >>
> >>OMG, `tap` in tarantool is slightly different so the test is flaky.
> >>
> >>Fixes with increasing amount of calls of `fibb()` function.
> >>
> >>Also, removes `jit.bc.dump()` output.
> >>
> >>See the iterative patch below.
> >>===================================================================
> >>diff --git a/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua b/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
> >>index b5146f70..475d9200 100644
> >>--- a/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
> >>+++ b/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
> >>@@ -18,11 +18,16 @@ local function fibb(n)
> >>   return n < 2 and n or fibb(n - 1) + fibb(n - 2)
> >> end
> >> 
> >>+local function empty() end
> >> -- Compile `jit.bc` functions, that are used in vmevent handler.
> >>-require('jit.bc').dump(loadstring(string.dump(fibb)))
> >>+require('jit.bc').dump(loadstring(string.dump(fibb)), {
> >>+ write = empty,
> >>+ close = empty,
> >>+ flush = empty,
> >>+})
> >> 
> >> -- Here we dump (to /dev/null) info about `fibb()` traces and run
> >> -- `jit.bc` functions inside.
> >>-test:ok(fibb(0) == 0, 'run compiled function inside vmevent handler')
> >>+test:ok(fibb(2) == 1, 'run compiled function inside vmevent handler')
> >> 
> >> os.exit(test:check() and 0 or 1)
> >>===================================================================
> >That test passes even without the patch on M1 LJ_64, LJ_GC64, LJ_DUALNUM.
> ><snipped>

I suppose, that this is not the huge problem. Maybe some traces for
`jit.bc` are not compiled on M1. The fact that the test fails on x86/x64
!GC64 is enough for me (at least it is how bug has been detected).

> > 
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
Sergey Kaplun