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

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

[Igor Munkin via Tarantool-patches]

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@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({' ..
-- 
2.25.0

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

[Alexander Turenko via Tarantool-patches]

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@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
test/box-tap/suite.ini. After this change the patch LGTM.

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.

WBR, Alexander Turenko.

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

[Igor Munkin via Tarantool-patches]

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@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@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

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

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the patch, LGTM!

-- 
Best regards,
Sergey Kaplun

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

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review!

On 03.08.21, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch, LGTM!

Added your tag:
| Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>

> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

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

[Vitaliia Ioffe via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]


Hi all, 
 
QA LGTM. 
 
 
--
Vitaliia Ioffe
 
 
>Вторник, 3 августа 2021, 23:56 +03:00 от Igor Munkin via Tarantool-patches < tarantool-patches@dev.tarantool.org >:
> 
>Sergey,
>
>Thanks for your review!
>
>On 03.08.21, Sergey Kaplun wrote:
>> Hi, Igor!
>>
>> Thanks for the patch, LGTM!
>
>Added your tag:
>| Reviewed-by: Sergey Kaplun < skaplun@tarantool.org >
>
>>
>> --
>> Best regards,
>> Sergey Kaplun
>
>--
>Best regards,
>IM
 
 

[-- Attachment #2: Type: text/html, Size: 1303 bytes --]

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

[Igor Munkin via Tarantool-patches]

I've checked the patch into 1.10, 2.7, 2.8 and master.

On 24.07.21, 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@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(-)
> 

<snipped>

> -- 
> 2.25.0
> 

-- 
Best regards,
IM