From 1b4917d40e5b4bbfeae5146bc826299bbc27898f Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Mon, 22 Jul 2024 18:32:24 +0200 Subject: just-mr: Remove progress and statistics singletons ...and instead use simple instances created in setup, fetch, and update, respectively. The various maps and the progress reporter get access to these instances via not_null pointers. --- src/other_tools/just_mr/fetch.cpp | 14 ++++--- .../just_mr/progress_reporting/progress.hpp | 21 ++++------ .../progress_reporting/progress_reporter.cpp | 23 ++++++----- .../just_mr/progress_reporting/statistics.hpp | 25 ++++-------- src/other_tools/just_mr/setup.cpp | 47 +++++++++++----------- src/other_tools/just_mr/update.cpp | 13 +++--- 6 files changed, 67 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp index 11a11306..b6495a6f 100644 --- a/src/other_tools/just_mr/fetch.cpp +++ b/src/other_tools/just_mr/fetch.cpp @@ -473,6 +473,10 @@ auto MultiRepoFetch(std::shared_ptr const& config, } } + // setup progress and statistics instances + JustMRStatistics stats{}; + JustMRProgress progress{nr_a + nr_gt}; + // create async maps auto crit_git_op_ptr = std::make_shared(); auto critical_git_op_map = CreateCriticalGitOpMap(crit_git_op_ptr); @@ -487,7 +491,7 @@ auto MultiRepoFetch(std::shared_ptr const& config, &storage, &(*apis.local), has_remote_api ? &*apis.remote : nullptr, - &JustMRProgress::Instance(), + &progress, common_args.jobs); auto archive_fetch_map = CreateArchiveFetchMap( @@ -497,7 +501,7 @@ auto MultiRepoFetch(std::shared_ptr const& config, &(*apis.local), (fetch_args.backup_to_remote and has_remote_api) ? &*apis.remote : nullptr, - &JustMRStatistics::Instance(), + &stats, common_args.jobs); auto import_to_git_map = @@ -517,15 +521,13 @@ auto MultiRepoFetch(std::shared_ptr const& config, &(*apis.local), has_remote_api ? &*apis.remote : nullptr, fetch_args.backup_to_remote, - &JustMRProgress::Instance(), + &progress, common_args.jobs); // set up progress observer - JustMRProgress::Instance().SetTotal(static_cast(nr_a + nr_gt)); std::atomic done{false}; std::condition_variable cv{}; - auto reporter = JustMRProgressReporter::Reporter( - &JustMRStatistics::Instance(), &JustMRProgress::Instance()); + auto reporter = JustMRProgressReporter::Reporter(&stats, &progress); auto observer = std::thread([reporter, &done, &cv]() { reporter(&done, &cv); }); diff --git a/src/other_tools/just_mr/progress_reporting/progress.hpp b/src/other_tools/just_mr/progress_reporting/progress.hpp index befceb4b..2838e9b8 100644 --- a/src/other_tools/just_mr/progress_reporting/progress.hpp +++ b/src/other_tools/just_mr/progress_reporting/progress.hpp @@ -15,32 +15,25 @@ #ifndef INCLUDED_SRC_OTHER_TOOLS_JUST_MR_PROGRESS_REPORTING_PROGRESS_HPP #define INCLUDED_SRC_OTHER_TOOLS_JUST_MR_PROGRESS_REPORTING_PROGRESS_HPP -#include -#include -#include -#include -#include +#include #include "src/buildtool/progress_reporting/task_tracker.hpp" -class JustMRProgress { +class JustMRProgress final { public: - [[nodiscard]] static auto Instance() noexcept -> JustMRProgress& { - static JustMRProgress instance{}; - return instance; - } + explicit JustMRProgress(std::size_t total) noexcept : total_{total} {}; [[nodiscard]] auto TaskTracker() noexcept -> TaskTracker& { return task_tracker_; } - [[nodiscard]] auto GetTotal() const noexcept -> int { return total_; } - - void SetTotal(int total) noexcept { total_ = total; } + [[nodiscard]] auto GetTotal() const noexcept -> std::size_t { + return total_; + } private: ::TaskTracker task_tracker_{}; - int total_{}; + std::size_t total_ = 0; }; #endif // INCLUDED_SRC_OTHER_TOOLS_JUST_MR_PROGRESS_REPORTING_PROGRESS_HPP diff --git a/src/other_tools/just_mr/progress_reporting/progress_reporter.cpp b/src/other_tools/just_mr/progress_reporting/progress_reporter.cpp index 54e8b9c5..b768a2f5 100644 --- a/src/other_tools/just_mr/progress_reporting/progress_reporter.cpp +++ b/src/other_tools/just_mr/progress_reporting/progress_reporter.cpp @@ -14,6 +14,8 @@ #include "src/other_tools/just_mr/progress_reporting/progress_reporter.hpp" +#include + #include "fmt/core.h" #include "nlohmann/json.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -24,14 +26,14 @@ auto JustMRProgressReporter::Reporter( gsl::not_null const& progress) noexcept -> progress_reporter_t { return BaseProgressReporter::Reporter([stats, progress]() { - int total = progress->GetTotal(); - int local = stats->LocalPathsCounter(); - int cached = stats->CacheHitsCounter(); - int run = stats->ExecutedCounter(); - auto active = progress->TaskTracker().Active(); - auto sample = progress->TaskTracker().Sample(); - std::string msg; - msg = fmt::format("{} local, {} cached, {} done", local, cached, run); + auto const total = progress->GetTotal(); + auto const local = stats->LocalPathsCounter(); + auto const cached = stats->CacheHitsCounter(); + auto const run = stats->ExecutedCounter(); + auto const active = progress->TaskTracker().Active(); + auto const sample = progress->TaskTracker().Sample(); + auto msg = + fmt::format("{} local, {} cached, {} done", local, cached, run); if ((active > 0) && !sample.empty()) { msg = fmt::format("{}; {} fetches ({}{})", msg, @@ -40,10 +42,9 @@ auto JustMRProgressReporter::Reporter( active > 1 ? ", ..." : ""); } constexpr int kOneHundred{100}; - int total_work = total - cached - local; int progress = kOneHundred; // default if no work has to be done - if (total_work > 0) { - progress = run * kOneHundred / total_work; + if (auto const noops = cached + local; noops < total) { + progress = static_cast(run * kOneHundred / (total - noops)); } Logger::Log(LogLevel::Progress, "[{:3}%] {}", progress, msg); }); diff --git a/src/other_tools/just_mr/progress_reporting/statistics.hpp b/src/other_tools/just_mr/progress_reporting/statistics.hpp index 846d628b..dc5cd181 100644 --- a/src/other_tools/just_mr/progress_reporting/statistics.hpp +++ b/src/other_tools/just_mr/progress_reporting/statistics.hpp @@ -16,37 +16,28 @@ #define INCLUDED_SRC_OTHER_TOOLS_JUST_MR_PROGRESS_REPORTING_STATISTICS_HPP #include +#include -class JustMRStatistics { +class JustMRStatistics final { public: - [[nodiscard]] static auto Instance() noexcept -> JustMRStatistics& { - static JustMRStatistics instance{}; - return instance; - } - - void Reset() noexcept { - num_local_paths_ = 0; - num_cache_hits_ = 0; - num_executed_ = 0; - } void IncrementLocalPathsCounter() noexcept { ++num_local_paths_; } void IncrementCacheHitsCounter() noexcept { ++num_cache_hits_; } void IncrementExecutedCounter() noexcept { ++num_executed_; } - [[nodiscard]] auto LocalPathsCounter() const noexcept -> int { + [[nodiscard]] auto LocalPathsCounter() const noexcept -> size_t { return num_local_paths_; } - [[nodiscard]] auto CacheHitsCounter() const noexcept -> int { + [[nodiscard]] auto CacheHitsCounter() const noexcept -> size_t { return num_cache_hits_; } - [[nodiscard]] auto ExecutedCounter() const noexcept -> int { + [[nodiscard]] auto ExecutedCounter() const noexcept -> size_t { return num_executed_; } private: - std::atomic num_local_paths_{}; // roots that are actual paths - std::atomic num_cache_hits_{}; // no-ops - std::atomic num_executed_{}; // actual work done + std::atomic num_local_paths_ = 0; // roots that are real paths + std::atomic num_cache_hits_ = 0; // no-ops + std::atomic num_executed_ = 0; // actual work done }; #endif // INCLUDED_SRC_OTHER_TOOLS_JUST_MR_PROGRESS_REPORTING_STATISTICS_HPP diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp index dbed15a1..14d678c9 100644 --- a/src/other_tools/just_mr/setup.cpp +++ b/src/other_tools/just_mr/setup.cpp @@ -119,6 +119,9 @@ auto MultiRepoSetup(std::shared_ptr const& config, if (main and not setup_args.sub_all) { JustMR::Utils::ReachableRepositories(repos, *main, setup_repos); } + Logger::Log(LogLevel::Info, + "Found {} repositories to set up", + setup_repos->to_setup.size()); // setup remote execution config auto remote_exec_config = JustMR::Utils::CreateRemoteExecutionConfig( @@ -192,6 +195,10 @@ auto MultiRepoSetup(std::shared_ptr const& config, } } + // setup progress and statistics instances + JustMRStatistics stats{}; + JustMRProgress progress{setup_repos->to_setup.size()}; + // setup the required async maps auto crit_git_op_ptr = std::make_shared(); auto critical_git_op_map = CreateCriticalGitOpMap(crit_git_op_ptr); @@ -206,7 +213,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, &storage, &(*apis.local), has_remote_api ? &*apis.remote : nullptr, - &JustMRProgress::Instance(), + &progress, common_args.jobs); auto import_to_git_map = @@ -226,7 +233,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, &(*apis.local), has_remote_api ? &*apis.remote : nullptr, false, /* backup_to_remote */ - &JustMRProgress::Instance(), + &progress, common_args.jobs); auto resolve_symlinks_map = CreateResolveSymlinksMap(); @@ -243,7 +250,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, &(*apis.local), has_remote_api ? &*apis.remote : nullptr, common_args.fetch_absent, - &JustMRProgress::Instance(), + &progress, common_args.jobs); auto content_git_map = @@ -259,7 +266,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, &storage, has_remote_api ? &*apis.remote : nullptr, common_args.fetch_absent, - &JustMRProgress::Instance(), + &progress, common_args.jobs); auto foreign_file_git_map = @@ -306,29 +313,23 @@ auto MultiRepoSetup(std::shared_ptr const& config, has_remote_api ? &*apis.remote : nullptr, common_args.jobs); - auto repos_to_setup_map = - CreateReposToSetupMap(config, - main, - interactive, - &commit_git_map, - &content_git_map, - &foreign_file_git_map, - &fpath_git_map, - &distdir_git_map, - &tree_id_git_map, - common_args.fetch_absent, - &JustMRStatistics::Instance(), - common_args.jobs); + auto repos_to_setup_map = CreateReposToSetupMap(config, + main, + interactive, + &commit_git_map, + &content_git_map, + &foreign_file_git_map, + &fpath_git_map, + &distdir_git_map, + &tree_id_git_map, + common_args.fetch_absent, + &stats, + common_args.jobs); // set up progress observer - Logger::Log(LogLevel::Info, - "Found {} repositories to set up", - setup_repos->to_setup.size()); - JustMRProgress::Instance().SetTotal(setup_repos->to_setup.size()); std::atomic done{false}; std::condition_variable cv{}; - auto reporter = JustMRProgressReporter::Reporter( - &JustMRStatistics::Instance(), &JustMRProgress::Instance()); + auto reporter = JustMRProgressReporter::Reporter(&stats, &progress); auto observer = std::thread([reporter, &done, &cv]() { reporter(&done, &cv); }); diff --git a/src/other_tools/just_mr/update.cpp b/src/other_tools/just_mr/update.cpp index 9d2c3a0e..df9de506 100644 --- a/src/other_tools/just_mr/update.cpp +++ b/src/other_tools/just_mr/update.cpp @@ -217,21 +217,24 @@ auto MultiRepoUpdate(std::shared_ptr const& config, // Initialize resulting config to be updated auto mr_config = config->ToJson(); + + // Setup progress and statistics instances + JustMRStatistics stats{}; + JustMRProgress progress{repos_to_update.size()}; + // Create async map auto git_update_map = CreateGitUpdateMap(git_repo->GetGitCAS(), common_args.git_path->string(), *common_args.local_launcher, &storage_config, - &JustMRStatistics::Instance(), - &JustMRProgress::Instance(), + &stats, + &progress, common_args.jobs); // set up progress observer - JustMRProgress::Instance().SetTotal(repos_to_update.size()); std::atomic done{false}; std::condition_variable cv{}; - auto reporter = JustMRProgressReporter::Reporter( - &JustMRStatistics::Instance(), &JustMRProgress::Instance()); + auto reporter = JustMRProgressReporter::Reporter(&stats, &progress); auto observer = std::thread([reporter, &done, &cv]() { reporter(&done, &cv); }); -- cgit v1.2.3