instrumentation: Prevent unnecessary query loading

Don't load queries from instrumentation directories when GetIsInTryCompile
or before ClearGeneratedQueries from previous configures has run.
This commit is contained in:
Martin Duffy
2025-06-09 11:19:44 -04:00
parent f26f127183
commit e01d12c14f
5 changed files with 46 additions and 21 deletions

View File

@@ -27,7 +27,10 @@
#include "cmUVProcessChain.h"
#include "cmValue.h"
cmInstrumentation::cmInstrumentation(std::string const& binary_dir)
using LoadQueriesAfter = cmInstrumentation::LoadQueriesAfter;
cmInstrumentation::cmInstrumentation(std::string const& binary_dir,
LoadQueriesAfter loadQueries)
{
std::string const uuid =
cmExperimental::DataForFeature(cmExperimental::Feature::Instrumentation)
@@ -40,7 +43,9 @@ cmInstrumentation::cmInstrumentation(std::string const& binary_dir)
this->userTimingDirv1 =
cmStrCat(configDir.value(), "/instrumentation-", uuid, "/v1");
}
this->LoadQueries();
if (loadQueries == LoadQueriesAfter::Yes) {
this->LoadQueries();
}
}
void cmInstrumentation::LoadQueries()
@@ -439,12 +444,12 @@ int cmInstrumentation::InstrumentCommand(
std::function<int()> const& callback,
cm::optional<std::map<std::string, std::string>> options,
cm::optional<std::map<std::string, std::string>> arrayOptions,
bool reloadQueriesAfterCommand)
LoadQueriesAfter reloadQueriesAfterCommand)
{
// Always begin gathering data for configure in case cmake_instrumentation
// command creates a query
if (!this->hasQuery && !reloadQueriesAfterCommand) {
if (!this->hasQuery && reloadQueriesAfterCommand == LoadQueriesAfter::No) {
return callback();
}
@@ -466,7 +471,7 @@ int cmInstrumentation::InstrumentCommand(
if (this->HasQuery(
cmInstrumentationQuery::Query::DynamicSystemInformation)) {
this->InsertDynamicSystemInformation(root, "before");
} else if (reloadQueriesAfterCommand) {
} else if (reloadQueriesAfterCommand == LoadQueriesAfter::Yes) {
this->GetDynamicSystemInformation(preConfigureMemory, preConfigureLoad);
}
@@ -475,7 +480,7 @@ int cmInstrumentation::InstrumentCommand(
root["result"] = ret;
// Exit early if configure didn't generate a query
if (reloadQueriesAfterCommand) {
if (reloadQueriesAfterCommand == LoadQueriesAfter::Yes) {
this->LoadQueries();
if (!this->HasQuery()) {
return ret;
@@ -634,7 +639,7 @@ int cmInstrumentation::CollectTimingAfterBuild(int ppid)
};
int ret = this->InstrumentCommand(
"build", {}, [waitForBuild]() { return waitForBuild(); }, cm::nullopt,
cm::nullopt, false);
cm::nullopt, LoadQueriesAfter::No);
this->CollectTimingData(cmInstrumentationQuery::Hook::PostBuild);
return ret;
}

View File

@@ -25,14 +25,21 @@
class cmInstrumentation
{
public:
cmInstrumentation(std::string const& binary_dir);
enum class LoadQueriesAfter
{
Yes,
No
};
cmInstrumentation(std::string const& binary_dir,
LoadQueriesAfter loadQueries = LoadQueriesAfter::Yes);
void LoadQueries();
int InstrumentCommand(
std::string command_type, std::vector<std::string> const& command,
std::function<int()> const& callback,
cm::optional<std::map<std::string, std::string>> options = cm::nullopt,
cm::optional<std::map<std::string, std::string>> arrayOptions =
cm::nullopt,
bool reloadQueriesAfterCommand = false);
LoadQueriesAfter reloadQueriesAfterCommand = LoadQueriesAfter::No);
std::string InstrumentTest(std::string const& name,
std::string const& command,
std::vector<std::string> const& args,
@@ -41,7 +48,6 @@ public:
std::chrono::system_clock::time_point systemStart,
std::string config);
void GetPreTestStats();
void LoadQueries();
bool HasQuery() const;
bool HasQuery(cmInstrumentationQuery::Query) const;
bool HasHook(cmInstrumentationQuery::Hook) const;

View File

@@ -2620,16 +2620,18 @@ int cmake::ActualConfigure()
this->FileAPI = cm::make_unique<cmFileAPI>(this);
this->FileAPI->ReadQueries();
this->Instrumentation = cm::make_unique<cmInstrumentation>(
this->State->GetBinaryDirectory(),
cmInstrumentation::LoadQueriesAfter::No);
this->Instrumentation->ClearGeneratedQueries();
if (!this->GetIsInTryCompile()) {
this->TruncateOutputLog("CMakeConfigureLog.yaml");
this->ConfigureLog = cm::make_unique<cmConfigureLog>(
cmStrCat(this->GetHomeOutputDirectory(), "/CMakeFiles"_s),
this->FileAPI->GetConfigureLogVersions());
this->Instrumentation->LoadQueries();
}
this->Instrumentation =
cm::make_unique<cmInstrumentation>(this->State->GetBinaryDirectory());
this->Instrumentation->ClearGeneratedQueries();
#endif
// actually do the configure
@@ -2645,7 +2647,9 @@ int cmake::ActualConfigure()
};
int ret = this->Instrumentation->InstrumentCommand(
"configure", this->cmdArgs, [doConfigure]() { return doConfigure(); },
cm::nullopt, cm::nullopt, true);
cm::nullopt, cm::nullopt,
this->GetIsInTryCompile() ? cmInstrumentation::LoadQueriesAfter::No
: cmInstrumentation::LoadQueriesAfter::Yes);
if (ret != 0) {
return ret;
}
@@ -2688,7 +2692,6 @@ int cmake::ActualConfigure()
}
// Setup launchers for instrumentation
#if !defined(CMAKE_BOOTSTRAP)
this->Instrumentation->LoadQueries();
if (this->Instrumentation->HasQuery()) {
std::string launcher;
if (mf->IsOn("CTEST_USE_LAUNCHERS")) {
@@ -3046,7 +3049,6 @@ int cmake::Generate()
return 0;
};
this->Instrumentation->LoadQueries();
int ret = this->Instrumentation->InstrumentCommand(
"generate", this->cmdArgs, [doGenerate]() { return doGenerate(); });
if (ret != 0) {

View File

@@ -6,7 +6,7 @@ function(instrument test)
set(config "${CMAKE_CURRENT_LIST_DIR}/config")
set(ENV{CMAKE_CONFIG_DIR} ${config})
cmake_parse_arguments(ARGS
"BUILD;BUILD_MAKE_PROGRAM;INSTALL;TEST;COPY_QUERIES;NO_WARN;STATIC_QUERY;DYNAMIC_QUERY;INSTALL_PARALLEL;MANUAL_HOOK"
"BUILD;BUILD_MAKE_PROGRAM;INSTALL;TEST;COPY_QUERIES;COPY_QUERIES_GENERATED;NO_WARN;STATIC_QUERY;DYNAMIC_QUERY;INSTALL_PARALLEL;MANUAL_HOOK"
"CHECK_SCRIPT;CONFIGURE_ARG" "" ${ARGN})
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test})
set(uuid "a37d1069-1972-4901-b9c9-f194aaf2b6e0")
@@ -34,14 +34,18 @@ function(instrument test)
list(APPEND ARGS_CONFIGURE_ARG "-DINSTRUMENT_COMMAND_FILE=${cmake_file}")
endif()
# Configure generated query files to compare CMake output
set(copy_loc ${RunCMake_TEST_BINARY_DIR}/query)
if (ARGS_COPY_QUERIES_GENERATED)
set(ARGS_COPY_QUERIES TRUE)
set(copy_loc ${v1}/query/generated) # Copied files here should be cleared on configure
endif()
if (ARGS_COPY_QUERIES)
file(MAKE_DIRECTORY ${RunCMake_TEST_BINARY_DIR}/query)
file(MAKE_DIRECTORY ${copy_loc})
set(generated_queries "0;1;2")
foreach(n IN LISTS generated_queries)
configure_file(
"${query_dir}/generated/query-${n}.json.in"
"${RunCMake_TEST_BINARY_DIR}/query/query-${n}.json"
"${copy_loc}/query-${n}.json"
)
endforeach()
endif()
@@ -118,6 +122,10 @@ instrument(cmake-command-bad-arg NO_WARN)
instrument(cmake-command-parallel-install
BUILD INSTALL TEST NO_WARN INSTALL_PARALLEL DYNAMIC_QUERY
CHECK_SCRIPT check-data-dir.cmake)
instrument(cmake-command-resets-generated NO_WARN
COPY_QUERIES_GENERATED
CHECK_SCRIPT check-data-dir.cmake
)
# FIXME(#26668) This does not work on Windows
if (UNIX)

View File

@@ -0,0 +1,4 @@
cmake_instrumentation(
API_VERSION 1
DATA_VERSION 1
)