From 566bcb8c4c36d12e398f00349c5f27cae06fa7a9 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 27 Jul 2017 18:40:52 +0200 Subject: Implement displaying build progress (--progress|-p) --- build2/algorithm.cxx | 4 +- build2/algorithm.ixx | 4 +- build2/b-options.cxx | 10 ++++ build2/b-options.hxx | 4 ++ build2/b-options.ixx | 6 +++ build2/b.cli | 6 +++ build2/context.cxx | 83 +++++++++++++++++++++++++-------- build2/context.hxx | 29 ++++++++++-- build2/operation.cxx | 126 +++++++++++++++++++++++++++++++++++++++------------ build2/scheduler.cxx | 11 +++++ build2/scheduler.hxx | 113 +++++++++++++++++++++++++++++++++++++++------ build2/target.ixx | 21 ++++++--- 12 files changed, 341 insertions(+), 76 deletions(-) diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index fe71d54..588980e 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -830,6 +830,8 @@ namespace build2 ts = t.recipe_ (a, t); + target_count.fetch_sub (1, memory_order_relaxed); + // See the recipe documentation for details on what's going on here. // Note that if the result is group, then the group's state can be // failed. @@ -876,8 +878,8 @@ namespace build2 // Update dependency counts and make sure they are not skew. // + size_t gd (dependency_count.fetch_sub (1, memory_order_relaxed)); size_t td (t.dependents.fetch_sub (1, memory_order_release)); - size_t gd (dependency_count.fetch_sub (1, memory_order_release)); assert (td != 0 && gd != 0); td--; diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx index 27759c9..5226af4 100644 --- a/build2/algorithm.ixx +++ b/build2/algorithm.ixx @@ -154,7 +154,7 @@ namespace build2 if (fail && s == target_state::failed) throw failed (); - dependency_count.fetch_add (1, memory_order_release); + dependency_count.fetch_add (1, memory_order_relaxed); t.dependents.fetch_add (1, memory_order_release); return s; @@ -192,7 +192,7 @@ namespace build2 } } - dependency_count.fetch_add (1, memory_order_release); + dependency_count.fetch_add (1, memory_order_relaxed); t.dependents.fetch_add (1, memory_order_release); return false; diff --git a/build2/b-options.cxx b/build2/b-options.cxx index 50871f0..1cb5b2e 100644 --- a/build2/b-options.cxx +++ b/build2/b-options.cxx @@ -571,6 +571,7 @@ namespace build2 options () : v_ (), V_ (), + progress_ (), quiet_ (), verbose_ (1), verbose_specified_ (false), @@ -675,6 +676,11 @@ namespace build2 << " equivalent to \033[1m--verbose 3\033[0m." << ::std::endl; os << std::endl + << "\033[1m--progress\033[0m|\033[1m-p\033[0m Display build progress. Only usable when printing to a" << ::std::endl + << " terminal but can be combined with various verbosity" << ::std::endl + << " levels." << ::std::endl; + + os << std::endl << "\033[1m--quiet\033[0m|\033[1m-q\033[0m Run quietly, only printing error messages. This is" << ::std::endl << " equivalent to \033[1m--verbose 0\033[0m." << ::std::endl; @@ -791,6 +797,10 @@ namespace build2 &::build2::cl::thunk< options, bool, &options::v_ >; _cli_options_map_["-V"] = &::build2::cl::thunk< options, bool, &options::V_ >; + _cli_options_map_["--progress"] = + &::build2::cl::thunk< options, bool, &options::progress_ >; + _cli_options_map_["-p"] = + &::build2::cl::thunk< options, bool, &options::progress_ >; _cli_options_map_["--quiet"] = &::build2::cl::thunk< options, bool, &options::quiet_ >; _cli_options_map_["-q"] = diff --git a/build2/b-options.hxx b/build2/b-options.hxx index a280a79..b0c8091 100644 --- a/build2/b-options.hxx +++ b/build2/b-options.hxx @@ -402,6 +402,9 @@ namespace build2 V () const; const bool& + progress () const; + + const bool& quiet () const; const uint16_t& @@ -494,6 +497,7 @@ namespace build2 public: bool v_; bool V_; + bool progress_; bool quiet_; uint16_t verbose_; bool verbose_specified_; diff --git a/build2/b-options.ixx b/build2/b-options.ixx index 6c49ede..579ba4d 100644 --- a/build2/b-options.ixx +++ b/build2/b-options.ixx @@ -229,6 +229,12 @@ namespace build2 } inline const bool& options:: + progress () const + { + return this->progress_; + } + + inline const bool& options:: quiet () const { return this->quiet_; diff --git a/build2/b.cli b/build2/b.cli index c0f9eb9..79edd59 100644 --- a/build2/b.cli +++ b/build2/b.cli @@ -333,6 +333,12 @@ namespace build2 \cb{--verbose 3}." } + bool --progress|-p + { + "Display build progress. Only usable when printing to a terminal but + can be combined with various verbosity levels." + } + bool --quiet|-q { "Run quietly, only printing error messages. This is equivalent to diff --git a/build2/context.cxx b/build2/context.cxx index 669fcae..2464973 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -197,6 +197,7 @@ namespace build2 execution_mode current_mode; atomic_count dependency_count; + atomic_count target_count; bool keep_going = false; @@ -576,81 +577,123 @@ namespace build2 // diag_do(), etc. // - void - diag_do (ostream& os, const action&, const target& t) + string + diag_do (const action&) { const meta_operation_info& m (*current_mif); const operation_info& io (*current_inner_oif); const operation_info* oo (current_outer_oif); + string r; + // perform(update(x)) -> "update x" // configure(update(x)) -> "configure updating x" // if (m.name_do.empty ()) - os << io.name_do << ' '; + r = io.name_do; else { - os << m.name_do << ' '; + r = m.name_do; if (!io.name_doing.empty ()) - os << io.name_doing << ' '; + { + r += ' '; + r += io.name_doing; + } } if (oo != nullptr) - os << "(for " << oo->name << ") "; + { + r += " (for "; + r += oo->name; + r += ')'; + } - os << t; + return r; } void - diag_doing (ostream& os, const action&, const target& t) + diag_do (ostream& os, const action& a, const target& t) + { + os << diag_do (a) << ' ' << t; + } + + string + diag_doing (const action&) { const meta_operation_info& m (*current_mif); const operation_info& io (*current_inner_oif); const operation_info* oo (current_outer_oif); + string r; + // perform(update(x)) -> "updating x" // configure(update(x)) -> "configuring updating x" // if (!m.name_doing.empty ()) - os << m.name_doing << ' '; + r = m.name_doing; if (!io.name_doing.empty ()) - os << io.name_doing << ' '; + { + if (!r.empty ()) r += ' '; + r += io.name_doing; + } if (oo != nullptr) - os << "(for " << oo->name << ") "; + { + r += " (for "; + r += oo->name; + r += ')'; + } - os << t; + return r; } void - diag_did (ostream& os, const action&, const target& t) + diag_doing (ostream& os, const action& a, const target& t) + { + os << diag_doing (a) << ' ' << t; + } + + string + diag_did (const action&) { const meta_operation_info& m (*current_mif); const operation_info& io (*current_inner_oif); const operation_info* oo (current_outer_oif); + string r; + // perform(update(x)) -> "updated x" // configure(update(x)) -> "configured updating x" // if (!m.name_did.empty ()) { - os << m.name_did << ' '; + r = m.name_did; if (!io.name_doing.empty ()) - os << io.name_doing << ' '; + { + r += ' '; + r += io.name_doing; + } } else + r += io.name_did; + + if (oo != nullptr) { - if (!io.name_did.empty ()) - os << io.name_did << ' '; + r += " (for "; + r += oo->name; + r += ')'; } - if (oo != nullptr) - os << "(for " << oo->name << ") "; + return r; + } - os << t; + void + diag_did (ostream& os, const action& a, const target& t) + { + os << diag_did (a) << ' ' << t; } void diff --git a/build2/context.hxx b/build2/context.hxx index ff8b5b3..3aa2f73 100644 --- a/build2/context.hxx +++ b/build2/context.hxx @@ -269,12 +269,19 @@ namespace build2 extern execution_mode current_mode; - // Total number of dependency relationships in the current action. Together - // with the target::dependents count it is incremented during the rule - // search & match phase and is decremented during execution with the - // expectation of it reaching 0. Used as a sanity check. + // Total number of dependency relationships and targets with non-noop + // recipe in the current action. + // + // Together with target::dependents the dependency count is incremented + // during the rule search & match phase and is decremented during execution + // with the expectation of it reaching 0. Used as a sanity check. + // + // The target count is incremented after a non-noop recipe is matched and + // decremented after such recipe has been executed. Used for progress + // monitoring. // extern atomic_count dependency_count; + extern atomic_count target_count; inline void set_current_mif (const meta_operation_info& mif) @@ -293,7 +300,10 @@ namespace build2 current_outer_oif = outer_oif; current_on++; current_mode = inner_oif.mode; - dependency_count.store (0, memory_order_relaxed); // Serial. + + // Serial. + dependency_count.store (0, memory_order_relaxed); + target_count.store (0, memory_order_relaxed); } // Keep going flag. @@ -357,6 +367,9 @@ namespace build2 return os; } + string + diag_do (const action&); + void diag_do (ostream&, const action&, const target&); @@ -366,6 +379,9 @@ namespace build2 return diag_phrase {a, t, &diag_do}; } + string + diag_doing (const action&); + void diag_doing (ostream&, const action&, const target&); @@ -375,6 +391,9 @@ namespace build2 return diag_phrase {a, t, &diag_doing}; } + string + diag_did (const action&); + void diag_did (ostream&, const action&, const target&); diff --git a/build2/operation.cxx b/build2/operation.cxx index 7dea25b..a5424bc 100644 --- a/build2/operation.cxx +++ b/build2/operation.cxx @@ -101,32 +101,63 @@ namespace build2 { phase_lock l (run_phase::match); + // Setup progress reporting if requested. + // + scheduler::monitor_guard mg; + string what; + if (ops.progress ()) + { + what = " targets to " + diag_do (a); + + mg = sched.monitor ( + target_count, + 20, + [&what] (size_t c) -> size_t + { + diag_progress_lock pl; + diag_progress = ' '; + diag_progress += to_string (c); + diag_progress += what; + return c + 20; + }); + } + // Start asynchronous matching of prerequisites keeping track of how // many we have started. Wait with unlocked phase to allow phase // switching. // - atomic_count task_count (0); - wait_guard wg (task_count, true); - size_t i (0), n (ts.size ()); - for (; i != n; ++i) { - const target& t (*static_cast (ts[i])); - l5 ([&]{trace << diag_doing (a, t);}); - - target_state s (match_async (a, t, 0, task_count, false)); + atomic_count task_count (0); + wait_guard wg (task_count, true); - // Bail out if the target has failed and we weren't instructed to - // keep going. - // - if (s == target_state::failed && !keep_going) + for (; i != n; ++i) { - ++i; - break; + const target& t (*static_cast (ts[i])); + l5 ([&]{trace << diag_doing (a, t);}); + + target_state s (match_async (a, t, 0, task_count, false)); + + // Bail out if the target has failed and we weren't instructed to + // keep going. + // + if (s == target_state::failed && !keep_going) + { + ++i; + break; + } } + + wg.wait (); } - wg.wait (); + // Clear the progress if present. + // + if (ops.progress ()) + { + diag_progress_lock pl; + diag_progress.clear (); + } // We are now running serially. Re-examine targets that we have matched. // @@ -213,28 +244,66 @@ namespace build2 phase_lock pl (run_phase::execute); // Never switched. + // Setup progress reporting if requested. + // + scheduler::monitor_guard mg; + string what; + if (ops.progress ()) + { + what = "% " + diag_did (a); + + size_t init (target_count.load (memory_order_relaxed)); + size_t incr (init / 100); // 1%. + if (incr == 0) + incr = 1; + + mg = sched.monitor ( + target_count, + init - incr, + [&what, init, incr] (size_t c) -> size_t + { + size_t p ((init - c) * 100 / init); + diag_progress_lock pl; + diag_progress = ' '; + diag_progress += to_string (p); + diag_progress += what; + return c - incr; + }); + } + // Similar logic to execute_members(): first start asynchronous execution // of all the top-level targets. // - atomic_count task_count (0); - wait_guard wg (task_count); - - for (const void* vt: ts) { - const target& t (*static_cast (vt)); + atomic_count task_count (0); + wait_guard wg (task_count); + + for (const void* vt: ts) + { + const target& t (*static_cast (vt)); - l5 ([&]{trace << diag_doing (a, t);}); + l5 ([&]{trace << diag_doing (a, t);}); - target_state s (execute_async (a, t, 0, task_count, false)); + target_state s (execute_async (a, t, 0, task_count, false)); - // Bail out if the target has failed and we weren't instructed to keep - // going. - // - if (s == target_state::failed && !keep_going) - break; + // Bail out if the target has failed and we weren't instructed to keep + // going. + // + if (s == target_state::failed && !keep_going) + break; + } + + wg.wait (); + } + + // Clear the progress if present. + // + if (ops.progress ()) + { + diag_progress_lock pl; + diag_progress.clear (); } - wg.wait (); sched.tune (0); // Restore original scheduler settings. // We are now running serially. Re-examine them all. @@ -291,6 +360,7 @@ namespace build2 // We should have executed every target that we matched, provided we // haven't failed (in which case we could have bailed out early). // + assert (target_count.load (memory_order_relaxed) == 0); assert (dependency_count.load (memory_order_relaxed) == 0); } diff --git a/build2/scheduler.cxx b/build2/scheduler.cxx index fdf4121..65684eb 100644 --- a/build2/scheduler.cxx +++ b/build2/scheduler.cxx @@ -388,6 +388,17 @@ namespace build2 return r; } + scheduler::monitor_guard scheduler:: + monitor (atomic_count& c, size_t t, function f) + { + assert (monitor_count_ == nullptr && t != 0); + monitor_count_ = &c; + monitor_tshold_.store (t, memory_order_relaxed); + monitor_init_ = c.load (memory_order_relaxed); + monitor_func_ = move (f); + return monitor_guard (this); + } + void scheduler:: activate_helper (lock& l) { diff --git a/build2/scheduler.hxx b/build2/scheduler.hxx index ad14c56..33d0709 100644 --- a/build2/scheduler.hxx +++ b/build2/scheduler.hxx @@ -25,7 +25,7 @@ namespace build2 // done in parallel (e.g., update all the prerequisites or run all the // tests). To acomplish this, the master, via a call to async(), can ask the // scheduler to run a task in another thread (called "helper"). If a helper - // is available, then the task is executed asynchronously by such helper. + // is available, then the task is executed asynchronously by such a helper. // Otherwise, the task is (normally) executed synchronously as part of the // wait() call below. However, in certain cases (serial execution or full // queue), the task may be executed synchronously as part of the async() @@ -226,6 +226,49 @@ namespace build2 stat shutdown (); + // Progress monitoring. + // + // Setting and clearing of the monitor is not thread-safe. That is, it + // should be set before any tasks are queued and cleared after all of + // them have completed. + // + // The counter must go in one direction, either increasing or decreasing, + // and should contain the initial value during the call. Zero threshold + // value is reserved. + // + struct monitor_guard + { + explicit + monitor_guard (scheduler* s = nullptr): s_ (s) {} + + ~monitor_guard () + { + if (s_ != nullptr) + { + s_->monitor_count_ = nullptr; + s_->monitor_func_ = nullptr; + } + } + + monitor_guard (monitor_guard&& x): s_ (x.s_) {x.s_ = nullptr;} + + monitor_guard& operator= (monitor_guard&& x) + { + if (&x != this) + { + s_ = x.s_; + x.s_ = nullptr; + } + return *this; + } + + private: + scheduler* s_; + }; + + monitor_guard + monitor (atomic_count&, size_t threshold, function); + // If initially active thread(s) (besides the one that calls startup()) // exist before the call to startup(), then they must call join() before // executing any tasks. The two common cases where you don't have to call @@ -321,8 +364,14 @@ namespace build2 decay_copy (T&& x) {return forward (x);} private: - std::mutex mutex_; + // Monitor. + // + atomic_count* monitor_count_ = nullptr; // NULL if not used. + atomic_count monitor_tshold_; // 0 means locked. + size_t monitor_init_; // Initial count. + function monitor_func_; + std::mutex mutex_; bool shutdown_ = true; // Shutdown flag. // The constraints that we must maintain: @@ -502,13 +551,7 @@ namespace build2 if (--s == 0 || a) m = h; // Reset or adjust the mark. - queued_task_count_.fetch_sub (1, std::memory_order_release); - - // The thunk moves the task data to its stack, releases the lock, - // and continues to execute the task. - // - td.thunk (*this, ql, &td.data); - ql.lock (); + execute (ql, td); } bool @@ -539,10 +582,7 @@ namespace build2 t = s != 1 ? (t != 0 ? t - 1 : task_queue_depth_ - 1) : t; --s; - queued_task_count_.fetch_sub (1, std::memory_order_release); - - td.thunk (*this, ql, &td.data); - ql.lock (); + execute (ql, td); // Restore the old mark (which we might have to adjust). // @@ -569,6 +609,53 @@ namespace build2 m = om; } + void + execute (lock& ql, task_data& td) + { + queued_task_count_.fetch_sub (1, std::memory_order_release); + + // The thunk moves the task data to its stack, releases the lock, + // and continues to execute the task. + // + td.thunk (*this, ql, &td.data); + + // See if we need to call the monitor. + // + if (monitor_count_ != nullptr) + { + // Note that we don't care if we don't see the updated values right + // away. + // + if (size_t t = monitor_tshold_.load (memory_order_relaxed)) + { + // "Lock" the monitor by setting threshold to 0. + // + if (monitor_tshold_.compare_exchange_strong ( + t, + 0, + memory_order_release, + memory_order_relaxed)) + { + // Now we are the only ones messing with this. + // + size_t v (monitor_count_->load (memory_order_relaxed)); + + if (v != monitor_init_) + { + // See which direction we are going. + // + if (v > monitor_init_ ? (v >= t) : (v <= t)) + t = monitor_func_ (v); + } + + monitor_tshold_.store (t, memory_order_release); + } + } + } + + ql.lock (); + } + // Each thread has its own queue which are stored in this list. // std::list task_queues_; diff --git a/build2/target.ixx b/build2/target.ixx index d2edf89..48036be 100644 --- a/build2/target.ixx +++ b/build2/target.ixx @@ -124,6 +124,8 @@ namespace build2 return r; } + extern atomic_count target_count; // context.hxx + inline void target:: recipe (recipe_type r) { @@ -135,16 +137,21 @@ namespace build2 // if (state_ != target_state::failed) { + state_ = target_state::unknown; + // If this is a noop recipe, then mark the target unchanged to allow for // some optimizations. // - state_ = target_state::unknown; - - if (recipe_function** f = recipe_.target ()) - { - if (*f == &noop_action) - state_ = target_state::unchanged; - } + recipe_function** f (recipe_.target ()); + + if (f != nullptr && *f == &noop_action) + state_ = target_state::unchanged; + else + // This gets tricky when we start considering overrides (which can + // only happen for noop recipes), direct execution, etc. So here seems + // like the best place to do this. + // + target_count.fetch_add (1, memory_order_relaxed); } } -- cgit v1.1