Преглед изворни кода

instrumentation: Prevent unnecessary query loading

Don't load queries from instrumentation directories when GetIsInTryCompile
or before ClearGeneratedQueries from previous configures has run.
Martin Duffy пре 7 месеци
родитељ
комит
e01d12c14f

+ 12 - 7
Source/cmInstrumentation.cxx

@@ -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;
 }

+ 9 - 3
Source/cmInstrumentation.h

@@ -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;

+ 9 - 7
Source/cmake.cxx

@@ -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) {

+ 12 - 4
Tests/RunCMake/Instrumentation/RunCMakeTest.cmake

@@ -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)

+ 4 - 0
Tests/RunCMake/Instrumentation/query/cmake-command-resets-generated.cmake

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