From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 8F37F13E7535; Fri, 6 Jun 2025 11:51:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8F37F13E7535 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1749199902; bh=8Y6fV9hYokLUlVR0yPdQ1PKlprZG9/kxYd9DYRABXR8=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=ADcEN3jaFArdWIUSGaaV9qyx+zC17x+f1qi1V91OXgpI6VrvId4bSpwJqJG1z0yU8 RSZ04eSr7SXvZAyfXIpV+HuabDsMZSKYtJREuOuviNlW2tl7QxuokO4rE8RGKafsOB YKtxWR0+k8I+8rGeuiWXRtFUlV/ZUk8beyKCF0xQ= Received: from send58.i.mail.ru (send58.i.mail.ru [89.221.237.153]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5ED726ECCE for ; Fri, 6 Jun 2025 11:51:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5ED726ECCE Received: by exim-smtp-567cc788d4-wmv86 with esmtpa (envelope-from ) id 1uNSnb-00000000Qb7-1Rej; Fri, 06 Jun 2025 11:51:39 +0300 Content-Type: multipart/alternative; boundary="------------xtf5rzYaphLX0qJSJ8BRC594" Message-ID: Date: Fri, 6 Jun 2025 11:51:39 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <20250603173559.24591-1-skaplun@tarantool.org> <59b25b0e-7cfc-4e67-8716-403b2ca2713a@tarantool.org> Content-Language: en-US In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD95E97EA1894635BC3577730E5842974C3685F41BE2997D50B182A05F53808504066759F62083364823DE06ABAFEAF6705D184FFD951B2C59A5A1D8B7E3AD97713C252A97683EF5A77 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BCC85671EC7A750CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB5533756680FD86C7FE90CFE50A6FEF232668C3570ABD13BA9F7CF0BD71B3CF3E472AF666389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C091DAD9F922AA71188941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C2249D9EEDE0C260548BB76E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B13AA280BE71E98733AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B1D471462564A2E1935872C767BF85DA227C277FBC8AE2E8BA56E11165BA017C7EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A51B7938DD744763145002B1117B3ED6962C85BC47ABA17A98108A05421C070DB8823CB91A9FED034534781492E4B8EEADADEF88395FA75C5FBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34ADE558D2B396DA7C8DA7B3234E25A6B70D8A26C200837B2022D2C556B91CB65C4821A85E9E59E2001D7E09C32AA3244C25FE90AFDFEFEE0977DD89D51EBB7742F6FA917C072618CDEA455F16B58544A2E30DDF7C44BCB90DA5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVSykAyseJQ6/PjPJBJhaJeE= X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D614019E3E5759D5A133C4BD03525DDD6C8961021A4CC35CAAD970152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] build: provide LUAJIT_USE_PERFTOOLS option X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------xtf5rzYaphLX0qJSJ8BRC594 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello, Sergey, thanks for fixes! LGTM On 6/5/25 10:40, Sergey Kaplun wrote: > 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 > >> >> >> script: unexpected number of arguments >> Try 'script --help' for more information. >> >> >> 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. It is a note after testing the patch, please disregard it > > > >>> 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 (%d = pid of process) file. It should be >>> +# used with Linux perf tools. See 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. > We can build LuaJIT in CI with enabled macro, but without test it does not guarantee anything. Ok, let's leave it without test for now. >>> + >>> # 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 > --------------xtf5rzYaphLX0qJSJ8BRC594 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hello, Sergey,

thanks for fixes! LGTM

On 6/5/25 10:40, Sergey Kaplun wrote:
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.
It is a note after testing the patch, please disregard it

<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.

We can build LuaJIT in CI with enabled macro, but without test

it does not guarantee anything. Ok, let's leave it without test for now.


        

          
+
  # 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



    

  
  


--------------xtf5rzYaphLX0qJSJ8BRC594--