instrumentation: Refactor cmInstrumentation constructor and usage

Creates a global cmInstrumentation pointer on the CMake Instance to
prevent creating multiple instrumentation objects.

Source/CTest/cmCTestLaunch.cxx

@@ -254,7 +254,7 @@ void cmCTestLaunch::RunChild()
 
 int cmCTestLaunch::Run()
 {
-  auto instrumenter = cmInstrumentation(this->Reporter.OptionBuildDir);
+  auto instrumentation = cmInstrumentation(this->Reporter.OptionBuildDir);
   std::map<std::string, std::string> options;
   options["target"] = this->Reporter.OptionTargetName;
   options["source"] = this->Reporter.OptionSource;
@@ -264,7 +264,7 @@ int cmCTestLaunch::Run()
   std::map<std::string, std::string> arrayOptions;
   arrayOptions["outputs"] = this->Reporter.OptionOutput;
   arrayOptions["targetLabels"] = this->Reporter.OptionTargetLabels;
-  instrumenter.InstrumentCommand(
+  instrumentation.InstrumentCommand(
     this->Reporter.OptionCommandType, this->RealArgV,
     [this]() -> int {
       this->RunChild();

Source/cmInstrumentation.cxx

@@ -23,8 +23,7 @@
 #include "cmSystemTools.h"
 #include "cmTimestamp.h"
 
-cmInstrumentation::cmInstrumentation(std::string const& binary_dir,
+cmInstrumentation::cmInstrumentation(std::string const& binary_dir)
-                                     bool clear_generated)
 {
   std::string const uuid =
     cmExperimental::DataForFeature(cmExperimental::Feature::Instrumentation)
@@ -32,9 +31,6 @@ cmInstrumentation::cmInstrumentation(std::string const& binary_dir,
   this->binaryDir = binary_dir;
   this->timingDirv1 =
     cmStrCat(this->binaryDir, "/.cmake/instrumentation-", uuid, "/v1");
-  if (clear_generated) {
-    this->ClearGeneratedQueries();
-  }
   if (cm::optional<std::string> configDir =
         cmSystemTools::GetCMakeConfigDirectory()) {
     this->userTimingDirv1 =
@@ -57,24 +53,6 @@ void cmInstrumentation::LoadQueries()
   }
 }
 
-cmInstrumentation::cmInstrumentation(
-  std::string const& binary_dir,
-  std::set<cmInstrumentationQuery::Query>& queries_,
-  std::set<cmInstrumentationQuery::Hook>& hooks_, std::string& callback)
-{
-  this->binaryDir = binary_dir;
-  this->timingDirv1 = cmStrCat(
-    this->binaryDir, "/.cmake/instrumentation-",
-    cmExperimental::DataForFeature(cmExperimental::Feature::Instrumentation)
-      .Uuid,
-    "/v1");
-  this->queries = queries_;
-  this->hooks = hooks_;
-  if (!callback.empty()) {
-    this->callbacks.push_back(callback);
-  }
-}
-
 bool cmInstrumentation::ReadJSONQueries(std::string const& directory)
 {
   cmsys::Directory d;
@@ -99,20 +77,22 @@ void cmInstrumentation::ReadJSONQuery(std::string const& file)
                  this->callbacks);
 }
 
-void cmInstrumentation::WriteJSONQuery()
+void cmInstrumentation::WriteJSONQuery(
+  std::set<cmInstrumentationQuery::Query>& queries_,
+  std::set<cmInstrumentationQuery::Hook>& hooks_, std::string& callback)
 {
   Json::Value root;
   root["version"] = 1;
   root["queries"] = Json::arrayValue;
-  for (auto const& query : this->queries) {
+  for (auto const& query : queries_) {
     root["queries"].append(cmInstrumentationQuery::QueryString[query]);
   }
   root["hooks"] = Json::arrayValue;
-  for (auto const& hook : this->hooks) {
+  for (auto const& hook : hooks_) {
     root["hooks"].append(cmInstrumentationQuery::HookString[hook]);
   }
   root["callbacks"] = Json::arrayValue;
-  for (auto const& callback : this->callbacks) {
+  if (!callback.empty()) {
     root["callbacks"].append(callback);
   }
   cmsys::Directory d;
@@ -132,16 +112,27 @@ void cmInstrumentation::ClearGeneratedQueries()
   }
 }
 
-bool cmInstrumentation::HasQuery()
+bool cmInstrumentation::HasQuery() const
 {
   return this->hasQuery;
 }
 
-bool cmInstrumentation::HasQuery(cmInstrumentationQuery::Query query)
+bool cmInstrumentation::HasQuery(cmInstrumentationQuery::Query query) const
 {
   return (this->queries.find(query) != this->queries.end());
 }
 
+bool cmInstrumentation::HasHook(cmInstrumentationQuery::Hook hook) const
+{
+  return (this->hooks.find(hook) != this->hooks.end());
+}
+
+bool cmInstrumentation::HasPreOrPostBuildHook() const
+{
+  return (this->HasHook(cmInstrumentationQuery::Hook::PreBuild) ||
+          this->HasHook(cmInstrumentationQuery::Hook::PostBuild));
+}
+
 int cmInstrumentation::CollectTimingData(cmInstrumentationQuery::Hook hook)
 {
   // Don't run collection if hook is disabled

Source/cmInstrumentation.h

@@ -21,14 +21,7 @@
 class cmInstrumentation
 {
 public:
-  // Read Queries
+  cmInstrumentation(std::string const& binary_dir);
-  cmInstrumentation(std::string const& binary_dir,
-                    bool clear_generated = false);
-  // Create Query
-  cmInstrumentation(std::string const& binary_dir,
-                    std::set<cmInstrumentationQuery::Query>& queries,
-                    std::set<cmInstrumentationQuery::Hook>& hooks,
-                    std::string& callback);
   int InstrumentCommand(
     std::string command_type, std::vector<std::string> const& command,
     std::function<int()> const& callback,
@@ -42,11 +35,16 @@ public:
                      std::chrono::system_clock::time_point systemStart);
   void GetPreTestStats();
   void LoadQueries();
-  bool HasQuery();
+  bool HasQuery() const;
-  bool HasQuery(cmInstrumentationQuery::Query);
+  bool HasQuery(cmInstrumentationQuery::Query) const;
+  bool HasHook(cmInstrumentationQuery::Hook) const;
+  bool HasPreOrPostBuildHook() const;
   bool ReadJSONQueries(std::string const& directory);
   void ReadJSONQuery(std::string const& file);
-  void WriteJSONQuery();
+  void WriteJSONQuery(std::set<cmInstrumentationQuery::Query>& queries,
+                      std::set<cmInstrumentationQuery::Hook>& hooks,
+                      std::string& callback);
+  void ClearGeneratedQueries();
   int CollectTimingData(cmInstrumentationQuery::Hook hook);
   std::string errorMsg;
 
@@ -61,7 +59,6 @@ private:
   static void InsertTimingData(
     Json::Value& root, std::chrono::steady_clock::time_point steadyStart,
     std::chrono::system_clock::time_point systemStart);
-  void ClearGeneratedQueries();
   bool HasQueryFile(std::string const& file);
   static std::string GetCommandStr(std::vector<std::string> const& args);
   static std::string ComputeSuffixHash(std::string const& command_str);

Source/cmInstrumentationCommand.cxx

@@ -18,6 +18,7 @@ file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmInstrumentationQuery.h"
 #include "cmMakefile.h"
 #include "cmStringAlgorithms.h"
+#include "cmake.h"
 
 namespace {
 
@@ -141,9 +142,9 @@ bool cmInstrumentationCommand(std::vector<std::string> const& args,
     callback = cmStrCat(callback, arg);
   }
 
-  auto instrument = cmInstrumentation(
+  status.GetMakefile()
-    status.GetMakefile().GetHomeOutputDirectory(), queries, hooks, callback);
+    .GetCMakeInstance()
-  instrument.WriteJSONQuery();
+    ->GetInstrumentation()
-
+    ->WriteJSONQuery(queries, hooks, callback);
   return true;
 }

Source/cmake.cxx

@@ -2611,21 +2611,24 @@ int cmake::ActualConfigure()
       cmStrCat(this->GetHomeOutputDirectory(), "/CMakeFiles"_s),
       this->FileAPI->GetConfigureLogVersions());
   }
+
+  this->Instrumentation =
+    cm::make_unique<cmInstrumentation>(this->State->GetBinaryDirectory());
+  this->Instrumentation->ClearGeneratedQueries();
 #endif
 
   // actually do the configure
   auto startTime = std::chrono::steady_clock::now();
 #if !defined(CMAKE_BOOTSTRAP)
-  cmInstrumentation instrumentation(this->State->GetBinaryDirectory(), true);
+  if (!this->Instrumentation->errorMsg.empty()) {
-  if (!instrumentation.errorMsg.empty()) {
+    cmSystemTools::Error(this->Instrumentation->errorMsg);
-    cmSystemTools::Error(instrumentation.errorMsg);
     return 1;
   }
   std::function<int()> doConfigure = [this]() -> int {
     this->GlobalGenerator->Configure();
     return 0;
   };
-  int ret = instrumentation.InstrumentCommand(
+  int ret = this->Instrumentation->InstrumentCommand(
     "configure", this->cmdArgs, [doConfigure]() { return doConfigure(); },
     cm::nullopt, cm::nullopt, true);
   if (ret != 0) {
@@ -2670,8 +2673,8 @@ int cmake::ActualConfigure()
   }
   // Setup launchers for instrumentation
 #if !defined(CMAKE_BOOTSTRAP)
-  instrumentation.LoadQueries();
+  this->Instrumentation->LoadQueries();
-  if (instrumentation.HasQuery()) {
+  if (this->Instrumentation->HasQuery()) {
     std::string launcher;
     if (mf->IsOn("CTEST_USE_LAUNCHERS")) {
       launcher =
@@ -3015,7 +3018,6 @@ int cmake::Generate()
   auto startTime = std::chrono::steady_clock::now();
 #if !defined(CMAKE_BOOTSTRAP)
   auto profilingRAII = this->CreateProfilingEntry("project", "generate");
-  cmInstrumentation instrumentation(this->State->GetBinaryDirectory());
   std::function<int()> doGenerate = [this]() -> int {
     if (!this->GlobalGenerator->Compute()) {
       return -1;
@@ -3024,7 +3026,8 @@ int cmake::Generate()
     return 0;
   };
 
-  int ret = instrumentation.InstrumentCommand(
+  this->Instrumentation->LoadQueries();
+  int ret = this->Instrumentation->InstrumentCommand(
     "generate", this->cmdArgs, [doGenerate]() { return doGenerate(); });
   if (ret != 0) {
     return ret;
@@ -3045,7 +3048,7 @@ int cmake::Generate()
     this->UpdateProgress(msg.str(), -1);
   }
 #if !defined(CMAKE_BOOTSTRAP)
-  instrumentation.CollectTimingData(
+  this->Instrumentation->CollectTimingData(
     cmInstrumentationQuery::Hook::PostGenerate);
 #endif
   if (!this->GraphVizFile.empty()) {

Source/cmake.h

@@ -49,6 +49,7 @@ class cmDebuggerAdapter;
 
 class cmExternalMakefileProjectGeneratorFactory;
 class cmFileAPI;
+class cmInstrumentation;
 class cmFileTimeCache;
 class cmGlobalGenerator;
 class cmMakefile;
@@ -663,6 +664,10 @@ public:
 
 #if !defined(CMAKE_BOOTSTRAP)
   cmFileAPI* GetFileAPI() const { return this->FileAPI.get(); }
+  cmInstrumentation* GetInstrumentation() const
+  {
+    return this->Instrumentation.get();
+  }
 #endif
 
   cmState* GetState() const { return this->State.get(); }
@@ -816,6 +821,7 @@ private:
 #if !defined(CMAKE_BOOTSTRAP)
   std::unique_ptr<cmVariableWatch> VariableWatch;
   std::unique_ptr<cmFileAPI> FileAPI;
+  std::unique_ptr<cmInstrumentation> Instrumentation;
 #endif
 
   std::unique_ptr<cmState> State;