[Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Sergey Kaplun via Tarantool-patches]

The aforementioned test is flaky when run under Tarantool due to not
enough room for all traces. The flushing of all traces at the start and
collecting them solves the issue.

The same issue may occur for the
<gh-6098-fix-side-exit-patching-on-arm64.test.lua>, so it is adjusted as
well.
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-mips64-flaky-test
To reproduce, run the following command without the patch from the root
of the Tarantool repository:
| ctest --repeat-until-fail 1000 --test-dir third_party/luajit/ -R fix-mips64-spare-side-exit-patching

 .../fix-mips64-spare-side-exit-patching.test.lua             | 5 +++++
 .../gh-6098-fix-side-exit-patching-on-arm64.test.lua         | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
index 703d8e69..dd9f2073 100644
--- a/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
+++ b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
@@ -15,6 +15,11 @@ local MAXTRACE = 2000;
 
 test:plan(1)
 
+-- Flush all possible traces and collect them to be sure that
+-- we have enough space.
+jit.flush()
+collectgarbage()
+
 local function find_last_trace()
   local candidate = misc.getmetrics().jit_trace_num
   for traceno = candidate, MAXTRACE do
diff --git a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
index 678ac914..f5fc4681 100644
--- a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
+++ b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
@@ -9,6 +9,11 @@ local frontend = require('utils').frontend
 
 test:plan(1)
 
+-- Flush all possible traces and collect them to be sure that
+-- we have enough space.
+jit.flush()
+collectgarbage()
+
 -- The function to be tested for side exit patching:
 -- * At the beginning of the test case, the <if> branch is
 --   recorded as a root trace.
-- 
2.48.1

Re: [Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey,

thanks for the patch!

On 27.02.2025 12:18, Sergey Kaplun wrote:
> The aforementioned test is flaky when run under Tarantool due to not
> enough room for all traces. The flushing of all traces at the start and
> collecting them solves the issue.
Have you considered another options like increasing `maxtrace` value?
> The same issue may occur for the
> <gh-6098-fix-side-exit-patching-on-arm64.test.lua>, so it is adjusted as
> well.
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-mips64-flaky-test
> To reproduce, run the following command without the patch from the root
> of the Tarantool repository:
> | ctest --repeat-until-fail 1000 --test-dir third_party/luajit/ -R fix-mips64-spare-side-exit-patching
>
>   .../fix-mips64-spare-side-exit-patching.test.lua             | 5 +++++
>   .../gh-6098-fix-side-exit-patching-on-arm64.test.lua         | 5 +++++
>   2 files changed, 10 insertions(+)
>
> diff --git a/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
> index 703d8e69..dd9f2073 100644
> --- a/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
> +++ b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
> @@ -15,6 +15,11 @@ local MAXTRACE = 2000;
>   
>   test:plan(1)
>   
> +-- Flush all possible traces and collect them to be sure that
> +-- we have enough space.
> +jit.flush()
> +collectgarbage()
> +
>   local function find_last_trace()
>     local candidate = misc.getmetrics().jit_trace_num
>     for traceno = candidate, MAXTRACE do
> diff --git a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
> index 678ac914..f5fc4681 100644
> --- a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
> +++ b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
> @@ -9,6 +9,11 @@ local frontend = require('utils').frontend
>   
>   test:plan(1)
>   
> +-- Flush all possible traces and collect them to be sure that
> +-- we have enough space.
> +jit.flush()
> +collectgarbage()
> +
>   -- The function to be tested for side exit patching:
>   -- * At the beginning of the test case, the <if> branch is
>   --   recorded as a root trace.

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

Re: [Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!

On 27.02.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> 
> On 27.02.2025 12:18, Sergey Kaplun wrote:
> > The aforementioned test is flaky when run under Tarantool due to not
> > enough room for all traces. The flushing of all traces at the start and
> > collecting them solves the issue.
> Have you considered another options like increasing `maxtrace` value?

We may also:
1) Increase the maxtrace value (as you are asking) -- but this is fragile,
since the number of built-in traces inside the Tarantool may grow up
again, so we should continuously adjust it in the future.
2) Disable the test for the Tarantool. Not so bad, but unnecessary --
since we may make the test platform-independent.

> > The same issue may occur for the
> > <gh-6098-fix-side-exit-patching-on-arm64.test.lua>, so it is adjusted as
> > well.
> > ---

<snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey,

LGTM,  thanks!

On 27.02.2025 19:05, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> On 27.02.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch!
>>
>> On 27.02.2025 12:18, Sergey Kaplun wrote:
>>> The aforementioned test is flaky when run under Tarantool due to not
>>> enough room for all traces. The flushing of all traces at the start and
>>> collecting them solves the issue.
>> Have you considered another options like increasing `maxtrace` value?
> We may also:
> 1) Increase the maxtrace value (as you are asking) -- but this is fragile,
> since the number of built-in traces inside the Tarantool may grow up
> again, so we should continuously adjust it in the future.
> 2) Disable the test for the Tarantool. Not so bad, but unnecessary --
> since we may make the test platform-independent.
Ok, I got it.
>>> The same issue may occur for the
>>> <gh-6098-fix-side-exit-patching-on-arm64.test.lua>, so it is adjusted as
>>> well.
>>> ---
> <snipped>
>

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

Re: [Tarantool-patches] [PATCH luajit] test: fix fix-mips64-spare-side-exit-patching

[Sergey Kaplun via Tarantool-patches]

I've applied the patch into all long-term branches in tarantool/luajit
and bumped a new version in master [1], release/3.3 [2], release/3.2 [3]
and release/2.11 [4].

[1]: https://github.com/tarantool/tarantool/pull/11216
[2]: https://github.com/tarantool/tarantool/pull/11217
[3]: https://github.com/tarantool/tarantool/pull/11218
[4]: https://github.com/tarantool/tarantool/pull/11219

-- 
Best regards,
Sergey Kaplun