Re: [Tarantool-patches] [PATCH luajit] build: provide LUAJIT_USE_PERFTOOLS option

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

Hi, Sergey!
Thanks for the review!
See my answers below.
Fixed your comments regarding the commit message and force-pushed the
branch.

On 04.06.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> 
> On 6/3/25 20:35, Sergey Kaplun wrote:
> > This patch provides the LUAJIT_USE_PERFTOOLS flag via the CMake build
> > system. It allows you to avoid the definition of the cognominal macro
> 
> it's better to write impersonally: It allows avoiding the definition of ...
> 
> Feel free to ignore.

Rephrased as has been suggested.

> 
> 
> > definition via CMAKE_C_FLAGS to use integration with the Linux perf
> > tools interface [1] to resolve symbols for traces generated by a JIT.
> >
> > It may be used like the following:
> >
> > | perf record script -f luajit test.lua
> 
> seems command is incorrect, because it does not work for me:

Ooops. I forget that the `--force` flag has been removed [1].
Fixed.

The resulting commit message is the following:

| build: provide LUAJIT_USE_PERFTOOLS option
|
| This patch provides the LUAJIT_USE_PERFTOOLS flag via the CMake build
| system. It allows avoiding the definition of the cognominal macro
| definition via CMAKE_C_FLAGS to use integration with the Linux perf
| tools interface [1] to resolve symbols for traces generated by a JIT.
|
| It may be used like the following:
|
| | perf record script luajit test.lua
| | perf report -s symbol
|
| [1]: https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt
|
| Resolves tarantool/tarantool#11300

> 
> <snipped>
> 
> script: unexpected number of arguments
> Try 'script --help' for more information.
> <snipped>
> 
> I've used instead:
> 
> $ sudo perf record -F 2000 ./build/src/luajit fib.lua
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.024 MB perf.data (8 samples) ]
> 
> > | perf report -s symbol
> >
> and "perf report /tmp/perf-1699839.map" to check that luajit report 
> symbols in map file.

Don't get this part.

<snipped>

> > Branch:https://github.com/tarantool/luajit/tree/skaplun/gh-11300-use-perftools-flag
> > Issue:https://github.com/tarantool/tarantool/issues/11300
> >
> >   CMakeLists.txt | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index 854b3613..c0da4362 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> > @@ -259,6 +259,14 @@ if(LUAJIT_USE_VALGRIND)
> >     AppendFlags(TARGET_C_FLAGS -DLUAJIT_USE_VALGRIND)
> >   endif()
> >   
> > +# This creates a symbol table of the JIT-compiled code in
> > +# a </tmp/perf-%d.map> (%d = pid of process) file. It should be
> > +# used with Linux perf tools. See <src/lj_trace.c> for details.
> > +option(LUAJIT_USE_PERFTOOLS "Linux perf JIT support" OFF)
> > +if(LUAJIT_USE_PERFTOOLS)
> > +  AppendFlags(TARGET_C_FLAGS -DLUAJIT_USE_PERFTOOLS)
> > +endif()
> 
> Adding a CMake flag means that we support it in our fork (users will 
> rely on this functionality).
> 
> Do want a regression test for this option?

I've honestly don't see a way to conveniently check for it, and it looks
like overkill for now (since its functionality is rather frugal).
Moreover, perf annotate is not working as expected with that.

I'm glad to hear any ideas of yours about it.

> 
> > +
> >   # This is the client for the GDB JIT API. GDB 7.0 or higher is
> >   # required to make use of it. See lj_gdbjit.c for details.
> >   # Enabling this causes a non-negligible overhead, even when not

[1]: http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=4a4d371a4dfbd3b84a7eab8d535d4c7c3647b09e

-- 
Best regards,
Sergey Kaplun