<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey!</p>
<p>LGTM</p>
<div class="moz-cite-prefix">On 1/14/26 11:50, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite" cite="mid:aWdY255lKRAdui3C@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Fixed your comments, see the iterative patches below.
On 13.01.26, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
thanks for the patch! LGTM in general, see my comments below.
Sergey
On 12/26/25 12:18, Sergey Kaplun wrote:
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+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}
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">use long option "--label-regex" instead "-L"
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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
===================================================================
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ --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}"
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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
===================================================================
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ )
+ 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}"
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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<a class="moz-txt-link-rfc2396E" href="mailto:)@@-78,7+80,7@@macro(AddBenchbench_namebench_pathperf_suiteLUA_PATH) #Reportintheconsole. add_test(NAME${bench_title}- COMMANDsh-c">")
@@ -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 "</a>${bench_command}"
+ COMMAND ${SHELL} -c "${bench_command}"
)
set_tests_properties(${bench_title} PROPERTIES
ENVIRONMENT "LUA_PATH=${LUA_PATH}"
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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.<a class="moz-txt-link-rfc2396E" href="mailto:)@@-81,7+86,7@@macro(AddBenchbench_namebench_pathperf_suiteLUA_PATH)#Reportintheconsole.add_test(NAME${bench_title}-COMMANDsh-c">")
@@ -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 "</a>${bench_command}"
+ COMMAND ${SH} -c "${bench_command}"
)
set_tests_properties(${bench_title} PROPERTIES
ENVIRONMENT "LUA_PATH=${LUA_PATH}"
===================================================================
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ )
+ set_tests_properties(${bench_title} PROPERTIES
+ ENVIRONMENT "LUA_PATH=${LUA_PATH}"
+ LABELS ${perf_suite}
+ DEPENDS luajit-main
+ )
+ unset(input_file)
+endmacro()
+
</pre>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>