forked from catchorg/Catch2
Optimize adding test commands in catch_discover_tests
The previous approach was for `add_command` to behave as append function via concatenating the command string internally and then saving it into `PARENT_SCOPE`. Because this in practice ended up meaning concatenating a copy of the string inside the function and then overwriting the string outside the function, the performance was lacking. The new approach is for `prepare_command` to only escape & return the command from single call, and the caller is responsible for concatenating the result. Since the actual concatenation no longer crosses function scope boundaries, the performance is much better, even though the scaling is still quadratic. The new approach only takes 1/4-1/5 of the time, saving ~1s at 1k tests, 4.5s at 2k tests and 19s at 4k tests.
This commit is contained in:
Martin Hořeňovský
@@ -1,7 +1,21 @@
|
||||
# Distributed under the OSI-approved BSD 3-Clause License. See accompanying
|
||||
# file Copyright.txt or https://cmake.org/licensing for details.
|
||||
|
||||
function(add_command NAME)
|
||||
# TBD: Further possible optimization is that most arguments for per-test
|
||||
# `prepare_command` call are constant across one invocation of
|
||||
# `catch_discover_tests`, and thus need checking and escaping only
|
||||
# once, instead of for each test.
|
||||
# This would provide nice speed-up of the actual command preparation,
|
||||
# but it will make the script much harder to read, and it is utterly
|
||||
# dwarfed by the quadratic scaling of parsing JSON arrays in CMake.
|
||||
|
||||
|
||||
# Prepare command with escaped (bracketed) arguments and return it via `_Command` out variable.
|
||||
#
|
||||
# To avoid quadratic performance when concatenating all commands together,
|
||||
# the actual concatenation must be done by the caller, by appending it
|
||||
# into a string of all other commands.
|
||||
function(prepare_command NAME)
|
||||
set(_args "")
|
||||
# use ARGV* instead of ARGN, because ARGN splits arrays into multiple arguments
|
||||
math(EXPR _last_arg ${ARGC}-1)
|
||||
@@ -13,7 +27,7 @@ function(add_command NAME)
|
||||
set(_args "${_args} ${_arg}")
|
||||
endif()
|
||||
endforeach()
|
||||
set(script "${script}${NAME}(${_args})\n" PARENT_SCOPE)
|
||||
set(_Command "${NAME}(${_args})\n" PARENT_SCOPE)
|
||||
endfunction()
|
||||
|
||||
# Generates random filename in the temp folder.
|
||||
@@ -220,7 +234,7 @@ function(catch_discover_tests_impl)
|
||||
endif()
|
||||
|
||||
# ...and add to script
|
||||
add_command(add_test
|
||||
prepare_command(add_test
|
||||
"${prefix}${plain_name}${suffix}"
|
||||
${_TEST_EXECUTOR}
|
||||
"${_TEST_EXECUTABLE}"
|
||||
@@ -229,12 +243,14 @@ function(catch_discover_tests_impl)
|
||||
"${reporter_arg}"
|
||||
"${output_dir_arg}"
|
||||
)
|
||||
add_command(set_tests_properties
|
||||
string(APPEND script "${_Command}")
|
||||
prepare_command(set_tests_properties
|
||||
"${prefix}${plain_name}${suffix}"
|
||||
PROPERTIES
|
||||
WORKING_DIRECTORY "${_TEST_WORKING_DIR}"
|
||||
${properties}
|
||||
)
|
||||
string(APPEND script "${_Command}")
|
||||
|
||||
if(add_tags)
|
||||
string(JSON num_tags LENGTH "${test_tags}")
|
||||
@@ -254,19 +270,21 @@ function(catch_discover_tests_impl)
|
||||
list(APPEND tag_list "${a_tag}")
|
||||
endforeach()
|
||||
|
||||
add_command(set_tests_properties
|
||||
prepare_command(set_tests_properties
|
||||
"${prefix}${plain_name}${suffix}"
|
||||
PROPERTIES
|
||||
LABELS "${tag_list}"
|
||||
)
|
||||
string(APPEND script "${_Command}")
|
||||
endif()
|
||||
endif(add_tags)
|
||||
|
||||
if(environment_modifications)
|
||||
add_command(set_tests_properties
|
||||
prepare_command(set_tests_properties
|
||||
"${prefix}${plain_name}${suffix}"
|
||||
PROPERTIES
|
||||
ENVIRONMENT_MODIFICATION "${environment_modifications}")
|
||||
string(APPEND script "${_Command}")
|
||||
endif()
|
||||
|
||||
list(APPEND tests "${prefix}${plain_name}${suffix}")
|
||||
@@ -274,13 +292,16 @@ function(catch_discover_tests_impl)
|
||||
|
||||
# Create a list of all discovered tests, which users may use to e.g. set
|
||||
# properties on the tests
|
||||
add_command(set ${_TEST_LIST} ${tests})
|
||||
prepare_command(set ${_TEST_LIST} ${tests})
|
||||
string(APPEND script "${_Command}")
|
||||
|
||||
# Write CTest script
|
||||
file(WRITE "${_CTEST_FILE}" "${script}")
|
||||
endfunction()
|
||||
|
||||
if(CMAKE_SCRIPT_MODE_FILE)
|
||||
# To enable `include`ing this file in the unit test scripts, we only run
|
||||
# the impl if an actual `TEST_EXECUTABLE` is provided.
|
||||
if(CMAKE_SCRIPT_MODE_FILE AND DEFINED TEST_EXECUTABLE)
|
||||
catch_discover_tests_impl(
|
||||
TEST_EXECUTABLE ${TEST_EXECUTABLE}
|
||||
TEST_EXECUTOR ${TEST_EXECUTOR}
|
||||
|
||||
@@ -625,6 +625,13 @@ if(CATCH_ENABLE_CMAKE_HELPER_TESTS)
|
||||
COST 240
|
||||
LABELS "uses-python"
|
||||
)
|
||||
|
||||
add_test(NAME "CMakeHelper::PrepareCommand"
|
||||
COMMAND
|
||||
"${CMAKE_COMMAND}"
|
||||
"-DCATCH_ADD_TESTS_SCRIPT=${CATCH_DIR}/extras/CatchAddTests.cmake"
|
||||
-P "${CMAKE_CURRENT_LIST_DIR}/TestScripts/DiscoverTests/TestPrepareCommand.cmake"
|
||||
)
|
||||
endif()
|
||||
|
||||
foreach(reporterName # "Automake" - the simple .trs format does not support any kind of comments/metadata
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
# SPDX-License-Identifier: BSL-1.0
|
||||
|
||||
# Unit tests for `prepare_command` helper in `extras/CatchAddTests.cmake`.
|
||||
#
|
||||
# Yes, we are at the stage where the script helpers need unit tests.
|
||||
#
|
||||
# Run as
|
||||
# cmake -DCATCH_ADD_TESTS_SCRIPT=/path/to/extras/CatchAddTests.cmake \
|
||||
# -P TestPrepareCommand.cmake
|
||||
|
||||
|
||||
cmake_minimum_required(VERSION 3.19)
|
||||
|
||||
if(NOT DEFINED CATCH_ADD_TESTS_SCRIPT)
|
||||
message(FATAL_ERROR "Missing argument `CATCH_ADD_TESTS_SCRIPT`")
|
||||
endif()
|
||||
|
||||
if(NOT EXISTS "${CATCH_ADD_TESTS_SCRIPT}")
|
||||
message(FATAL_ERROR "Cannot find CatchAddTests.cmake at '${CATCH_ADD_TESTS_SCRIPT}'")
|
||||
endif()
|
||||
|
||||
# Pull in the helper functions. Without `TEST_EXECUTABLE` being defined,
|
||||
# `catch_discover_tests_impl` is not called.
|
||||
include("${CATCH_ADD_TESTS_SCRIPT}")
|
||||
|
||||
set(_failures 0)
|
||||
|
||||
function(expect_equal description actual expected)
|
||||
if(actual STREQUAL expected)
|
||||
message(" [PASS] ${description}")
|
||||
else()
|
||||
message(" [FAIL] ${description}")
|
||||
message(" expected: [${expected}]")
|
||||
message(" actual: [${actual}]")
|
||||
math(EXPR _n "${_failures} + 1")
|
||||
set(_failures "${_n}" PARENT_SCOPE)
|
||||
endif()
|
||||
endfunction()
|
||||
|
||||
prepare_command(add_test SimpleName /path/to/tests)
|
||||
expect_equal("Simple arg, no quotes" "${_Command}" "add_test( SimpleName /path/to/tests)\n")
|
||||
|
||||
prepare_command(add_test "Name with spaces")
|
||||
expect_equal("Spaces in arg, needs quotes" "${_Command}" "add_test( [==[Name with spaces]==])\n")
|
||||
|
||||
prepare_command(set_tests_properties Foo PROPERTIES LABELS "tagA\;tagB\;tagC")
|
||||
expect_equal("semicolons in argument are kept and quoted"
|
||||
"${_Command}" "set_tests_properties( Foo PROPERTIES LABELS [==[tagA\;tagB\;tagC]==])\n")
|
||||
|
||||
set(_Command "PRE-EXISTING")
|
||||
prepare_command(set_tests_properties Foo PROPERTIES BAR baz)
|
||||
expect_equal("_Command var does no accumulate commands"
|
||||
"${_Command}" "set_tests_properties( Foo PROPERTIES BAR baz)\n")
|
||||
|
||||
|
||||
|
||||
if(_failures GREATER 0)
|
||||
message(FATAL_ERROR "${_failures} prepare_command test(s) failed")
|
||||
else()
|
||||
message(STATUS "All prepare_command tests passed")
|
||||
endif()
|
||||
Reference in New Issue
Block a user