From f62a4ab2ee67874f15e3f9cc5f05c087a49188e8 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Thu, 30 Jan 2025 15:49:37 -0500 Subject: 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 | 4 +-- Source/cmInstrumentation.cxx | 49 +++++++++++++++---------------------- Source/cmInstrumentation.h | 21 +++++++--------- Source/cmInstrumentationCommand.cxx | 9 ++++--- Source/cmake.cxx | 21 +++++++++------- Source/cmake.h | 6 +++++ 6 files changed, 54 insertions(+), 56 deletions(-) diff --git a/Source/CTest/cmCTestLaunch.cxx b/Source/CTest/cmCTestLaunch.cxx index 2ab2976..f71d965 100644 --- a/Source/CTest/cmCTestLaunch.cxx +++ b/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 options; options["target"] = this->Reporter.OptionTargetName; options["source"] = this->Reporter.OptionSource; @@ -264,7 +264,7 @@ int cmCTestLaunch::Run() std::map arrayOptions; arrayOptions["outputs"] = this->Reporter.OptionOutput; arrayOptions["targetLabels"] = this->Reporter.OptionTargetLabels; - instrumenter.InstrumentCommand( + instrumentation.InstrumentCommand( this->Reporter.OptionCommandType, this->RealArgV, [this]() -> int { this->RunChild(); diff --git a/Source/cmInstrumentation.cxx b/Source/cmInstrumentation.cxx index d00fdd1..a919621 100644 --- a/Source/cmInstrumentation.cxx +++ b/Source/cmInstrumentation.cxx @@ -23,8 +23,7 @@ #include "cmSystemTools.h" #include "cmTimestamp.h" -cmInstrumentation::cmInstrumentation(std::string const& binary_dir, - bool clear_generated) +cmInstrumentation::cmInstrumentation(std::string const& binary_dir) { 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 configDir = cmSystemTools::GetCMakeConfigDirectory()) { this->userTimingDirv1 = @@ -57,24 +53,6 @@ void cmInstrumentation::LoadQueries() } } -cmInstrumentation::cmInstrumentation( - std::string const& binary_dir, - std::set& queries_, - std::set& 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& queries_, + std::set& 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 diff --git a/Source/cmInstrumentation.h b/Source/cmInstrumentation.h index 9578a5b..07b9e1d 100644 --- a/Source/cmInstrumentation.h +++ b/Source/cmInstrumentation.h @@ -21,14 +21,7 @@ class cmInstrumentation { public: - // Read Queries - cmInstrumentation(std::string const& binary_dir, - bool clear_generated = false); - // Create Query - cmInstrumentation(std::string const& binary_dir, - std::set& queries, - std::set& hooks, - std::string& callback); + cmInstrumentation(std::string const& binary_dir); int InstrumentCommand( std::string command_type, std::vector const& command, std::function const& callback, @@ -42,11 +35,16 @@ public: std::chrono::system_clock::time_point systemStart); void GetPreTestStats(); void LoadQueries(); - bool HasQuery(); - bool HasQuery(cmInstrumentationQuery::Query); + bool HasQuery() const; + 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& queries, + std::set& 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 const& args); static std::string ComputeSuffixHash(std::string const& command_str); diff --git a/Source/cmInstrumentationCommand.cxx b/Source/cmInstrumentationCommand.cxx index cb8fcaa..406ced8 100644 --- a/Source/cmInstrumentationCommand.cxx +++ b/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 const& args, callback = cmStrCat(callback, arg); } - auto instrument = cmInstrumentation( - status.GetMakefile().GetHomeOutputDirectory(), queries, hooks, callback); - instrument.WriteJSONQuery(); - + status.GetMakefile() + .GetCMakeInstance() + ->GetInstrumentation() + ->WriteJSONQuery(queries, hooks, callback); return true; } diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 3f2dc7f..5acd381 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2611,21 +2611,24 @@ int cmake::ActualConfigure() cmStrCat(this->GetHomeOutputDirectory(), "/CMakeFiles"_s), this->FileAPI->GetConfigureLogVersions()); } + + this->Instrumentation = + cm::make_unique(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 (!instrumentation.errorMsg.empty()) { - cmSystemTools::Error(instrumentation.errorMsg); + if (!this->Instrumentation->errorMsg.empty()) { + cmSystemTools::Error(this->Instrumentation->errorMsg); return 1; } std::function 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(); - if (instrumentation.HasQuery()) { + this->Instrumentation->LoadQueries(); + 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 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()) { diff --git a/Source/cmake.h b/Source/cmake.h index d59e844..49231f4 100644 --- a/Source/cmake.h +++ b/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 VariableWatch; std::unique_ptr FileAPI; + std::unique_ptr Instrumentation; #endif std::unique_ptr State; -- cgit v0.12