[Tarantool-patches] [PATCH] test: enable JIT for Lua Fun chain iterator back

[Igor Munkin]

Sasha,

Thanks for your review!

On 26.07.21, Alexander Turenko wrote:
> On Sat, Jul 24, 2021 at 08:24:54PM +0300, Igor Munkin wrote:
> > This patch reverts the temporary fix introduced in commit
> > 5fa7ded22c1f6b5ab2ed78563538d234776bac76 ("test: disable JIT for Lua Fun
> > chain iterator") since the issues with invalid traces generation for
> > <fun.chain> iterator are resolved and JIT can be enabled back then.
> > 
> > Follows up #4252
> > Follows up #5118
> > 
> > Signed-off-by: Igor Munkin <imun at tarantool.org>
> > ---
> > 
> > Related issues:
> > * https://github.com/tarantool/tarantool/issues/5118
> > * https://github.com/tarantool/tarantool/issues/4252
> > * https://github.com/LuaJIT/LuaJIT/issues/584
> > Branch: https://github.com/tarantool/tarantool/tree/imun/lj-584-bad-renames-for-sunk-values
> > CI: https://github.com/tarantool/tarantool/commit/b35e2ee
> > 
> >  test/box-tap/key_def.test.lua | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
> > index 41eed2191..2ade668a5 100755
> > --- a/test/box-tap/key_def.test.lua
> > +++ b/test/box-tap/key_def.test.lua
> > @@ -5,13 +5,6 @@ local ffi = require('ffi')
> >  local json = require('json')
> >  local fun = require('fun')
> >  
> > --- XXX fix for gh-4252: to prevent invalid trace assembling (see
> > --- https://github.com/LuaJIT/LuaJIT/issues/584) disable JIT for
> > --- <fun.chain> iterator (i.e. <chain_gen_r1>). Since the function
> > --- is local, the dummy chain generator is created to obtain the
> > --- function GC object.
> > -jit.off(fun.chain({}).gen)
> > -
> >  local key_def_lib = require('key_def')
> >  
> >  local usage_error = 'Bad params, use: key_def.new({' ..
> 
> I cannot say much about the fix itself. So I'll just ensure that the
> test is stable now.
> 
> Please, also remove the test from the fragile list in

I didn't touch suite.ini since there is another issue mentioned with no
checksums at all. Now I see this is a duplicate of #4252, so I removed
it and mentioned #5049 in the commit message. Here is the diff:

================================================================================

diff --git a/test/box-tap/suite.ini b/test/box-tap/suite.ini
index d854fddbb..b09d7db4f 100644
--- a/test/box-tap/suite.ini
+++ b/test/box-tap/suite.ini
@@ -12,9 +12,6 @@ fragile = {
             "issues": [ "gh-5346", "gh-4344" ],
             "checksums": [ "d41d8cd98f00b204e9800998ecf8427e", "53be182412baa1509cb7a3e27beca3ed" ]
         },
-        "key_def.test.lua": {
-            "issues": [ "gh-5049" ]
-        },
         "net.box.test.lua": {
             "issues": [ "gh-5346" ],
             "checksums": [ "d41d8cd98f00b204e9800998ecf8427e" ]

================================================================================

> test/box-tap/suite.ini. After this change the patch LGTM.

Added your tag:
| Reviewed-by: Alexander Turenko <alexander.turenko at tarantool.org>

> 
> How I tested:
> 
> $ ./test/test-run.py $(yes box-tap/key_def.test.lua | head -n 1000)
> 
> I run it with and without c4bbd4013711c33c3ced9c2e8ac31a80992d2cf0
> ("luajit: bump new version: Detect inconsistent renames even in the
> presence of sunk values.").
> 
> Before the patch it usually fails. After applying the patch: nope, works
> fine.

Neato, thanks for verifying!

> 
> WBR, Alexander Turenko.

-- 
Best regards,
IM