Re: [Tarantool-patches] [PATCH v1 luajit 39/41] perf: add a script for the environment setup

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

Hi, Sergey!
Thanks for the review.
Please, consider my answers below.

On 18.11.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! See my comments.
> 
> Sergey
> 
> On 10/24/25 14:00, Sergey Kaplun wrote:
> > The patch adds a script for setting the environment before running
> > performance tests. Most of the settings are taken from the Tarantool's
> > wiki page dedicated to the benchmarking [1].
> 
> Honestly, I don't like that we have similar files in two repositories 
> (aggregate, setup-env.sh).

The <aggregate.lua> file is different due to different libraries
available. Also, it has its own logic of scanning of directories.

<setup-env.sh> may be updated too with time independently between the
repositories, since Tarantool and LuaJIT may require the different
setup.

> 
> This makes maintenance more complicated. I would put these files to a 
> shared repository

No. It isn't a good approach. As we discussed a thousand times with Igor
before, there is no need in the third-party dependency for this
repository, it should be self-sufficient. Trust me, you don't want to
investigate some regression in the performance testing results because
somebody updated a third-party repository. This is why we have the our
own tap module for the <tarantool-tests> suite. As you can see, we have
zero problems with it. Also, considering my comment above.

> 
> and reuse it for tarantool and luajit repos.
> 
> Original files are even not specified in commit messages for 
> setup-env.sh and aggregate.lua,

Mentioned that this file was taken from the Tarantool repository in the
commit message.

> 
> I think it is worth it. No sense to review these files if it was already 
> done previously.

It is good to check that I didn't mess-up the content by accident,
anyway.

> 
> >
> > [1]:https://github.com/tarantool/tarantool/wiki/Benchmarking
> > ---

The updated commit message is the following:

| perf: add a script for the environment setup
|
| The patch adds a script for setting the environment before running
| performance tests. The script originated from the Tarantool's repository
| [2]. Most of the settings are taken from the Tarantool's wiki page
| dedicated to the benchmarking [1].
|
| [1]: https://github.com/tarantool/tarantool/blob/dcdb3ee83b3d6324011e704b5a3f4ee3e19bbf47/perf/tools/setup_env.sh
| [2]: https://github.com/tarantool/tarantool/wiki/Benchmarking

<snipped>

-- 
Best regards,
Sergey Kaplun