cmake: Correctly handle cost data of tests with arbitrary chars in name

ctest automatically optimizes the order of (parallel) test execution based on
historic test case runtime via the COST property (see [0]), which can have a
significant impact on overall test run times. Sadly this feature is broken in
CMake < 4.0.0 for test cases that have spaces in their name (see [1]).

This commit backports the upstream fix. As repeated test runs are expected to
mainly take place inside the SDK, the patch is only applied to 'nativesdk'
builds.

[0]: https://cmake.org/cmake/help/latest/prop_test/COST.html
[1]: https://gitlab.kitware.com/cmake/cmake/-/issues/26594

Reported-By: John Drouhard <john@drouhard.dev>
(From OE-Core rev: ead0d06d6335fb74c1ae0dc7bdcf414c66b3e4ab)

Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit dcbaf42dd74cc0bda7254856589613718ed3f057)
Signed-off-by: Steve Sakoman <steve@sakoman.com>
This commit is contained in:
Moritz Haase 2025-06-17 11:24:56 +02:00 committed by Steve Sakoman
parent 747bddc1ab
commit 1fe29dbf01
3 changed files with 204 additions and 1 deletions

View File

@ -51,7 +51,7 @@ do_compile() {
do_install() {
oe_runmake 'DESTDIR=${D}' install
# The following codes are here because eSDK needs to provide compatibilty
# The following codes are here because eSDK needs to provide compatibility
# for SDK. That is, eSDK could also be used like traditional SDK.
mkdir -p ${D}${datadir}/cmake
install -m 644 ${UNPACKDIR}/OEToolchainConfig.cmake ${D}${datadir}/cmake/

View File

@ -0,0 +1,202 @@
From c7e8b03324883760a2d6fab86ae034beb82af651 Mon Sep 17 00:00:00 2001
From: John Drouhard <john@drouhard.dev>
Date: Thu, 9 Jan 2025 20:34:42 -0600
Subject: [PATCH] ctest: Allow arbitrary characters in test names of
CTestCostData.txt
This changes the way lines in CTestCostData.txt are parsed to allow for
spaces in the test name.
It does so by looking for space characters from the end; and once two
have been found, assumes everything from the beginning up to that
second-to-last-space is the test name.
Additionally, parsing the file should be much more efficient since there
is no string or vector heap allocation per line. The std::string used by
the parse function to convert the int and float should be within most
standard libraries' small string optimization.
Fixes: #26594
Upstream-Status: Backport [4.0.0, 040da7d83216ace59710407e8ce35d5fd38e1340]
Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
---
Source/CTest/cmCTestMultiProcessHandler.cxx | 77 +++++++++++++++------
Source/CTest/cmCTestMultiProcessHandler.h | 3 +-
Tests/CTestTestScheduler/CMakeLists.txt | 4 +-
3 files changed, 61 insertions(+), 23 deletions(-)
diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx
index 84ea32b84d40025ec333a90d30c42eeaf7adc9ef..231e7b5f39b1d8aa75f4e59a890a099b53fcdaac 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.cxx
+++ b/Source/CTest/cmCTestMultiProcessHandler.cxx
@@ -20,6 +20,7 @@
#include <cm/memory>
#include <cm/optional>
+#include <cm/string_view>
#include <cmext/algorithm>
#include <cm3p/json/value.h>
@@ -52,6 +53,48 @@ constexpr unsigned long kParallelLevelMinimum = 2u;
// Under a job server, parallelism is effectively limited
// only by available job server tokens.
constexpr unsigned long kParallelLevelUnbounded = 0x10000u;
+
+struct CostEntry
+{
+ cm::string_view name;
+ int prevRuns;
+ float cost;
+};
+
+cm::optional<CostEntry> splitCostLine(cm::string_view line)
+{
+ std::string part;
+ cm::string_view::size_type pos1 = line.size();
+ cm::string_view::size_type pos2 = line.find_last_of(' ', pos1);
+ auto findNext = [line, &part, &pos1, &pos2]() -> bool {
+ if (pos2 != cm::string_view::npos) {
+ cm::string_view sub = line.substr(pos2 + 1, pos1 - pos2 - 1);
+ part.assign(sub.begin(), sub.end());
+ pos1 = pos2;
+ if (pos1 > 0) {
+ pos2 = line.find_last_of(' ', pos1 - 1);
+ }
+ return true;
+ }
+ return false;
+ };
+
+ // parse the cost
+ if (!findNext()) {
+ return cm::nullopt;
+ }
+ float cost = static_cast<float>(atof(part.c_str()));
+
+ // parse the previous runs
+ if (!findNext()) {
+ return cm::nullopt;
+ }
+ int prev = atoi(part.c_str());
+
+ // from start to the last found space is the name
+ return CostEntry{ line.substr(0, pos1), prev, cost };
+}
+
}
namespace cmsys {
@@ -797,24 +840,21 @@ void cmCTestMultiProcessHandler::UpdateCostData()
if (line == "---") {
break;
}
- std::vector<std::string> parts = cmSystemTools::SplitString(line, ' ');
// Format: <name> <previous_runs> <avg_cost>
- if (parts.size() < 3) {
+ cm::optional<CostEntry> entry = splitCostLine(line);
+ if (!entry) {
break;
}
- std::string name = parts[0];
- int prev = atoi(parts[1].c_str());
- float cost = static_cast<float>(atof(parts[2].c_str()));
-
- int index = this->SearchByName(name);
+ int index = this->SearchByName(entry->name);
if (index == -1) {
// This test is not in memory. We just rewrite the entry
- fout << name << " " << prev << " " << cost << "\n";
+ fout << entry->name << " " << entry->prevRuns << " " << entry->cost
+ << "\n";
} else {
// Update with our new average cost
- fout << name << " " << this->Properties[index]->PreviousRuns << " "
- << this->Properties[index]->Cost << "\n";
+ fout << entry->name << " " << this->Properties[index]->PreviousRuns
+ << " " << this->Properties[index]->Cost << "\n";
temp.erase(index);
}
}
@@ -850,28 +890,25 @@ void cmCTestMultiProcessHandler::ReadCostData()
break;
}
- std::vector<std::string> parts = cmSystemTools::SplitString(line, ' ');
+ // Format: <name> <previous_runs> <avg_cost>
+ cm::optional<CostEntry> entry = splitCostLine(line);
// Probably an older version of the file, will be fixed next run
- if (parts.size() < 3) {
+ if (!entry) {
fin.close();
return;
}
- std::string name = parts[0];
- int prev = atoi(parts[1].c_str());
- float cost = static_cast<float>(atof(parts[2].c_str()));
-
- int index = this->SearchByName(name);
+ int index = this->SearchByName(entry->name);
if (index == -1) {
continue;
}
- this->Properties[index]->PreviousRuns = prev;
+ this->Properties[index]->PreviousRuns = entry->prevRuns;
// When not running in parallel mode, don't use cost data
if (this->GetParallelLevel() > 1 && this->Properties[index] &&
this->Properties[index]->Cost == 0) {
- this->Properties[index]->Cost = cost;
+ this->Properties[index]->Cost = entry->cost;
}
}
// Next part of the file is the failed tests
@@ -884,7 +921,7 @@ void cmCTestMultiProcessHandler::ReadCostData()
}
}
-int cmCTestMultiProcessHandler::SearchByName(std::string const& name)
+int cmCTestMultiProcessHandler::SearchByName(cm::string_view name)
{
int index = -1;
diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h
index fd6c17f2fac06949c20f3792dd3eae442b15850b..811be613c3387240c0181f8372b24cf09219621f 100644
--- a/Source/CTest/cmCTestMultiProcessHandler.h
+++ b/Source/CTest/cmCTestMultiProcessHandler.h
@@ -13,6 +13,7 @@
#include <vector>
#include <cm/optional>
+#include <cm/string_view>
#include "cmCTest.h"
#include "cmCTestResourceAllocator.h"
@@ -110,7 +111,7 @@ protected:
void UpdateCostData();
void ReadCostData();
// Return index of a test based on its name
- int SearchByName(std::string const& name);
+ int SearchByName(cm::string_view name);
void CreateTestCostList();
diff --git a/Tests/CTestTestScheduler/CMakeLists.txt b/Tests/CTestTestScheduler/CMakeLists.txt
index 6f8cb4dbc0de35984540e1868788e0a02124e819..daf6ce2b23d8c048334ae1047759130b246dccef 100644
--- a/Tests/CTestTestScheduler/CMakeLists.txt
+++ b/Tests/CTestTestScheduler/CMakeLists.txt
@@ -1,9 +1,9 @@
-cmake_minimum_required(VERSION 3.10)
+cmake_minimum_required(VERSION 3.19)
project (CTestTestScheduler)
include (CTest)
add_executable (Sleep sleep.c)
foreach (time RANGE 1 4)
- add_test (TestSleep${time} Sleep ${time})
+ add_test ("TestSleep ${time}" Sleep ${time})
endforeach ()

View File

@ -5,6 +5,7 @@ inherit cmake bash-completion
DEPENDS += "curl expat zlib libarchive xz ncurses bzip2"
SRC_URI:append:class-nativesdk = " \
file://0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch \
file://OEToolchainConfig.cmake \
file://SDKToolchainConfig.cmake.template \
file://cmake-setup.py \