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 940D81624106; Thu, 15 Jan 2026 10:47:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 940D81624106 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1768463228; bh=Bv4j2/m6LqTZS1Rvmj0WibJQKXUI1WDs/GRVbAsyEm0=; 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=j1rvBhkNxyN4X1N1rMy9ZNzRmbPSzz0sQumI54cYNpQgVgilXg1EsoOn8PXPSj7DT cPJcsvxXIHQgay+whm56Pm9aqqydndfPMEn7rozVNUYZDHoqhndeejMmNhNvwYi63F QfhinkKv0L3lVSYKkK4p0P3GKFgmfpHzfA/2IDSk= Received: from send240.i.mail.ru (send240.i.mail.ru [95.163.59.79]) (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 0DD9E1624106 for ; Thu, 15 Jan 2026 10:47:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0DD9E1624106 Received: by exim-smtp-55f9c6db88-n7kgv with esmtpa (envelope-from ) id 1vgI4P-00000000Fxf-3Nz8; Thu, 15 Jan 2026 10:47:06 +0300 Content-Type: multipart/alternative; boundary="------------XRYOzR2GEdFiw0eaBIYXZtSi" Message-ID: Date: Thu, 15 Jan 2026 10:47:04 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <997b5837-5e1b-4fd1-a8e8-bc25d6a7a8da@tarantool.org> Content-Language: en-US In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD99B884C1B59F9AF80EBA57780F58D6300D000DD0A7B235B67182A05F538085040E1575C82901FCB693DE06ABAFEAF67059DF7E3EEA341C57DA3281B6BB245D3B519F33B7625D3E01E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE757F64E7FD849EB4FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB5533756680939BF45BBEA696B3ECB85A65B2A8E42BF27AB91571256C84288CB13DA703D6389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B64854413538E1713FCC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE70BB89B22BF4660DC731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5ABFB400F00C872F85002B1117B3ED696948E48EA5ED76C09957033528158102E823CB91A9FED034534781492E4B8EEADCF86CE9B81855096 X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE191716CD42B3DD1D34CAB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D659CD5EF6BB38AEF45925A421C7FD70A090E0DB6D3959D09102E6EE654BCDBDF6F3D2FA47FE695380C9B8341EE9D5BE9A0A91625ADA531DD9A7CB3E08663C48719120A010C61ACC18338CD93680B12512CF4C41F94D744909CE2512F26BEC029E55448553D2254B8D95CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVVt0N+pdRHOZR9/eTdNB2aQ= X-Mailru-Sender: 689FA8AB762F7393DDD5FD59B456EAD28A6F8E66794F2EACFB54B90273B00DA9A8A9E483CF974901EF86D5F70DA33880E41E8EF7A07863ECB274557F927329BE2DDF8182D28ACDB545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 luajit 37/41] perf: add CMake infrastructure 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. --------------XRYOzR2GEdFiw0eaBIYXZtSi Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Sergey! LGTM On 1/14/26 11:50, Sergey Kaplun wrote: > Hi, Sergey! > Fixed your comments, see the iterative patches below. > > On 13.01.26, Sergey Bronnikov wrote: >> Hi, Sergey! >> >> thanks for the patch! LGTM in general, see my comments below. >> >> Sergey >> >> On 12/26/25 12:18, Sergey Kaplun wrote: > > >>> +macro(AddBenchTarget perf_suite) >>> + file(MAKE_DIRECTORY "${PERF_OUTPUT_DIR}/${perf_suite}/") >>> + message(STATUS "Add perf suite ${perf_suite}") >>> + add_custom_target(${perf_suite}) >>> + add_custom_target(${perf_suite}-console >>> + COMMAND ${CMAKE_CTEST_COMMAND} >>> + -L ${perf_suite} >> use long option "--label-regex" instead "-L" > Fixed: > =================================================================== > diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt > index 06603dfd..d03d74a0 100644 > --- a/perf/CMakeLists.txt > +++ b/perf/CMakeLists.txt > @@ -47,7 +47,7 @@ macro(AddBenchTarget perf_suite) > add_custom_target(${perf_suite}) > add_custom_target(${perf_suite}-console > COMMAND ${CMAKE_CTEST_COMMAND} > - -L ${perf_suite} > + --label-regex ${perf_suite} > --parallel 1 > --verbose > --output-on-failure > =================================================================== > >>> + --parallel 1 >>> + --verbose >>> + --output-on-failure >>> + --no-tests=error >>> + ) >>> + add_dependencies(${perf_suite}-console luajit-main) >>> +endmacro() >>> + >>> +# Add the bench to the pair of targets created by the call above. >>> +macro(AddBench bench_name bench_path perf_suite LUA_PATH) >>> + set(bench_title "perf/${perf_suite}/${bench_name}") >>> + get_filename_component(bench_name_stripped ${bench_name} NAME_WE) >>> + set(bench_out_file >>> + ${PERF_OUTPUT_DIR}/${perf_suite}/${bench_name_stripped}.json >>> + ) >>> + set(bench_command "${LUAJIT_BINARY} ${bench_path}") >>> + if(${ARGC} GREATER 4) >>> + set(input_file ${ARGV4}) >>> + set(bench_command "${bench_command} < ${input_file}") >>> + endif() >>> + set(BENCH_FLAGS >>> + "--benchmark_out_format=json --benchmark_out=${bench_out_file}" >> Why BENCH_FLAGS is in uppercase and bench_command_flags in lower-case? >> >> What is the difference? >> >> Also, it seems these variables are unused and can be removed: >> >> @ -56,10 +58,6 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH) >>      set(input_file ${ARGV4}) >>      set(bench_command "${bench_command} < ${input_file}") >>    endif() >> -  set(BENCH_FLAGS >> -    "--benchmark_out_format=json --benchmark_out=${bench_out_file}" >> -  ) >> -  set(bench_command_flags ${bench_command} ${BENCH_FLAGS}) >>    separate_arguments(bench_command_separated UNIX_COMMAND >> ${bench_command}) >>    add_custom_command( >>      COMMAND ${CMAKE_COMMAND} -E env >> > Dropped, thanks! > > =================================================================== > diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt > index 5a5cf777..0916402e 100644 > --- a/perf/CMakeLists.txt > +++ b/perf/CMakeLists.txt > @@ -63,10 +63,6 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH) > set(input_file ${ARGV4}) > set(bench_command "${bench_command} < ${input_file}") > endif() > - set(BENCH_FLAGS > - "--benchmark_out_format=json --benchmark_out=${bench_out_file}" > - ) > - set(bench_command_flags ${bench_command} ${BENCH_FLAGS}) > separate_arguments(bench_command_separated UNIX_COMMAND ${bench_command}) > add_custom_command( > COMMAND ${CMAKE_COMMAND} -E env > =================================================================== > >>> + ) >>> + set(bench_command_flags ${bench_command} ${BENCH_FLAGS}) >>> + separate_arguments(bench_command_separated UNIX_COMMAND ${bench_command}) >>> + add_custom_command( >>> + COMMAND ${CMAKE_COMMAND} -E env >>> + LUA_PATH="${LUA_PATH}" >>> + LUA_CPATH="${LUA_CPATH}" >>> + ${bench_command_separated} >>> + --benchmark_out_format=json >>> + --benchmark_out="${bench_out_file}" >>> + OUTPUT ${bench_out_file} >>> + DEPENDS luajit-main >>> + COMMENT >>> + "Running benchmark ${bench_title} saving results in ${bench_out_file}." >>> + ) >>> + add_custom_target(${bench_name} DEPENDS ${bench_out_file}) >>> + add_dependencies(${perf_suite} ${bench_name}) >>> + >>> + # Report in the console. >>> + add_test(NAME ${bench_title} >>> + COMMAND sh -c "${bench_command}" >> I propose to use find_program for shell executable, like we do in >> tarantool's root cmakelists.txt: >> >> --- a/perf/CMakeLists.txt >> +++ b/perf/CMakeLists.txt >> @@ -1,5 +1,7 @@ >>  # Running various bench suites against LuaJIT. >> >> +find_program(SHELL sh) >> + >>  include(MakeLuaPath) >> >>  if(CMAKE_BUILD_TYPE STREQUAL "Debug") @@ -78,7 +80,7 @@ macro(AddBench bench_name bench_path perf_suite >> LUA_PATH)    # Report in the console.    add_test(NAME ${bench_title} >> -    COMMAND sh -c "${bench_command}" >> +    COMMAND ${SHELL} -c "${bench_command}" >>    ) >>    set_tests_properties(${bench_title} PROPERTIES >>      ENVIRONMENT "LUA_PATH=${LUA_PATH}" > Added: > > =================================================================== > diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt > index 0916402e..e3af1996 100644 > --- a/perf/CMakeLists.txt > +++ b/perf/CMakeLists.txt > @@ -2,6 +2,11 @@ > > include(MakeLuaPath) > > +find_program(SH sh) > +if(NOT SH) > + message(FATAL_ERROR "`sh' is not found") > +endif() > + > if(CMAKE_BUILD_TYPE STREQUAL "Debug") > message(WARNING "LuaJIT and perf tests are built in the Debug mode. " > "Timings may be affected.") @@ -81,7 +86,7 @@ macro(AddBench bench_name bench_path perf_suite > LUA_PATH) # Report in the console. add_test(NAME ${bench_title} - > COMMAND sh -c "${bench_command}" > + COMMAND ${SH} -c "${bench_command}" > ) > set_tests_properties(${bench_title} PROPERTIES > ENVIRONMENT "LUA_PATH=${LUA_PATH}" > =================================================================== > >>> + ) >>> + set_tests_properties(${bench_title} PROPERTIES >>> + ENVIRONMENT "LUA_PATH=${LUA_PATH}" >>> + LABELS ${perf_suite} >>> + DEPENDS luajit-main >>> + ) >>> + unset(input_file) >>> +endmacro() >>> + > > --------------XRYOzR2GEdFiw0eaBIYXZtSi Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Sergey!

LGTM

On 1/14/26 11:50, Sergey Kaplun wrote:
Hi, Sergey!
Fixed your comments, see the iterative patches below.

On 13.01.26, Sergey Bronnikov wrote:
Hi, Sergey!

thanks for the patch! LGTM in general, see my comments below.

Sergey

On 12/26/25 12:18, Sergey Kaplun wrote:
<snipped>

+macro(AddBenchTarget perf_suite)
+  file(MAKE_DIRECTORY "${PERF_OUTPUT_DIR}/${perf_suite}/")
+  message(STATUS "Add perf suite ${perf_suite}")
+  add_custom_target(${perf_suite})
+  add_custom_target(${perf_suite}-console
+    COMMAND ${CMAKE_CTEST_COMMAND}
+      -L ${perf_suite}
use long option "--label-regex" instead "-L"
Fixed:
===================================================================
diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt
index 06603dfd..d03d74a0 100644
--- a/perf/CMakeLists.txt
+++ b/perf/CMakeLists.txt
@@ -47,7 +47,7 @@ macro(AddBenchTarget perf_suite)
   add_custom_target(${perf_suite})
   add_custom_target(${perf_suite}-console
     COMMAND ${CMAKE_CTEST_COMMAND}
-      -L ${perf_suite}
+      --label-regex ${perf_suite}
       --parallel 1
       --verbose
       --output-on-failure
===================================================================

+      --parallel 1
+      --verbose
+      --output-on-failure
+      --no-tests=error
+  )
+  add_dependencies(${perf_suite}-console luajit-main)
+endmacro()
+
+# Add the bench to the pair of targets created by the call above.
+macro(AddBench bench_name bench_path perf_suite LUA_PATH)
+  set(bench_title "perf/${perf_suite}/${bench_name}")
+  get_filename_component(bench_name_stripped  ${bench_name} NAME_WE)
+  set(bench_out_file
+    ${PERF_OUTPUT_DIR}/${perf_suite}/${bench_name_stripped}.json
+  )
+  set(bench_command "${LUAJIT_BINARY} ${bench_path}")
+  if(${ARGC} GREATER 4)
+    set(input_file ${ARGV4})
+    set(bench_command "${bench_command} < ${input_file}")
+  endif()
+  set(BENCH_FLAGS
+    "--benchmark_out_format=json --benchmark_out=${bench_out_file}"
Why BENCH_FLAGS is in uppercase and bench_command_flags in lower-case?

What is the difference?

Also, it seems these variables are unused and can be removed:

@ -56,10 +58,6 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH)
      set(input_file ${ARGV4})
      set(bench_command "${bench_command} < ${input_file}")
    endif()
-  set(BENCH_FLAGS
-    "--benchmark_out_format=json --benchmark_out=${bench_out_file}"
-  )
-  set(bench_command_flags ${bench_command} ${BENCH_FLAGS})
    separate_arguments(bench_command_separated UNIX_COMMAND 
${bench_command})
    add_custom_command(
      COMMAND ${CMAKE_COMMAND} -E env

Dropped, thanks!

===================================================================
diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt
index 5a5cf777..0916402e 100644
--- a/perf/CMakeLists.txt
+++ b/perf/CMakeLists.txt
@@ -63,10 +63,6 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH)
     set(input_file ${ARGV4})
     set(bench_command "${bench_command} < ${input_file}")
   endif()
-  set(BENCH_FLAGS
-    "--benchmark_out_format=json --benchmark_out=${bench_out_file}"
-  )
-  set(bench_command_flags ${bench_command} ${BENCH_FLAGS})
   separate_arguments(bench_command_separated UNIX_COMMAND ${bench_command})
   add_custom_command(
     COMMAND ${CMAKE_COMMAND} -E env
===================================================================

+  )
+  set(bench_command_flags ${bench_command} ${BENCH_FLAGS})
+  separate_arguments(bench_command_separated UNIX_COMMAND ${bench_command})
+  add_custom_command(
+    COMMAND ${CMAKE_COMMAND} -E env
+      LUA_PATH="${LUA_PATH}"
+      LUA_CPATH="${LUA_CPATH}"
+        ${bench_command_separated}
+          --benchmark_out_format=json
+          --benchmark_out="${bench_out_file}"
+    OUTPUT ${bench_out_file}
+    DEPENDS luajit-main
+    COMMENT
+      "Running benchmark ${bench_title} saving results in ${bench_out_file}."
+  )
+  add_custom_target(${bench_name} DEPENDS ${bench_out_file})
+  add_dependencies(${perf_suite} ${bench_name})
+
+  # Report in the console.
+  add_test(NAME ${bench_title}
+    COMMAND sh -c "${bench_command}"
I propose to use find_program for shell executable, like we do in 
tarantool's root cmakelists.txt:

--- a/perf/CMakeLists.txt
+++ b/perf/CMakeLists.txt
@@ -1,5 +1,7 @@
  # Running various bench suites against LuaJIT.

+find_program(SHELL sh)
+
  include(MakeLuaPath)

  if(CMAKE_BUILD_TYPE STREQUAL "Debug")
@@ -78,7 +80,7 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH)

    # Report in the console.
    add_test(NAME ${bench_title}
-    COMMAND sh -c "${bench_command}"
+    COMMAND ${SHELL} -c "${bench_command}"
    )
    set_tests_properties(${bench_title} PROPERTIES
      ENVIRONMENT "LUA_PATH=${LUA_PATH}"
Added:

===================================================================
diff --git a/perf/CMakeLists.txt b/perf/CMakeLists.txt
index 0916402e..e3af1996 100644
--- a/perf/CMakeLists.txt
+++ b/perf/CMakeLists.txt
@@ -2,6 +2,11 @@
 
 include(MakeLuaPath)
 
+find_program(SH sh)
+if(NOT SH)
+  message(FATAL_ERROR "`sh' is not found")
+endif()
+
 if(CMAKE_BUILD_TYPE STREQUAL "Debug")
   message(WARNING "LuaJIT and perf tests are built in the Debug mode. "
                   "Timings may be affected.")
@@ -81,7 +86,7 @@ macro(AddBench bench_name bench_path perf_suite LUA_PATH)
 
   # Report in the console.
   add_test(NAME ${bench_title}
-    COMMAND sh -c "${bench_command}"
+    COMMAND ${SH} -c "${bench_command}"
   )
   set_tests_properties(${bench_title} PROPERTIES
     ENVIRONMENT "LUA_PATH=${LUA_PATH}"
===================================================================

+  )
+  set_tests_properties(${bench_title} PROPERTIES
+    ENVIRONMENT "LUA_PATH=${LUA_PATH}"
+    LABELS ${perf_suite}
+    DEPENDS luajit-main
+  )
+  unset(input_file)
+endmacro()
+
<snipped>

--------------XRYOzR2GEdFiw0eaBIYXZtSi--