diff options
Diffstat (limited to 'src')
22 files changed, 200 insertions, 100 deletions
diff --git a/src/buildtool/build_engine/target_map/TARGETS b/src/buildtool/build_engine/target_map/TARGETS index b753e503..38c261d5 100644 --- a/src/buildtool/build_engine/target_map/TARGETS +++ b/src/buildtool/build_engine/target_map/TARGETS @@ -45,6 +45,7 @@ , ["src/buildtool/build_engine/base_maps", "rule_map"] , ["src/buildtool/build_engine/base_maps", "source_map"] , ["src/buildtool/build_engine/base_maps", "targets_file_map"] + , ["src/buildtool/common", "common"] , ["src/buildtool/common", "config"] , ["src/buildtool/multithreading", "async_map_consumer"] , ["src/buildtool/storage", "storage"] @@ -97,6 +98,7 @@ [ "configured_target" , "result_map" , ["src/buildtool/build_engine/analysed_target", "target"] + , ["src/buildtool/common", "common"] , ["src/buildtool/common", "config"] , ["src/buildtool/multithreading", "async_map_consumer"] , ["@", "gsl", "", "gsl"] diff --git a/src/buildtool/build_engine/target_map/absent_target_map.cpp b/src/buildtool/build_engine/target_map/absent_target_map.cpp index 27eb9e86..f72dc697 100644 --- a/src/buildtool/build_engine/target_map/absent_target_map.cpp +++ b/src/buildtool/build_engine/target_map/absent_target_map.cpp @@ -23,13 +23,15 @@ auto BuildMaps::Target::CreateAbsentTargetMap( const gsl::not_null<ResultTargetMap*>& result_map, gsl::not_null<RepositoryConfig*> const& repo_config, + gsl::not_null<Statistics*> const& local_stats, std::size_t jobs) -> AbsentTargetMap { #ifndef BOOTSTRAP_BUILD_TOOL - auto target_reader = [result_map, repo_config](auto /*ts*/, - auto setter, - auto logger, - auto /*subcaller*/, - auto key) { + auto target_reader = [result_map, repo_config, local_stats]( + auto /*ts*/, + auto setter, + auto logger, + auto /*subcaller*/, + auto key) { // assumptions: // - target with absent targets file requested // - ServeApi correctly configured @@ -137,7 +139,7 @@ auto BuildMaps::Target::CreateAbsentTargetMap( info.ToString()); (*setter)(std::move(analysis_result)); - Statistics::Instance().IncrementExportsCachedCounter(); + local_stats->IncrementExportsCachedCounter(); return; } (*logger)(fmt::format("Reading target entry for key {} failed", diff --git a/src/buildtool/build_engine/target_map/absent_target_map.hpp b/src/buildtool/build_engine/target_map/absent_target_map.hpp index aa2b5b42..a00fc05d 100644 --- a/src/buildtool/build_engine/target_map/absent_target_map.hpp +++ b/src/buildtool/build_engine/target_map/absent_target_map.hpp @@ -19,6 +19,7 @@ #include "src/buildtool/build_engine/target_map/configured_target.hpp" #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/common/repository_config.hpp" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/multithreading/async_map_consumer.hpp" namespace BuildMaps::Target { @@ -26,6 +27,7 @@ using AbsentTargetMap = AsyncMapConsumer<ConfiguredTarget, AnalysedTargetPtr>; auto CreateAbsentTargetMap(const gsl::not_null<ResultTargetMap*>&, gsl::not_null<RepositoryConfig*> const& repo_config, + gsl::not_null<Statistics*> const& local_stats, std::size_t jobs = 0) -> AbsentTargetMap; } // namespace BuildMaps::Target diff --git a/src/buildtool/build_engine/target_map/built_in_rules.cpp b/src/buildtool/build_engine/target_map/built_in_rules.cpp index d28a6ed9..2c90cc9d 100644 --- a/src/buildtool/build_engine/target_map/built_in_rules.cpp +++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp @@ -340,6 +340,7 @@ void FileGenRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -359,6 +360,7 @@ void SymlinkRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -487,6 +489,7 @@ void TreeRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -736,6 +739,7 @@ void InstallRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -1322,6 +1326,7 @@ void GenericRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -1400,6 +1405,7 @@ void ConfigureRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, [[maybe_unused]] const ActiveTargetCache& /*target_cache*/, + [[maybe_unused]] const gsl::not_null<Statistics*>& /*stats*/, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -1541,6 +1547,7 @@ auto const kBuiltIns = std::unordered_map< const BuildMaps::Target::ConfiguredTarget&, const gsl::not_null<RepositoryConfig*>&, const ActiveTargetCache&, + const gsl::not_null<Statistics*>&, const BuildMaps::Target::TargetMap::SubCallerPtr&, const BuildMaps::Target::TargetMap::SetterPtr&, const BuildMaps::Target::TargetMap::LoggerPtr&, @@ -1572,6 +1579,7 @@ auto HandleBuiltin( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -1598,6 +1606,7 @@ auto HandleBuiltin( key, repo_config, target_cache, + stats, subcaller, setter, target_logger, diff --git a/src/buildtool/build_engine/target_map/built_in_rules.hpp b/src/buildtool/build_engine/target_map/built_in_rules.hpp index e3e2f220..1eb6e086 100644 --- a/src/buildtool/build_engine/target_map/built_in_rules.hpp +++ b/src/buildtool/build_engine/target_map/built_in_rules.hpp @@ -21,6 +21,7 @@ #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/build_engine/target_map/target_map.hpp" #include "src/buildtool/common/repository_config.hpp" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/storage/target_cache.hpp" namespace BuildMaps::Target { @@ -30,6 +31,7 @@ auto HandleBuiltin( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, diff --git a/src/buildtool/build_engine/target_map/export.cpp b/src/buildtool/build_engine/target_map/export.cpp index c9aed0e3..29085716 100644 --- a/src/buildtool/build_engine/target_map/export.cpp +++ b/src/buildtool/build_engine/target_map/export.cpp @@ -98,6 +98,7 @@ void ExportRule( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, @@ -134,7 +135,7 @@ void ExportRule( #endif // BOOTSTRAP_BUILD_TOOL if (not target_cache_value) { - Statistics::Instance().IncrementExportsUncachedCounter(); + stats->IncrementExportsUncachedCounter(); Logger::Log(LogLevel::Performance, "Export target {} registered for caching: {}", key.target.ToString(), @@ -176,7 +177,7 @@ void ExportRule( info.ToString()); (*setter)(std::move(analysis_result)); - Statistics::Instance().IncrementExportsCachedCounter(); + stats->IncrementExportsCachedCounter(); return; } (*logger)(fmt::format("Reading target entry for key {} failed", @@ -185,7 +186,7 @@ void ExportRule( } } else { - Statistics::Instance().IncrementExportsNotEligibleCounter(); + stats->IncrementExportsNotEligibleCounter(); Logger::Log(LogLevel::Performance, "Export target {} is not eligible for target caching", key.target.ToString()); diff --git a/src/buildtool/build_engine/target_map/export.hpp b/src/buildtool/build_engine/target_map/export.hpp index 7f470706..3f43796b 100644 --- a/src/buildtool/build_engine/target_map/export.hpp +++ b/src/buildtool/build_engine/target_map/export.hpp @@ -21,12 +21,14 @@ #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/build_engine/target_map/target_map.hpp" #include "src/buildtool/common/repository_config.hpp" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/storage/target_cache.hpp" void ExportRule(const nlohmann::json& desc_json, const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, const BuildMaps::Target::TargetMap::SubCallerPtr& subcaller, const BuildMaps::Target::TargetMap::SetterPtr& setter, const BuildMaps::Target::TargetMap::LoggerPtr& logger, diff --git a/src/buildtool/build_engine/target_map/result_map.hpp b/src/buildtool/build_engine/target_map/result_map.hpp index 6911bf02..dcc6577e 100644 --- a/src/buildtool/build_engine/target_map/result_map.hpp +++ b/src/buildtool/build_engine/target_map/result_map.hpp @@ -170,7 +170,9 @@ class ResultTargetMap { } template <bool kIncludeOrigins = false> - [[nodiscard]] auto ToResult() const -> ResultType<kIncludeOrigins> { + [[nodiscard]] auto ToResult(gsl::not_null<Statistics const*> const& stats, + gsl::not_null<Progress*> const& progress) const + -> ResultType<kIncludeOrigins> { ResultType<kIncludeOrigins> result{}; size_t na = 0; size_t nb = 0; @@ -184,7 +186,7 @@ class ResultTargetMap { result.blobs.reserve(nb); result.trees.reserve(nt); - auto& origin_map = Progress::Instance().OriginMap(); + auto& origin_map = progress->OriginMap(); origin_map.clear(); origin_map.reserve(na); for (const auto& target : targets_) { @@ -298,7 +300,7 @@ class ResultTargetMap { }); result.actions.erase(lastaction, result.actions.end()); - int trees_traversed = Statistics::Instance().TreesAnalysedCounter(); + int trees_traversed = stats->TreesAnalysedCounter(); if (trees_traversed > 0) { Logger::Log(LogLevel::Performance, "Analysed {} non-known source trees", @@ -314,8 +316,10 @@ class ResultTargetMap { } template <bool kIncludeOrigins = false> - [[nodiscard]] auto ToJson() const -> nlohmann::json { - auto const result = ToResult<kIncludeOrigins>(); + [[nodiscard]] auto ToJson(gsl::not_null<Statistics const*> const& stats, + gsl::not_null<Progress*> const& progress) const + -> nlohmann::json { + auto const result = ToResult<kIncludeOrigins>(stats, progress); auto actions = nlohmann::json::object(); auto trees = nlohmann::json::object(); std::for_each(result.actions.begin(), @@ -340,11 +344,15 @@ class ResultTargetMap { } template <bool kIncludeOrigins = true> - auto ToFile(std::string const& graph_file, int indent = 2) const -> void { + auto ToFile(std::string const& graph_file, + gsl::not_null<Statistics const*> const& stats, + gsl::not_null<Progress*> const& progress, + int indent = 2) const -> void { Logger::Log( LogLevel::Info, "Dumping action graph to file {}.", graph_file); std::ofstream os(graph_file); - os << std::setw(indent) << ToJson<kIncludeOrigins>() << std::endl; + os << std::setw(indent) << ToJson<kIncludeOrigins>(stats, progress) + << std::endl; } void Clear(gsl::not_null<TaskSystem*> const& ts) { diff --git a/src/buildtool/build_engine/target_map/target_map.cpp b/src/buildtool/build_engine/target_map/target_map.cpp index 4d28ac6a..aed6d93a 100644 --- a/src/buildtool/build_engine/target_map/target_map.cpp +++ b/src/buildtool/build_engine/target_map/target_map.cpp @@ -1238,6 +1238,7 @@ void withTargetsFile( const BuildMaps::Target::ConfiguredTarget& key, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, const nlohmann::json& targets_file, const gsl::not_null<BuildMaps::Base::SourceTargetMap*>& source_target, const gsl::not_null<BuildMaps::Base::UserRuleMap*>& rule_map, @@ -1277,6 +1278,7 @@ void withTargetsFile( key, repo_config, target_cache, + stats, subcaller, setter, logger, @@ -1457,7 +1459,8 @@ void TreeTarget( const BuildMaps::Target::TargetMap::LoggerPtr& logger, const gsl::not_null<BuildMaps::Target::ResultTargetMap*>& result_map, const gsl::not_null<BuildMaps::Base::DirectoryEntriesMap*>& - directory_entries) { + directory_entries, + const gsl::not_null<Statistics*>& stats) { const auto& target = key.target.GetNamedTarget(); const auto dir_name = std::filesystem::path{target.module} / target.name; auto module_ = BuildMaps::Base::ModuleName{target.repository, dir_name}; @@ -1465,7 +1468,7 @@ void TreeTarget( directory_entries->ConsumeAfterKeysReady( ts, {module_}, - [setter, subcaller, target, key, result_map, logger, dir_name]( + [setter, subcaller, target, key, result_map, logger, dir_name, stats]( auto values) { // expected values.size() == 1 const auto& dir_entries = *values[0]; @@ -1495,7 +1498,7 @@ void TreeTarget( "Source tree reference for non-known tree {}", key.target.ToString()); }); - Statistics::Instance().IncrementTreesAnalysedCounter(); + stats->IncrementTreesAnalysedCounter(); using BuildMaps::Target::ConfiguredTarget; @@ -1660,6 +1663,7 @@ auto CreateTargetMap( const gsl::not_null<ResultTargetMap*>& result_map, const gsl::not_null<RepositoryConfig*>& repo_config, const ActiveTargetCache& target_cache, + const gsl::not_null<Statistics*>& stats, std::size_t jobs) -> TargetMap { auto target_reader = [source_target_map, targets_file_map, @@ -1668,11 +1672,12 @@ auto CreateTargetMap( absent_target_map, result_map, repo_config, - target_cache](auto ts, - auto setter, - auto logger, - auto subcaller, - auto key) { + target_cache, + stats](auto ts, + auto setter, + auto logger, + auto subcaller, + auto key) { if (key.target.IsAnonymousTarget()) { withTargetNode(key, repo_config, @@ -1700,7 +1705,8 @@ auto CreateTargetMap( setter, wrapped_logger, result_map, - directory_entries_map); + directory_entries_map, + stats); } else if (key.target.GetNamedTarget().reference_t == BuildMaps::Base::ReferenceType::kFile) { @@ -1808,6 +1814,7 @@ auto CreateTargetMap( [key, repo_config, target_cache, + stats, source_target_map, rule_map, ts, @@ -1818,6 +1825,7 @@ auto CreateTargetMap( withTargetsFile(key, repo_config, target_cache, + stats, *values[0], source_target_map, rule_map, diff --git a/src/buildtool/build_engine/target_map/target_map.hpp b/src/buildtool/build_engine/target_map/target_map.hpp index 2832eb35..eda27b77 100644 --- a/src/buildtool/build_engine/target_map/target_map.hpp +++ b/src/buildtool/build_engine/target_map/target_map.hpp @@ -28,6 +28,7 @@ #include "src/buildtool/build_engine/target_map/configured_target.hpp" #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/common/repository_config.hpp" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/multithreading/async_map_consumer.hpp" #include "src/buildtool/storage/target_cache.hpp" @@ -44,6 +45,7 @@ auto CreateTargetMap( const gsl::not_null<ResultTargetMap*>&, const gsl::not_null<RepositoryConfig*>&, const ActiveTargetCache&, + const gsl::not_null<Statistics*>& stats, std::size_t jobs = 0) -> TargetMap; // use explicit cast to std::function to allow template deduction when used diff --git a/src/buildtool/common/statistics.hpp b/src/buildtool/common/statistics.hpp index e8987d74..4022db15 100644 --- a/src/buildtool/common/statistics.hpp +++ b/src/buildtool/common/statistics.hpp @@ -19,11 +19,6 @@ class Statistics { public: - [[nodiscard]] static auto Instance() noexcept -> Statistics& { - static Statistics instance{}; - return instance; - } - void Reset() noexcept { num_actions_queued_ = 0; num_actions_executed_ = 0; diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index caf84b80..2220f15a 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -55,7 +55,9 @@ class ExecutorImpl { std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> const& dispatch_list, std::chrono::milliseconds const& timeout, - IExecutionAction::CacheFlag cache_flag) + IExecutionAction::CacheFlag cache_flag, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress) -> std::optional<IExecutionResponse::Ptr> { auto const& inputs = action->Dependencies(); auto const tree_action = action->Content().IsTreeAction(); @@ -95,8 +97,8 @@ class ExecutorImpl { // do not count statistics for rebuilder fetching from cache if (cache_flag != IExecutionAction::CacheFlag::FromCacheOnly) { - Progress::Instance().TaskTracker().Start(action->Content().Id()); - Statistics::Instance().IncrementActionsQueuedCounter(); + progress->TaskTracker().Start(action->Content().Id()); + stats->IncrementActionsQueuedCounter(); } auto alternative_api = @@ -501,6 +503,8 @@ class ExecutorImpl { Logger const& logger, IExecutionResponse::Ptr const& response, gsl::not_null<DependencyGraph::ActionNode const*> const& action, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress, bool count_as_executed = false) -> bool { logger.Emit(LogLevel::Trace, "finished execution"); @@ -511,12 +515,12 @@ class ExecutorImpl { if (not count_as_executed and response->IsCached()) { logger.Emit(LogLevel::Trace, " - served from cache"); - Statistics::Instance().IncrementActionsCachedCounter(); + stats->IncrementActionsCachedCounter(); } else { - Statistics::Instance().IncrementActionsExecutedCounter(); + stats->IncrementActionsExecutedCounter(); } - Progress::Instance().TaskTracker().Stop(action->Content().Id()); + progress->TaskTracker().Stop(action->Content().Id()); PrintInfo(logger, action->Command(), response); bool should_fail_outputs = false; @@ -535,7 +539,7 @@ class ExecutorImpl { logger.Emit(LogLevel::Error, "action returned non-zero exit code {}", response->ExitCode()); - PrintError(logger, action); + PrintError(logger, action, progress); return false; } } @@ -556,7 +560,7 @@ class ExecutorImpl { } return message; }); - PrintError(logger, action); + PrintError(logger, action, progress); return false; } @@ -601,12 +605,12 @@ class ExecutorImpl { void static PrintError( Logger const& logger, - gsl::not_null<DependencyGraph::ActionNode const*> const& - action) noexcept { + gsl::not_null<DependencyGraph::ActionNode const*> const& action, + gsl::not_null<Progress*> const& progress) noexcept { std::ostringstream msg{}; msg << "Failed to execute command "; msg << nlohmann::json(action->Command()).dump(); - auto const& origin_map = Progress::Instance().OriginMap(); + auto const& origin_map = progress->OriginMap(); auto origins = origin_map.find(action->Content().Id()); if (origins != origin_map.end() and !origins->second.empty()) { msg << "\nrequested by"; @@ -680,12 +684,16 @@ class Executor { std::map<std::string, std::string> properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress, std::chrono::milliseconds timeout = IExecutionAction::kDefaultTimeout) : repo_config_{repo_config}, local_api_{local_api}, remote_api_{remote_api}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, + stats_{stats}, + progress_{progress}, timeout_{timeout} {} /// \brief Run an action in a blocking manner @@ -704,10 +712,14 @@ class Executor { Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, Impl::ScaleTime(timeout_, action->TimeoutScale()), - action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput); + action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput, + stats_, + progress_); // check response and save digests of results - return not response or Impl::ParseResponse(logger, *response, action); + return not response or + Impl::ParseResponse( + logger, *response, action, stats_, progress_); } /// \brief Check artifact is available to the CAS or upload it. @@ -729,6 +741,8 @@ class Executor { std::map<std::string, std::string> properties_; std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list_; + gsl::not_null<Statistics*> stats_; + gsl::not_null<Progress*> progress_; std::chrono::milliseconds timeout_; }; @@ -751,6 +765,8 @@ class Rebuilder { std::map<std::string, std::string> properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress, std::chrono::milliseconds timeout = IExecutionAction::kDefaultTimeout) : repo_config_{repo_config}, local_api_{local_api}, @@ -758,6 +774,8 @@ class Rebuilder { api_cached_{api_cached}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, + stats_{stats}, + progress_{progress}, timeout_{timeout} {} [[nodiscard]] auto Process( @@ -772,7 +790,9 @@ class Rebuilder { Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, Impl::ScaleTime(timeout_, action->TimeoutScale()), - CF::PretendCached); + CF::PretendCached, + stats_, + progress_); if (not response) { return true; // action without response (e.g., tree action) @@ -786,7 +806,9 @@ class Rebuilder { Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, Impl::ScaleTime(timeout_, action->TimeoutScale()), - CF::FromCacheOnly); + CF::FromCacheOnly, + stats_, + progress_); if (not response_cached) { logger_cached.Emit(LogLevel::Error, @@ -795,8 +817,12 @@ class Rebuilder { } DetectFlakyAction(*response, *response_cached, action->Content()); - return Impl::ParseResponse( - logger, *response, action, /*count_as_executed=*/true); + return Impl::ParseResponse(logger, + *response, + action, + stats_, + progress_, + /*count_as_executed=*/true); } [[nodiscard]] auto Process( @@ -827,6 +853,8 @@ class Rebuilder { std::map<std::string, std::string> properties_; std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list_; + gsl::not_null<Statistics*> stats_; + gsl::not_null<Progress*> progress_; std::chrono::milliseconds timeout_; mutable std::mutex m_; mutable std::vector<std::string> cache_misses_{}; @@ -842,7 +870,7 @@ class Rebuilder { Action const& action) const noexcept { if (response and response_cached and response_cached->ActionDigest() == response->ActionDigest()) { - Statistics::Instance().IncrementRebuiltActionComparedCounter(); + stats_->IncrementRebuiltActionComparedCounter(); auto artifacts = response->Artifacts(); auto artifacts_cached = response_cached->Artifacts(); std::ostringstream msg{}; @@ -853,11 +881,10 @@ class Rebuilder { } } if (msg.tellp() > 0) { - Statistics::Instance().IncrementActionsFlakyCounter(); + stats_->IncrementActionsFlakyCounter(); bool tainted = action.MayFail() or action.NoCache(); if (tainted) { - Statistics::Instance() - .IncrementActionsFlakyTaintedCounter(); + stats_->IncrementActionsFlakyTaintedCounter(); } Logger::Log(tainted ? LogLevel::Debug : LogLevel::Warning, "{}", @@ -865,7 +892,7 @@ class Rebuilder { } } else { - Statistics::Instance().IncrementRebuiltActionMissingCounter(); + stats_->IncrementRebuiltActionMissingCounter(); std::unique_lock lock{m_}; cache_misses_.emplace_back(action.Id()); } diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index 611ea3e0..b4604ef8 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -75,11 +75,15 @@ class GraphTraverser { gsl::not_null<RepositoryConfig*> const& repo_config, std::map<std::string, std::string> platform_properties, std::vector<std::pair<std::map<std::string, std::string>, - ServerAddress>> dispatch_list) + ServerAddress>> dispatch_list, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress) : clargs_{std::move(clargs)}, repo_config_{repo_config}, platform_properties_{std::move(platform_properties)}, dispatch_list_{std::move(dispatch_list)}, + stats_{stats}, + progress_{progress}, local_api_{CreateExecutionApi(std::nullopt, std::make_optional(repo_config))}, remote_api_{CreateExecutionApi(RemoteExecutionConfig::RemoteAddress(), @@ -92,11 +96,15 @@ class GraphTraverser { std::map<std::string, std::string> platform_properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list, + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress, progress_reporter_t reporter) : clargs_{std::move(clargs)}, repo_config_{repo_config}, platform_properties_{std::move(platform_properties)}, dispatch_list_{std::move(dispatch_list)}, + stats_{stats}, + progress_{progress}, local_api_{CreateExecutionApi(std::nullopt, std::make_optional(repo_config))}, remote_api_{CreateExecutionApi(RemoteExecutionConfig::RemoteAddress(), @@ -246,6 +254,8 @@ class GraphTraverser { std::map<std::string, std::string> platform_properties_; std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list_; + gsl::not_null<Statistics*> stats_; + gsl::not_null<Progress*> progress_; gsl::not_null<IExecutionApi::Ptr> const local_api_; gsl::not_null<IExecutionApi::Ptr> const remote_api_; progress_reporter_t reporter_; @@ -368,6 +378,8 @@ class GraphTraverser { &(*remote_api_), platform_properties_, dispatch_list_, + stats_, + progress_, clargs_.build.timeout}; bool traversing{}; std::atomic<bool> done = false; @@ -399,6 +411,8 @@ class GraphTraverser { &(*api_cached), platform_properties_, dispatch_list_, + stats_, + progress_, clargs_.build.timeout}; bool traversing{false}; std::atomic<bool> done = false; @@ -444,20 +458,19 @@ class GraphTraverser { } void LogStatistics() const noexcept { - auto const& stats = Statistics::Instance(); if (clargs_.rebuild) { std::stringstream ss{}; - ss << stats.RebuiltActionComparedCounter() + ss << stats_->RebuiltActionComparedCounter() << " actions compared with cache"; - if (stats.ActionsFlakyCounter() > 0) { - ss << ", " << stats.ActionsFlakyCounter() + if (stats_->ActionsFlakyCounter() > 0) { + ss << ", " << stats_->ActionsFlakyCounter() << " flaky actions found"; - ss << " (" << stats.ActionsFlakyTaintedCounter() + ss << " (" << stats_->ActionsFlakyTaintedCounter() << " of which tainted)"; } - if (stats.RebuiltActionMissingCounter() > 0) { + if (stats_->RebuiltActionMissingCounter() > 0) { ss << ", no cache entry found for " - << stats.RebuiltActionMissingCounter() << " actions"; + << stats_->RebuiltActionMissingCounter() << " actions"; } ss << "."; Logger::Log(LogLevel::Info, ss.str()); @@ -465,8 +478,8 @@ class GraphTraverser { else { Logger::Log(LogLevel::Info, "Processed {} actions, {} cache hits.", - stats.ActionsQueuedCounter(), - stats.ActionsCachedCounter()); + stats_->ActionsQueuedCounter(), + stats_->ActionsCachedCounter()); } } diff --git a/src/buildtool/main/TARGETS b/src/buildtool/main/TARGETS index fd2adf7c..1919101c 100644 --- a/src/buildtool/main/TARGETS +++ b/src/buildtool/main/TARGETS @@ -4,11 +4,13 @@ , "name": ["just"] , "srcs": ["main.cpp"] , "private-deps": - [ ["src/buildtool/common", "config"] + [ ["src/buildtool/common", "common"] + , ["src/buildtool/common", "config"] , ["src/buildtool/storage", "storage"] , ["src/buildtool/compatibility", "compatibility"] , ["src/buildtool/graph_traverser", "graph_traverser"] , ["src/buildtool/logging", "logging"] + , ["src/buildtool/progress_reporting", "progress"] , ["src/buildtool/progress_reporting", "progress_reporter"] , ["src/buildtool/build_engine/target_map", "result_map"] , ["src/buildtool/build_engine/target_map", "target_map"] @@ -113,6 +115,7 @@ , "srcs": ["analyse.cpp"] , "deps": [ ["src/buildtool/common", "cli"] + , ["src/buildtool/common", "common"] , ["src/buildtool/build_engine/target_map", "configured_target"] , ["src/buildtool/build_engine/target_map", "result_map"] , ["src/buildtool/build_engine/analysed_target", "target"] diff --git a/src/buildtool/main/analyse.cpp b/src/buildtool/main/analyse.cpp index 6ea61de2..09973446 100644 --- a/src/buildtool/main/analyse.cpp +++ b/src/buildtool/main/analyse.cpp @@ -97,6 +97,7 @@ namespace Target = BuildMaps::Target; gsl::not_null<Target::ResultTargetMap*> const& result_map, gsl::not_null<RepositoryConfig*> const& repo_config, ActiveTargetCache const& target_cache, + gsl::not_null<Statistics*> const& stats, std::size_t jobs, std::optional<std::string> const& request_action_input) -> std::optional<AnalysisResult> { @@ -112,7 +113,7 @@ namespace Target = BuildMaps::Target; auto source_targets = Base::CreateSourceTargetMap(&directory_entries, repo_config, jobs); auto absent_target_map = - Target::CreateAbsentTargetMap(result_map, repo_config, jobs); + Target::CreateAbsentTargetMap(result_map, repo_config, stats, jobs); auto target_map = Target::CreateTargetMap(&source_targets, &targets_file_map, &rule_map, @@ -121,6 +122,7 @@ namespace Target = BuildMaps::Target; result_map, repo_config, target_cache, + stats, jobs); Logger::Log(LogLevel::Info, "Requested target is {}", id.ToString()); AnalysedTargetPtr target{}; diff --git a/src/buildtool/main/analyse.hpp b/src/buildtool/main/analyse.hpp index a7e1cb08..c3b4917a 100644 --- a/src/buildtool/main/analyse.hpp +++ b/src/buildtool/main/analyse.hpp @@ -20,6 +20,7 @@ #include "src/buildtool/build_engine/target_map/result_map.hpp" #include "src/buildtool/common/cli.hpp" #include "src/buildtool/common/repository_config.hpp" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/storage/target_cache.hpp" struct AnalysisResult { @@ -33,6 +34,7 @@ struct AnalysisResult { gsl::not_null<BuildMaps::Target::ResultTargetMap*> const& result_map, gsl::not_null<RepositoryConfig*> const& repo_config, ActiveTargetCache const& target_cache, + gsl::not_null<Statistics*> const& stats, std::size_t jobs, std::optional<std::string> const& request_action_input) -> std::optional<AnalysisResult>; diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp index 642251c4..65f5278d 100644 --- a/src/buildtool/main/main.cpp +++ b/src/buildtool/main/main.cpp @@ -55,6 +55,7 @@ #include "src/buildtool/serve_api/serve_service/serve_server_implementation.hpp" #include "src/buildtool/storage/garbage_collector.hpp" #endif // BOOTSTRAP_BUILD_TOOL +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/logging/log_config.hpp" #include "src/buildtool/logging/log_sink_cmdline.hpp" #include "src/buildtool/logging/log_sink_file.hpp" @@ -62,6 +63,7 @@ #include "src/buildtool/main/version.hpp" #include "src/buildtool/multithreading/async_map_consumer.hpp" #include "src/buildtool/multithreading/task_system.hpp" +#include "src/buildtool/progress_reporting/progress.hpp" #include "src/utils/cpp/concepts.hpp" #include "src/utils/cpp/json.hpp" @@ -913,6 +915,11 @@ auto main(int argc, char* argv[]) -> int { ? std::make_optional(std::move(arguments.rebuild)) : std::nullopt; + // statistics and progress instances; need to be kept alive + // used also in bootstrapped just + Statistics stats{}; + Progress progress{}; + #ifndef BOOTSTRAP_BUILD_TOOL SetupRetryConfig(arguments.retry); GraphTraverser const traverser{ @@ -923,7 +930,9 @@ auto main(int argc, char* argv[]) -> int { &repo_config, RemoteExecutionConfig::PlatformProperties(), RemoteExecutionConfig::DispatchList(), - ProgressReporter::Reporter()}; + &stats, + &progress, + ProgressReporter::Reporter(&stats, &progress)}; if (arguments.cmd == SubCommand::kInstallCas) { if (not repo_config.SetGitCAS(StorageConfig::GitRoot())) { @@ -999,11 +1008,13 @@ auto main(int argc, char* argv[]) -> int { &result_map, &repo_config, Storage::Instance().TargetCache(), + &stats, arguments.common.jobs, arguments.analysis.request_action_input); if (result) { if (arguments.analysis.graph_file) { - result_map.ToFile(*arguments.analysis.graph_file); + result_map.ToFile( + *arguments.analysis.graph_file, &stats, &progress); } auto const [artifacts, runfiles] = ReadOutputArtifacts(result->target); @@ -1028,11 +1039,10 @@ auto main(int argc, char* argv[]) -> int { "Analysed target {}", result->id.ToString()); - auto const& stat = Statistics::Instance(); { - auto cached = stat.ExportsCachedCounter(); - auto uncached = stat.ExportsUncachedCounter(); - auto not_eligible = stat.ExportsNotEligibleCounter(); + auto cached = stats.ExportsCachedCounter(); + auto uncached = stats.ExportsUncachedCounter(); + auto not_eligible = stats.ExportsNotEligibleCounter(); Logger::Log(cached + uncached + not_eligible > 0 ? LogLevel::Info : LogLevel::Debug, @@ -1044,7 +1054,8 @@ auto main(int argc, char* argv[]) -> int { } ReportTaintedness(*result); - auto const& [actions, blobs, trees] = result_map.ToResult(); + auto const& [actions, blobs, trees] = + result_map.ToResult(&stats, &progress); // collect cache targets and artifacts for target-level caching auto const cache_targets = result_map.CacheTargets(); diff --git a/src/buildtool/progress_reporting/TARGETS b/src/buildtool/progress_reporting/TARGETS index 45c3db1c..6a00bcdc 100644 --- a/src/buildtool/progress_reporting/TARGETS +++ b/src/buildtool/progress_reporting/TARGETS @@ -24,14 +24,14 @@ , "hdrs": ["progress_reporter.hpp"] , "srcs": ["progress_reporter.cpp"] , "stage": ["src", "buildtool", "progress_reporting"] - , "deps": ["base_progress_reporter"] - , "private-deps": - [ "progress" - , ["@", "fmt", "", "fmt"] + , "deps": + [ "base_progress_reporter" + , "progress" , ["@", "gsl", "", "gsl"] , ["src/buildtool/common", "common"] - , ["src/buildtool/logging", "logging"] ] + , "private-deps": + [["@", "fmt", "", "fmt"], ["src/buildtool/logging", "logging"]] } , "base_progress_reporter": { "type": ["@", "rules", "CC", "library"] diff --git a/src/buildtool/progress_reporting/progress.hpp b/src/buildtool/progress_reporting/progress.hpp index 43cdc7cc..d9b3aa8e 100644 --- a/src/buildtool/progress_reporting/progress.hpp +++ b/src/buildtool/progress_reporting/progress.hpp @@ -26,11 +26,6 @@ class Progress { public: - [[nodiscard]] static auto Instance() noexcept -> Progress& { - static Progress instance{}; - return instance; - } - [[nodiscard]] auto TaskTracker() noexcept -> TaskTracker& { return task_tracker_; } diff --git a/src/buildtool/progress_reporting/progress_reporter.cpp b/src/buildtool/progress_reporting/progress_reporter.cpp index 86ec0c0e..27960172 100644 --- a/src/buildtool/progress_reporting/progress_reporter.cpp +++ b/src/buildtool/progress_reporting/progress_reporter.cpp @@ -17,25 +17,23 @@ #include <string> #include "fmt/core.h" -#include "gsl/gsl" -#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" -#include "src/buildtool/progress_reporting/progress.hpp" -auto ProgressReporter::Reporter() noexcept -> progress_reporter_t { - return BaseProgressReporter::Reporter([]() { - int total = gsl::narrow<int>(Progress::Instance().OriginMap().size()); +auto ProgressReporter::Reporter( + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress) noexcept -> progress_reporter_t { + return BaseProgressReporter::Reporter([stats, progress]() { + int total = gsl::narrow<int>(progress->OriginMap().size()); // Note: order matters; queued has to be queried last - auto const& sample = Progress::Instance().TaskTracker().Sample(); - auto const& stats = Statistics::Instance(); - int cached = stats.ActionsCachedCounter(); - int run = stats.ActionsExecutedCounter(); - int queued = stats.ActionsQueuedCounter(); + auto const& sample = progress->TaskTracker().Sample(); + int cached = stats->ActionsCachedCounter(); + int run = stats->ActionsExecutedCounter(); + int queued = stats->ActionsQueuedCounter(); int active = queued - run - cached; std::string now_msg; if (active > 0 and !sample.empty()) { - auto const& origin_map = Progress::Instance().OriginMap(); + auto const& origin_map = progress->OriginMap(); auto origins = origin_map.find(sample); if (origins != origin_map.end() and !origins->second.empty()) { auto const& origin = origins->second[0]; diff --git a/src/buildtool/progress_reporting/progress_reporter.hpp b/src/buildtool/progress_reporting/progress_reporter.hpp index 5323578f..1c4206de 100644 --- a/src/buildtool/progress_reporting/progress_reporter.hpp +++ b/src/buildtool/progress_reporting/progress_reporter.hpp @@ -15,11 +15,17 @@ #ifndef INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP #define INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP +#include "gsl/gsl" +#include "src/buildtool/common/statistics.hpp" #include "src/buildtool/progress_reporting/base_progress_reporter.hpp" +#include "src/buildtool/progress_reporting/progress.hpp" class ProgressReporter { public: - [[nodiscard]] static auto Reporter() noexcept -> progress_reporter_t; + [[nodiscard]] static auto Reporter( + gsl::not_null<Statistics*> const& stats, + gsl::not_null<Progress*> const& progress) noexcept + -> progress_reporter_t; }; #endif // INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp index 97b75b53..202335d9 100644 --- a/src/buildtool/serve_api/serve_service/target.cpp +++ b/src/buildtool/serve_api/serve_service/target.cpp @@ -401,11 +401,17 @@ auto TargetService::ServeTarget( auto configured_target = BuildMaps::Target::ConfiguredTarget{ .target = std::move(*entity), .config = std::move(config)}; + // setup progress reporting; these instances need to be kept alive for + // graph traversal, analysis, and build + Statistics stats; + Progress progress; + // analyse the configured target auto result = AnalyseTarget(configured_target, &result_map, &repository_config, *tc, + &stats, RemoteServeConfig::Jobs(), std::nullopt /*request_action_input*/); @@ -420,7 +426,8 @@ auto TargetService::ServeTarget( auto const [artifacts, runfiles] = ReadOutputArtifacts(result->target); // get the result map outputs - auto const& [actions, blobs, trees] = result_map.ToResult(); + auto const& [actions, blobs, trees] = + result_map.ToResult(&stats, &progress); // collect cache targets and artifacts for target-level caching auto const cache_targets = result_map.CacheTargets(); @@ -443,11 +450,14 @@ auto TargetService::ServeTarget( traverser_args.build.timeout = RemoteServeConfig::ActionTimeout(); traverser_args.stage = std::nullopt; traverser_args.rebuild = std::nullopt; - GraphTraverser const traverser{std::move(traverser_args), - &repository_config, - std::move(platform_properties), - std::move(dispatch_list), - ProgressReporter::Reporter()}; + GraphTraverser const traverser{ + std::move(traverser_args), + &repository_config, + std::move(platform_properties), + std::move(dispatch_list), + &stats, + &progress, + ProgressReporter::Reporter(&stats, &progress)}; // perform build auto build_result = traverser.BuildAndStage( |