[libcxx-commits] [libcxx] [llvm] [openmp] [lit] Add a flag to disable lit time tests (PR #98270)

Vincent Lee via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 9 22:02:31 PDT 2024


https://github.com/thevinster created https://github.com/llvm/llvm-project/pull/98270

LLVM lit assumes control of the test parallelism when running a test suite. This style of testing doesn't play nicely with build systems like Buck or Bazel since it prefers finer grained actions on a per-test level. In order for external build systems to control the test parallelism, add an option to disable `.lit_test_times.txt` under the `--no-time-tests` flag, thus allowing other build systems to determine the parallelism and avoid race conditions when writing to that file. I went for `--no-time-tests` instead of `--time-tests` in order to preserve the original functionality of writing to `.lit_test_times.txt` as the default behavior and only opt-in for those who do _not_ want `.lit_test_times.txt` file. 

An additional change has been made to refactor the original `--time-tests` flag to `--time-tests-histogram` to better reflect the original functionality. This is technically a breaking API change since external users who build LLVM out-of-tree can fail, but I've made sure all the uses within LLVM have been switched to use the new flag.

>From b948e54960de38985defaae6c2738be4f1b94754 Mon Sep 17 00:00:00 2001
From: Vincent Lee <leevince at fb.com>
Date: Tue, 9 Jul 2024 18:56:46 -0700
Subject: [PATCH] [lit] Add a flag to disable lit time tests

---
 .ci/monolithic-linux.sh            |  4 ++--
 .ci/monolithic-windows.sh          |  2 +-
 libcxx/utils/ci/run-buildbot       |  4 ++--
 llvm/docs/CommandGuide/lit.rst     |  7 ++++++-
 llvm/utils/lit/lit/cl_arguments.py | 16 +++++++++++-----
 llvm/utils/lit/lit/main.py         |  5 +++--
 offload/cmake/OpenMPTesting.cmake  |  2 +-
 openmp/cmake/OpenMPTesting.cmake   |  2 +-
 8 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index b78dc59432b65..04395621a8f27 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -47,7 +47,7 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \
       -D LLVM_ENABLE_ASSERTIONS=ON \
       -D LLVM_BUILD_EXAMPLES=ON \
       -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
-      -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests" \
+      -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests-histogram" \
       -D LLVM_ENABLE_LLD=ON \
       -D CMAKE_CXX_FLAGS=-gmlt \
       -D LLVM_CCACHE_BUILD=ON \
@@ -124,6 +124,6 @@ if [[ "${runtimes}" != "" ]]; then
       -D LIBCXXABI_TEST_PARAMS="enable_modules=clang"
 
   echo "--- ninja runtimes clang modules"
-  
+
   ninja -vC "${RUNTIMES_BUILD_DIR}" ${runtime_targets}
 fi
diff --git a/.ci/monolithic-windows.sh b/.ci/monolithic-windows.sh
index 91e719c52d436..b64856af044be 100755
--- a/.ci/monolithic-windows.sh
+++ b/.ci/monolithic-windows.sh
@@ -53,7 +53,7 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \
       -D LLVM_ENABLE_ASSERTIONS=ON \
       -D LLVM_BUILD_EXAMPLES=ON \
       -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
-      -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests" \
+      -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests-histogram" \
       -D COMPILER_RT_BUILD_ORC=OFF \
       -D CMAKE_C_COMPILER_LAUNCHER=sccache \
       -D CMAKE_CXX_COMPILER_LAUNCHER=sccache \
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index f1c20b9d72190..a59c5930b5252 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -119,7 +119,7 @@ function generate-cmake-base() {
           -DLIBCXX_ENABLE_WERROR=YES \
           -DLIBCXXABI_ENABLE_WERROR=YES \
           -DLIBUNWIND_ENABLE_WERROR=YES \
-          -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests" \
+          -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests-histogram" \
           "${@}"
 }
 
@@ -381,7 +381,7 @@ bootstrapping-build)
           -DLLVM_TARGETS_TO_BUILD="host" \
           -DRUNTIMES_BUILD_ALLOW_DARWIN=ON \
           -DLLVM_ENABLE_ASSERTIONS=ON \
-          -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests"
+          -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests-histogram"
 
     echo "+++ Running the LLDB libc++ data formatter tests"
     ${NINJA} -vC "${BUILD_DIR}" check-lldb-api-functionalities-data-formatter-data-formatter-stl-libcxx
diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst
index 799ee34e9f9ff..c63f679e74a4b 100644
--- a/llvm/docs/CommandGuide/lit.rst
+++ b/llvm/docs/CommandGuide/lit.rst
@@ -151,7 +151,12 @@ EXECUTION OPTIONS
  feature that can be used to conditionally disable (or expect failure in)
  certain tests.
 
-.. option:: --time-tests
+.. option:: --no-time-tests
+
+ Disable tracking the wall time individual tests take to execute. This is useful
+ for external build systems to orchestrate the scheduled tests.
+
+.. option:: --time-tests-histogram
 
  Track the wall time individual tests take to execute and includes the results
  in the summary output.  This is useful for determining which tests in a test
diff --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py
index b9122d07afd8a..8df76ef4b8c82 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -154,11 +154,6 @@ def parse_args():
         action="append",
         default=[],
     )
-    execution_group.add_argument(
-        "--time-tests",
-        help="Track elapsed wall time for each test",
-        action="store_true",
-    )
     execution_group.add_argument(
         "--no-execute",
         dest="noExecute",
@@ -209,6 +204,17 @@ def parse_args():
         action="store_true",
         help="Exit with status zero even if some tests fail",
     )
+    execution_test_time_group = execution_group.add_mutually_exclusive_group()
+    execution_test_time_group.add_argument(
+        "--no-time-tests",
+        help="Do not track elapsed wall time for each test",
+        action="store_true",
+    )
+    execution_test_time_group.add_argument(
+        "--time-tests-histogram",
+        help="Track elapsed wall time for each test in a histogram",
+        action="store_true",
+    )
 
     selection_group = parser.add_argument_group("Test Selection")
     selection_group.add_argument(
diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index db9f24f748d9e..8cc784dbf6471 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -124,13 +124,14 @@ def main(builtin_params={}):
     run_tests(selected_tests, lit_config, opts, len(discovered_tests))
     elapsed = time.time() - start
 
-    record_test_times(selected_tests, lit_config)
+    if not opts.no_time_tests or opts.time_tests_histogram:
+        record_test_times(selected_tests, lit_config)
 
     selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
         selected_tests, discovered_tests
     )
 
-    if opts.time_tests:
+    if opts.time_tests_histogram:
         print_histogram(discovered_tests)
 
     print_results(discovered_tests, elapsed, opts)
diff --git a/offload/cmake/OpenMPTesting.cmake b/offload/cmake/OpenMPTesting.cmake
index 11eafeb764260..fa767bb291d8a 100644
--- a/offload/cmake/OpenMPTesting.cmake
+++ b/offload/cmake/OpenMPTesting.cmake
@@ -58,7 +58,7 @@ if (${OPENMP_STANDALONE_BUILD})
     set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --no-progress-bar")
   endif()
   if (${CMAKE_SYSTEM_NAME} MATCHES "AIX")
-    set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --time-tests --timeout=1800")
+    set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --time-tests-histogram --timeout=1800")
   endif()
   set(OPENMP_LIT_ARGS "${DEFAULT_LIT_ARGS}" CACHE STRING "Options for lit.")
   separate_arguments(OPENMP_LIT_ARGS)
diff --git a/openmp/cmake/OpenMPTesting.cmake b/openmp/cmake/OpenMPTesting.cmake
index c67ad8b1cbd9c..fb58cad04c019 100644
--- a/openmp/cmake/OpenMPTesting.cmake
+++ b/openmp/cmake/OpenMPTesting.cmake
@@ -58,7 +58,7 @@ if (${OPENMP_STANDALONE_BUILD})
     set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --no-progress-bar")
   endif()
   if (${CMAKE_SYSTEM_NAME} MATCHES "AIX")
-    set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --time-tests --timeout=3000")
+    set(DEFAULT_LIT_ARGS "${DEFAULT_LIT_ARGS} --time-tests-histogram --timeout=3000")
   endif()
   set(OPENMP_LIT_ARGS "${DEFAULT_LIT_ARGS}" CACHE STRING "Options for lit.")
   separate_arguments(OPENMP_LIT_ARGS)



More information about the libcxx-commits mailing list