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
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.
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
Hi, Igor! Thanks for the patch, LGTM! -- Best regards, Sergey Kaplun
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 #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 --]
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