Re: [Tarantool-patches] [PATCH v1 luajit 01/41] perf: add LuaJIT-test-cleanup perf suite

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

Hi, Sergey!
Thanks for the review!

Please consider my answers below.

On 11.11.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> 
> This is a big step forward for LuaJIT performance testing.
> 
> Please take a look on the comments below.
> 
> Sergey
> 
> On 10/24/25 13:50, Sergey Kaplun wrote:
> > This patch introduces the LuaJIT-test-cleanup bench suite [1] into our
> s/bench/benchmark/

Fixed.

> > LuaJIT fork source tree. To provide relatable reprodusible results
> 
> did not get it: "relatable"

I've meant reliable. Fixed.

> 
> s/reprodusible/reproducible/

Fixed, thanks!

> 
> > several benchmarks need to be adjusted. However, to be sure we initially use
> > the valid suite, everything in the <perf/LuaJIT-benches> directory is
> > moved intact.
> >
> > [1]:https://github.com/LuaJIT/LuaJIT-test-cleanup/tree/014708b/bench

The new commit message is the following:

| perf: add LuaJIT-test-cleanup perf suite
|
| This patch introduces the LuaJIT-test-cleanup benchmark suite [1] into
| our LuaJIT fork source tree. To provide reliable reproducible results
| several benchmarks need to be adjusted. However, to be sure we initially
| use the valid suite, everything in the <perf/LuaJIT-benches> directory
| is moved intact.
|
| [1]: https://github.com/LuaJIT/LuaJIT-test-cleanup/tree/014708b/bench

<snipped>

> > +  'perf/LuaJIT-benches/',
> 
> Please don't do this. It is better to ignore by code number and at least
> 
> some groups of warnings in the code.
> 

It is not clear anyway what these magic numbers mean.
For now it is just more convenient to disable the full set of the added
suite, since we need to ignore the enormous amount of warnings.
We may refactor it later if we have such a need.

I'm not sure that this particular suite will be updated somehow in the
future, so this format looks acceptable. We have the same approach for
LuaJIT tests and PUC-Rio-Lua-5.1 tests

> 
> It is not clear why exactly these parameters are used.

I suppose this is the empirical setup that Mike thinks is optimal.

> 
> Should we change them?
> 

This commit brings the original code as is. So, no, we don't need to
change them here.

> 
> Do we really need parameters for unsupported platforms (MIPS, x86, ppc)?
> 
> it deserves a comment in commit message
> 

This commit brings the original code as is. So, no, we don't need to
change it here.

> please remove a newline
> ...
> trailing newline
> ...
> unnecessary newline
> ...
> unnecessary newline
> ...
> unnecessary newline
> ...
> trailing space

This commit brings the original code as is. So, no, we don't need to
change it here. It should be removed in the corresponding refactoring
commit if you insist.

-- 
Best regards,
Sergey Kaplun