diff options
author | Boris Kolpackov <boris@codesynthesis.com> | 2017-02-10 08:15:48 +0200 |
---|---|---|
committer | Boris Kolpackov <boris@codesynthesis.com> | 2017-02-13 12:42:42 +0200 |
commit | abccaf9596461215fce0e32322133fb6c39be44f (patch) | |
tree | 3fc16a6e6142d65e6b47ae686ab845cc164478cc | |
parent | bcb2a89e111a918a48a132a2a29e0c26d724591d (diff) |
Implement parallel error propagation, keep_going mode
Keep going is the default but there is now the -s|--serial-stop that makes
the driver run serially and stop at first error.
Also fix some lockups, other minor improvements/features.
-rw-r--r-- | build2/algorithm | 12 | ||||
-rw-r--r-- | build2/algorithm.cxx | 208 | ||||
-rw-r--r-- | build2/algorithm.ixx | 19 | ||||
-rw-r--r-- | build2/b-options | 4 | ||||
-rw-r--r-- | build2/b-options.cxx | 10 | ||||
-rw-r--r-- | build2/b-options.ixx | 6 | ||||
-rw-r--r-- | build2/b.cli | 7 | ||||
-rw-r--r-- | build2/b.cxx | 57 | ||||
-rw-r--r-- | build2/cc/compile.cxx | 6 | ||||
-rw-r--r-- | build2/config/operation.cxx | 2 | ||||
-rw-r--r-- | build2/context | 19 | ||||
-rw-r--r-- | build2/context.cxx | 37 | ||||
-rw-r--r-- | build2/diagnostics.cxx | 2 | ||||
-rw-r--r-- | build2/dist/operation.cxx | 1 | ||||
-rw-r--r-- | build2/install/operation.cxx | 10 | ||||
-rw-r--r-- | build2/install/rule.cxx | 6 | ||||
-rw-r--r-- | build2/operation | 7 | ||||
-rw-r--r-- | build2/operation.cxx | 79 | ||||
-rw-r--r-- | build2/scheduler | 10 | ||||
-rw-r--r-- | build2/target | 43 | ||||
-rw-r--r-- | build2/target.cxx | 2 | ||||
-rw-r--r-- | build2/target.ixx | 35 | ||||
-rw-r--r-- | build2/test/operation.cxx | 2 | ||||
-rw-r--r-- | build2/test/rule.cxx | 4 | ||||
-rw-r--r-- | build2/test/script/parser | 6 | ||||
-rw-r--r-- | build2/test/script/parser.cxx | 86 |
26 files changed, 479 insertions, 201 deletions
diff --git a/build2/algorithm b/build2/algorithm index 5ee6c64..33da884 100644 --- a/build2/algorithm +++ b/build2/algorithm @@ -149,9 +149,11 @@ namespace build2 // Execute the action on target, assuming a rule has been matched and the // recipe for this action has been set. This is the synchrounous executor - // implementation (but may still return target_state::busy is the target + // implementation (but may still return target_state::busy if the target // is already being executed). Decrements the dependents count. // + // Note: does not translate target_state::failed to the failed exception. + // target_state execute (action, const target&); @@ -159,6 +161,8 @@ namespace build2 // if the asynchrounous execution has been started and target_state::busy if // the target has already been busy. // + // Note: does not translate target_state::failed to the failed exception. + // target_state execute_async (action, const target&, size_t start_count, atomic_count& task_count); @@ -174,8 +178,10 @@ namespace build2 // A special version of the above that should be used for "direct" and "now" // execution, that is, side-stepping the normal target-prerequisite // relationship (so no dependents count is decremented) and execution order - // (so this function never returns the postponed target state). It will also - // wait for the completion if the target is busy. + // (so this function never returns the postponed target state). + // + // Note: waits for the completion if the target is busy and translates + // target_state::failed to the failed exception. // target_state execute_direct (action, const target&); diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index 3887c64..0aad5b6 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -387,41 +387,53 @@ namespace build2 return r; } - inline target_state - execute_impl (action a, target& t) + target_state + execute_impl (action a, target& t) noexcept { // Task count should be count_executing. // assert (t.state_ == target_state::unknown); - auto g ( - make_exception_guard ( - [a, &t]() - { - t.state_ = target_state::failed; - - if (verb != 0) - info << "while " << diag_doing (a, t); - })); + target_state ts; - target_state ts (t.recipe (a) (a, t)); + try + { + ts = t.recipe (a) (a, t); - // The the recipe documentation for details on what's going on here. - // - switch (t.state_ = ts) + // 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. + // + switch (t.state_ = ts) + { + case target_state::changed: + case target_state::unchanged: + break; + case target_state::postponed: + ts = t.state_ = target_state::unchanged; + break; + case target_state::group: + ts = t.group->state_; + break; + default: + assert (false); + } + } + catch (const failed&) { - case target_state::changed: - case target_state::unchanged: break; - case target_state::postponed: t.state_ = target_state::unknown; break; - case target_state::group: ts = t.group->state_; break; - default: assert (false); + // The "how we got here" stack trace is useful but only during serial + // execution in the "stop going" mode. Otherwise, you not only get + // things interleaved, you may also get a whole bunch of such stacks. + // + if (verb != 0 && sched.serial () && !keep_going) + info << "while " << diag_doing (a, t); + + ts = t.state_ = target_state::failed; } // Decrement the task count (to count_executed) and wake up any threads // that might be waiting for this target. // - // @@ MT: not happenning in case of an exception! - // size_t tc (t.task_count--); assert (tc == target::count_executing); sched.resume (t.task_count); @@ -441,15 +453,14 @@ namespace build2 // Update dependency counts and make sure they are not skew. // - size_t d; - { - // Note: memory order can probably be relaxed. - // - size_t g (dependency_count--); - d = t.dependents--; - assert (g != 0 && d != 0); - d--; - } + size_t td (t.dependents--); +#ifndef NDEBUG + size_t gd (dependency_count--); + assert (td != 0 && gd != 0); +#else + dependency_count.fetch_sub (1, std::memory_order_release); +#endif + td--; // Handle the "last" execution mode. // @@ -472,7 +483,7 @@ namespace build2 // thread. For other threads the state will still be unknown (until they // try to execute it). // - if (current_mode == execution_mode::last && d != 0) + if (current_mode == execution_mode::last && td != 0) return target_state::postponed; // Try to atomically change unexecuted to executing. @@ -483,23 +494,31 @@ namespace build2 if (task_count == nullptr) return execute_impl (a, t); - sched.async (start_count, - *task_count, - [a] (target& t) - { - execute_impl (a, t); // @@ MT exception handling. - }, - ref (t)); + //text << this_thread::get_id () << " async " << t; - return target_state::unknown; - } + if (sched.async (start_count, + *task_count, + [a] (target& t) + { + //text << this_thread::get_id () << " deque " << t; + execute_impl (a, t); // Note: noexcept. + }, + ref (t))) + return target_state::unknown; // Queued. - switch (tc) + // Executed synchronously, fall through. + } + else { - case target::count_unexecuted: assert (false); - case target::count_executed: return t.synchronized_state (); - default: return target_state::busy; + switch (tc) + { + case target::count_unexecuted: assert (false); + case target::count_executed: break; + default: return target_state::busy; + } } + + return t.synchronized_state (false); } target_state @@ -509,16 +528,19 @@ namespace build2 size_t tc (target::count_unexecuted); if (t.task_count.compare_exchange_strong (tc, target::count_executing)) - return execute_impl (a, t); - - // If the target is busy, wait for it. - // - switch (tc) { - case target::count_unexecuted: assert (false); - case target::count_executed: break; - default: sched.wait (target::count_executed, - t.task_count); + execute_impl (a, t); + } + else + { + // If the target is busy, wait for it. + // + switch (tc) + { + case target::count_unexecuted: assert (false); + case target::count_executed: break; + default: sched.wait (target::count_executed, t.task_count, false); + } } return t.synchronized_state (); @@ -563,9 +585,11 @@ namespace build2 { target_state r (target_state::unchanged); - // Start asynchronous execution of prerequisites. + // Start asynchronous execution of prerequisites keeping track of how many + // we have handled. // - for (size_t i (0); i != n; ++i) + size_t i (0); + for (; i != n; ++i) { const target*& mt (ts[i]); @@ -583,15 +607,24 @@ namespace build2 } else if (s == target_state::busy) set_busy (mt); + // + // Bail out if the target has failed and we weren't instructed to + // keep going. + // + else if (s == target_state::failed && !keep_going) + { + ++i; + break; + } } sched.wait (target::count_executing, t.task_count); // Now all the targets in prerequisite_targets must be executed and // synchronized (and we have blanked out all the postponed ones). // - for (size_t i (0); i != n; ++i) + for (size_t j (0); j != i; ++j) { - const target*& mt (ts[i]); + const target*& mt (ts[j]); if (mt == nullptr) continue; @@ -599,11 +632,14 @@ namespace build2 // If the target was already busy, wait for its completion. // if (get_busy (mt)) - sched.wait (target::count_executed, mt->task_count); + sched.wait (target::count_executed, mt->task_count, false); - r |= mt->synchronized_state (); + r |= mt->synchronized_state (false); } + if (r == target_state::failed) + throw failed (); + return r; } @@ -617,7 +653,8 @@ namespace build2 // target_state r (target_state::unchanged); - for (size_t i (n); i != 0; --i) + size_t i (n); + for (; i != 0; --i) { const target*& mt (ts[i - 1]); @@ -635,24 +672,30 @@ namespace build2 } else if (s == target_state::busy) set_busy (mt); + else if (s == target_state::failed && !keep_going) + { + --i; + break; + } } sched.wait (target::count_executing, t.task_count); - for (size_t i (n); i != 0; --i) + for (size_t j (n); j != i; --j) { - const target*& mt (ts[i - 1]); + const target*& mt (ts[j - 1]); if (mt == nullptr) continue; - // If the target was already busy, wait for its completion. - // if (get_busy (mt)) - sched.wait (target::count_executed, mt->task_count); + sched.wait (target::count_executed, mt->task_count, false); - r |= mt->synchronized_state (); + r |= mt->synchronized_state (false); } + if (r == target_state::failed) + throw failed (); + return r; } @@ -667,8 +710,11 @@ namespace build2 // target_state rs (target_state::unchanged); - for (const target*& pt: t.prerequisite_targets) + size_t i (0); + for (size_t n (t.prerequisite_targets.size ()); i != n; ++i) { + const target*& pt (t.prerequisite_targets[i]); + if (pt == nullptr) // Skipped. continue; @@ -683,25 +729,37 @@ namespace build2 } else if (s == target_state::busy) set_busy (pt); + else if (s == target_state::failed && !keep_going) + { + ++i; + break; + } } sched.wait (target::count_executing, t.task_count); bool e (mt == timestamp_nonexistent); const target* rt (tt != nullptr ? nullptr : &t); - for (const target*& pt: t.prerequisite_targets) + for (size_t j (0); j != i; ++j) { + const target*& pt (t.prerequisite_targets[j]); + if (pt == nullptr) continue; // If the target was already busy, wait for its completion. // if (get_busy (pt)) - sched.wait (target::count_executed, pt->task_count); + sched.wait (target::count_executed, pt->task_count, false); - target_state s (pt->synchronized_state ()); + target_state s (pt->synchronized_state (false)); rs |= s; + // Don't bother with the rest if we are failing. + // + if (rs == target_state::failed) + continue; + // Should we compare the timestamp to this target's? // if (!e && (!pf || pf (*pt))) @@ -731,6 +789,9 @@ namespace build2 rt = pt; } + if (rs == target_state::failed) + throw failed (); + assert (rt != nullptr); return make_pair (e ? rt : nullptr, rs); } @@ -750,9 +811,10 @@ namespace build2 // const target& g (*t.group); if (execute (a, g) == target_state::busy) - sched.wait (target::count_executed, g.task_count); + sched.wait (target::count_executed, g.task_count, false); - // Indicate to execute() that this target's state comes from the group. + // Indicate to execute() that this target's state comes from the group + // (which, BTW, can be failed). // return target_state::group; } diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx index 33a5fe0..6bc310f 100644 --- a/build2/algorithm.ixx +++ b/build2/algorithm.ixx @@ -71,10 +71,8 @@ namespace build2 if (!t.recipe (a)) match_impl (ml, a, t, true); - //@@ MT - // - t.dependents++; - dependency_count++; + t.dependents.fetch_add (1, std::memory_order_release); + dependency_count.fetch_add (1, std::memory_order_release); // text << "M " << t << ": " << t.dependents << " " << dependency_count; } @@ -86,11 +84,14 @@ namespace build2 assert (phase == run_phase::search_match); - //@@ MT - // - assert (t.dependents != 0 && dependency_count != 0); - t.dependents--; - dependency_count--; +#ifndef NDEBUG + size_t td (t.dependents--); + size_t gd (dependency_count--); + assert (td != 0 && gd != 0); +#else + t.dependents.fetch_sub (1, std::memory_order_release); + dependency_count.fetch_sub (1, std::memory_order_release); +#endif } inline void diff --git a/build2/b-options b/build2/b-options index f05f483..9f787cf 100644 --- a/build2/b-options +++ b/build2/b-options @@ -417,6 +417,9 @@ namespace build2 jobs_specified () const; const bool& + serial_stop () const; + + const bool& no_column () const; const bool& @@ -484,6 +487,7 @@ namespace build2 bool verbose_specified_; size_t jobs_; bool jobs_specified_; + bool serial_stop_; bool no_column_; bool no_line_; path buildfile_; diff --git a/build2/b-options.cxx b/build2/b-options.cxx index 37daab6..8a133dd 100644 --- a/build2/b-options.cxx +++ b/build2/b-options.cxx @@ -576,6 +576,7 @@ namespace build2 verbose_specified_ (false), jobs_ (), jobs_specified_ (false), + serial_stop_ (), no_column_ (), no_line_ (), buildfile_ ("buildfile"), @@ -696,6 +697,11 @@ namespace build2 << " threads is used." << ::std::endl; os << std::endl + << "\033[1m--serial-stop\033[0m|\033[1m-s\033[0m Run serially and stop at the first error. This mode is" << ::std::endl + << " useful to investigate build failured that are caused by" << ::std::endl + << " build system errors rather than compilation errors." << ::std::endl; + + os << std::endl << "\033[1m--no-column\033[0m Don't print column numbers in diagnostics." << ::std::endl; os << std::endl @@ -777,6 +783,10 @@ namespace build2 _cli_options_map_["-j"] = &::build2::cl::thunk< options, size_t, &options::jobs_, &options::jobs_specified_ >; + _cli_options_map_["--serial-stop"] = + &::build2::cl::thunk< options, bool, &options::serial_stop_ >; + _cli_options_map_["-s"] = + &::build2::cl::thunk< options, bool, &options::serial_stop_ >; _cli_options_map_["--no-column"] = &::build2::cl::thunk< options, bool, &options::no_column_ >; _cli_options_map_["--no-line"] = diff --git a/build2/b-options.ixx b/build2/b-options.ixx index 190c960..61d0f11 100644 --- a/build2/b-options.ixx +++ b/build2/b-options.ixx @@ -259,6 +259,12 @@ namespace build2 } inline const bool& options:: + serial_stop () const + { + return this->serial_stop_; + } + + inline const bool& options:: no_column () const { return this->no_column_; diff --git a/build2/b.cli b/build2/b.cli index c2a34c8..6e6800f 100644 --- a/build2/b.cli +++ b/build2/b.cli @@ -251,6 +251,13 @@ namespace build2 is used." } + bool --serial-stop|-s + { + "Run serially and stop at the first error. This mode is useful to + investigate build failured that are caused by build system errors + rather than compilation errors." + } + bool --no-column { "Don't print column numbers in diagnostics." diff --git a/build2/b.cxx b/build2/b.cxx index 0ad404a..14258b3 100644 --- a/build2/b.cxx +++ b/build2/b.cxx @@ -67,6 +67,10 @@ main (int argc, char* argv[]) int build2:: main (int argc, char* argv[]) { + tracer trace ("main"); + + int r (0); + // This is a little hack to make out baseutils for Windows work when called // with absolute path. In a nutshell, MSYS2's exec*p() doesn't search in the // parent's executable directory, only in PATH. And since we are running @@ -93,8 +97,6 @@ main (int argc, char* argv[]) try { - tracer trace ("main"); - // On POSIX ignore SIGPIPE which is signaled to a pipe-writing process if // the pipe reading end is closed. Note that by default this signal // terminates a process. Also note that there is no way to disable this @@ -225,8 +227,7 @@ main (int argc, char* argv[]) // catch (const system_error& e) { - error << "pager failed: " << e; - return 1; + fail << "pager failed: " << e; } } @@ -270,12 +271,16 @@ main (int argc, char* argv[]) bm["cli"] = mf {nullptr, &cli::init}; } + keep_going = !ops.serial_stop (); + // Start up the scheduler. // size_t jobs (0); if (ops.jobs_specified ()) jobs = ops.jobs (); + else if (ops.serial_stop ()) + jobs = 1; if (jobs == 0) jobs = scheduler::hardware_concurrency (); @@ -1097,32 +1102,32 @@ main (int argc, char* argv[]) if (lifted == nullptr && skip == 0) ++mit; } // meta-operation - - // Shutdown the scheduler. - // - scheduler::stat st (sched.shutdown ()); - - if (verb >= (jobs > 1 ? 3 : 4)) - { - info << "scheduler statistics:" << '\n' - << " thread_max_active " << st.thread_max_active << '\n' - << " thread_max_total " << st.thread_max_total << '\n' - << " thread_helpers " << st.thread_helpers << '\n' - << " thread_max_waiting " << st.thread_max_waiting << '\n' - << '\n' - << " task_queue_depth " << st.task_queue_depth << '\n' - << " task_queue_full " << st.task_queue_full << '\n' - << '\n' - << " wait_queue_slots " << st.wait_queue_slots << '\n' - << " wait_queue_collisions " << st.wait_queue_collisions << '\n'; - } - - return 0; } catch (const failed&) { // Diagnostics has already been issued. + // + r = 1; + } + + // Shutdown the scheduler and print statistics. + // + scheduler::stat st (sched.shutdown ()); + + if (verb >= (st.thread_max_active > 1 ? 3 : 4)) + { + info << "scheduler statistics:" << "\n\n" + << " thread_max_active " << st.thread_max_active << '\n' + << " thread_max_total " << st.thread_max_total << '\n' + << " thread_helpers " << st.thread_helpers << '\n' + << " thread_max_waiting " << st.thread_max_waiting << '\n' + << '\n' + << " task_queue_depth " << st.task_queue_depth << '\n' + << " task_queue_full " << st.task_queue_full << '\n' + << '\n' + << " wait_queue_slots " << st.wait_queue_slots << '\n' + << " wait_queue_collisions " << st.wait_queue_collisions << '\n'; } - return 1; + return r; } diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index e08b40b..c8394a9 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -891,9 +891,7 @@ namespace build2 // auto update = [&trace, a] (path_target& pt, timestamp ts) -> bool { - //@@ MT extenal modification sync. - - target_state os (pt.atomic_state ()); //@@ MT: do we need atomic? + target_state os (pt.synchronized_state ()); //@@ MT? matched? if (os != target_state::unchanged) { @@ -902,7 +900,7 @@ namespace build2 // have been in target_state::changed because of a dependency // extraction run for some other source file. // - target_state ns (execute_direct (a, pt)); + target_state ns (execute_direct (a, pt)); //@@ MT extenal modification sync. if (ns != os && ns != target_state::unchanged) { diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx index 7bed621..ac18d82 100644 --- a/build2/config/operation.cxx +++ b/build2/config/operation.cxx @@ -402,6 +402,7 @@ namespace build2 "configure", "configure", "configuring", + "configured", "is configured", nullptr, // meta-operation pre &configure_operation_pre, @@ -627,6 +628,7 @@ namespace build2 "disfigure", "disfigure", "disfiguring", + "disfigured", "is disfigured", nullptr, // meta-operation pre &disfigure_operation_pre, diff --git a/build2/context b/build2/context index da2ced5..3c731a5 100644 --- a/build2/context +++ b/build2/context @@ -111,7 +111,7 @@ namespace build2 // } // } // - // sched.wait (); // (3) + // sched.wait (task_count); // (3) // // Here is what's going on here: // @@ -237,6 +237,14 @@ namespace build2 current_mode = inner_oif.mode; } + // Keep going flag. + // + // Note that setting it to false is not of much help unless we are running + // serially. In parallel we queue most of the things up before we see any + // failures. + // + extern bool keep_going; + // 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 @@ -321,6 +329,15 @@ namespace build2 } void + diag_did (ostream&, const action&, const target&); + + inline diag_phrase + diag_did (const action& a, const target& t) + { + return diag_phrase {a, t, &diag_did}; + } + + void diag_done (ostream&, const action&, const target&); inline diag_phrase diff --git a/build2/context.cxx b/build2/context.cxx index 400f68c..0357eb6 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -56,6 +56,8 @@ namespace build2 execution_mode current_mode; + bool keep_going = false; + atomic_count dependency_count; variable_override_cache var_override_cache; @@ -461,6 +463,35 @@ namespace build2 } void + diag_did (ostream& os, const action&, const target& t) + { + const meta_operation_info& m (*current_mif); + const operation_info& io (*current_inner_oif); + const operation_info* oo (current_outer_oif); + + // perform(update(x)) -> "updated x" + // configure(update(x)) -> "configured updating x" + // + if (!m.name_did.empty ()) + { + os << m.name_did << ' '; + + if (!io.name_doing.empty ()) + os << io.name_doing << ' '; + } + else + { + if (!io.name_did.empty ()) + os << io.name_did << ' '; + } + + if (oo != nullptr) + os << "(for " << oo->name << ") "; + + os << t; + } + + void diag_done (ostream& os, const action&, const target& t) { const meta_operation_info& m (*current_mif); @@ -475,10 +506,10 @@ namespace build2 os << t; if (!io.name_done.empty ()) - os << " " << io.name_done; + os << ' ' << io.name_done; if (oo != nullptr) - os << "(for " << oo->name << ") "; + os << " (for " << oo->name << ')'; } else { @@ -488,7 +519,7 @@ namespace build2 if (oo != nullptr) os << "(for " << oo->name << ") "; - os << t << " " << m.name_done; + os << t << ' ' << m.name_done; } } } diff --git a/build2/diagnostics.cxx b/build2/diagnostics.cxx index 0ddb529..26fd9f0 100644 --- a/build2/diagnostics.cxx +++ b/build2/diagnostics.cxx @@ -42,7 +42,7 @@ namespace build2 // Diagnostics verbosity level. // - uint16_t verb; + uint16_t verb = 0; // Keep disabled until set from options. // Diagnostic facility, project specifics. // diff --git a/build2/dist/operation.cxx b/build2/dist/operation.cxx index df0c7dd..e087069 100644 --- a/build2/dist/operation.cxx +++ b/build2/dist/operation.cxx @@ -477,6 +477,7 @@ namespace build2 "dist", "distribute", "distributing", + "distributed", "has nothing to distribute", // We cannot "be distributed". nullptr, // meta-operation pre &dist_operation_pre, diff --git a/build2/install/operation.cxx b/build2/install/operation.cxx index 56f65e1..9873ac2 100644 --- a/build2/install/operation.cxx +++ b/build2/install/operation.cxx @@ -19,13 +19,21 @@ namespace build2 return mo != disfigure_id ? update_id : 0; } + // Note that we run both install and uninstall serially. The reason for + // this is all the fuzzy things we are trying to do like removing empty + // outer directories if they are empty. If we do this in parallel, then + // those things get racy. Also, since all we do here is creating/removing + // files, there is not going to be much speedup from doing it in parallel. + const operation_info install { install_id, "install", "install", "installing", + "installed", "has nothing to install", // We cannot "be installed". execution_mode::first, + 0, &install_pre, nullptr }; @@ -44,8 +52,10 @@ namespace build2 "uninstall", "uninstall", "uninstalling", + "uninstalled", "is not installed", execution_mode::last, + 0, &install_pre, nullptr }; diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx index 13d8919..323060d 100644 --- a/build2/install/rule.cxx +++ b/build2/install/rule.cxx @@ -181,10 +181,10 @@ namespace build2 // will help a lot in case of any static installable content // (headers, documentation, etc). // - if (pt->synchronized_state () != target_state::unchanged) //@@ MT? - t.prerequisite_targets.push_back (pt); - else + if (pt->unchanged ()) //@@ MT? unmatch (a, *pt); // No intent to execute. + else + t.prerequisite_targets.push_back (pt); // Skip members of ad hoc groups. We handle them explicitly below. // diff --git a/build2/operation b/build2/operation index 700ee7e..544a9f9 100644 --- a/build2/operation +++ b/build2/operation @@ -185,6 +185,7 @@ namespace build2 // const string name_do; // E.g., [to] 'configure'. const string name_doing; // E.g., [while] 'configuring'. + const string name_did; // E.g., 'configured'. const string name_done; // E.g., 'is configured'. // If operation_pre() is not NULL, then it may translate default_id @@ -269,10 +270,16 @@ namespace build2 // const string name_do; // E.g., [to] 'update'. const string name_doing; // E.g., [while] 'updating'. + const string name_did; // E.g., [not] 'updated'. const string name_done; // E.g., 'is up to date'. const execution_mode mode; + // This is the operation's concurrency multiplier. 0 means run serially, + // 1 means run at hardware concurrency (unless overridden by the user). + // + const size_t concurrency; + // If the returned operation_id's are not 0, then they are injected // as pre/post operations for this operation. Can be NULL if unused. // The returned operation_id shall not be default_id. diff --git a/build2/operation.cxx b/build2/operation.cxx index ea9a462..c4366f7 100644 --- a/build2/operation.cxx +++ b/build2/operation.cxx @@ -135,47 +135,90 @@ namespace build2 phase_guard pg (run_phase::execute); + // Tune the scheduler. + // + switch (current_inner_oif->concurrency) + { + case 0: sched.tune (1); break; // Run serially. + case 1: break; // Run as is. + default: assert (false); // Not yet supported. + } + // Similar logic to execute_members(): first start asynchronous execution // of all the top-level targets. // atomic_count task_count (0); - for (const void* v: ts) + + size_t n (ts.size ()); + for (size_t i (0); i != n; ++i) { - const target& t (*static_cast<const target*> (v)); + const target& t (*static_cast<const target*> (ts[i])); l5 ([&]{trace << diag_doing (a, t);}); - execute_async (a, t, 0, task_count); + target_state s (execute_async (a, t, 0, task_count)); + + if (s == target_state::failed && !keep_going) + break; } sched.wait (task_count); + sched.tune (0); // Restore original scheduler settings. - // We are now running serially and we should have executed every target - // that we matched. + // We are now running serially. Re-examine them all. // - assert (dependency_count == 0); - - for (const void* v: ts) + bool fail (false); + for (size_t i (0); i != n; ++i) { - const target& t (*static_cast<const target*> (v)); + const target& t (*static_cast<const target*> (ts[i])); - switch (t.synchronized_state ()) + switch (t.synchronized_state (false)) { + case target_state::unknown: + { + // We bailed before executing it. + // + if (!quiet) + info << "not " << diag_did (a, t); + + break; + } case target_state::unchanged: { + // Nothing had to be done. + // if (!quiet) info << diag_done (a, t); + break; } case target_state::changed: - break; + { + // Something has been done. + // + break; + } case target_state::failed: - //@@ MT: This could probably happen in a parallel build. - // Or does state() throw? Do we want to print? - break; + { + // Things didn't go well for this target. + // + if (!quiet) + info << "failed to " << diag_do (a, t); + + fail = true; + break; + } default: assert (false); } } + + if (fail) + throw failed (); + + // We should have executed every target that we matched, provided we + // haven't failed (in which case we could have bailed out early). + // + assert (dependency_count == 0); } const meta_operation_info noop { @@ -184,6 +227,7 @@ namespace build2 "", // Presumably we will never need these since we are not going "", // to do anything. "", + "", nullptr, // meta-operation pre nullptr, // operation pre &load, @@ -200,6 +244,7 @@ namespace build2 "", "", "", + "", nullptr, // meta-operation pre nullptr, // operation pre &load, @@ -218,7 +263,9 @@ namespace build2 "", "", "", + "", execution_mode::first, + 1, nullptr, nullptr }; @@ -228,8 +275,10 @@ namespace build2 "update", "update", "updating", + "updated", "is up to date", execution_mode::first, + 1, nullptr, nullptr }; @@ -239,8 +288,10 @@ namespace build2 "clean", "clean", "cleaning", + "cleaned", "is clean", execution_mode::last, + 1, nullptr, nullptr }; diff --git a/build2/scheduler b/build2/scheduler index d2eb0fc..c487b88 100644 --- a/build2/scheduler +++ b/build2/scheduler @@ -155,6 +155,14 @@ namespace build2 size_t max_threads = 0, size_t queue_depth = 0); + // Return true if the scheduler was started up. + // + // Note: can only be called from threads that have observed creation, + // startup, or shutdown. + // + bool + started () const {return !shutdown_;} + // Tune a started up scheduler. // // Currently one cannot increase the number of max_active. Pass 0 to @@ -170,6 +178,8 @@ namespace build2 // Return true if the scheduler is running serial. // + // Note: can only be called from threads that have observed startup. + // bool serial () const {return max_active_ == 1;} diff --git a/build2/target b/build2/target index e709395..2e13e69 100644 --- a/build2/target +++ b/build2/target @@ -64,13 +64,16 @@ namespace build2 // Recipe. // // The returned target state is normally changed or unchanged. If there is - // an error, then the recipe should throw rather than returning failed. + // an error, then the recipe should throw failed rather than returning (this + // is the only exception that a recipe can throw). // // The return value of the recipe is used to update the target state. If it // is target_state::group then the target's state is the group's state. // - // The recipe can also return postponed. In this case the target state is - // set to unknown but the postponed state is propagated to the caller. + // The recipe may also return postponed in which case the target state is + // assumed to be unchanged (normally this means a prerequisite was postponed + // and while the prerequisite will be re-examined via another dependency, + // this target is done). // // Note that max size for the "small capture optimization" in std::function // ranges (in pointer sizes) from 0 (GCC prior to 5) to 2 (GCC 5) to 6 (VC @@ -401,13 +404,7 @@ namespace build2 // Target state. // - protected: - friend target_state execute_impl (action, target&); - - target_state state_; - public: - // Atomic task count that is used during execution to (atomically) track a // subset of the target's state as well as the number of its sub-tasks // (execution of prerequisites). @@ -429,16 +426,17 @@ namespace build2 mutable atomic_count task_count; // Return the "stapshot" of the target state. That is, unless the target - // has been executed, its state can change asynchronously. + // has been executed, its state can change asynchronously. If fail is + // true then translate target_state::failed to the failed exception. // target_state - atomic_state () const; + atomic_state (bool fail = true) const; // During execution this function can only be called if we have observed // (synchronization-wise) that this target has been executed. // target_state - synchronized_state () const; + synchronized_state (bool fail = true) const; // Number of direct targets that depend on this target in the current // action. It is incremented during the match phase and then decremented @@ -455,6 +453,17 @@ namespace build2 // atomic_count dependents; + protected: + friend target_state execute_impl (action, target&) noexcept; + + target_state state_; + + // Return fail-untranslated (but group-translated) state assuming the + // target is synchronized. + // + target_state + state () const; + // Auxilary data storage. // public: @@ -530,6 +539,13 @@ namespace build2 void recipe (action_type, recipe_type); + // After the recipe has been set (and target synchronized), check if the + // target is known to be unchanged. Used for various optimizations during + // search & match. + // + bool + unchanged () {return state () == target_state::unchanged;} + private: recipe_type recipe_; @@ -1181,8 +1197,7 @@ namespace build2 // much we can do here except detect the case where the target was // changed on this run. // - return mt < mp || (mt == mp && - synchronized_state () == target_state::changed); + return mt < mp || (mt == mp && state () == target_state::changed); } protected: diff --git a/build2/target.cxx b/build2/target.cxx index b22dfce..a6f7ef4 100644 --- a/build2/target.cxx +++ b/build2/target.cxx @@ -128,7 +128,7 @@ namespace build2 // if we are merely overriding with a "stronger" recipe. // if (!override) - dependents = 0; + dependents = 0; //@@ MT: either relax or use as match flag? } void target:: diff --git a/build2/target.ixx b/build2/target.ixx index a097e39..b73acd7 100644 --- a/build2/target.ixx +++ b/build2/target.ixx @@ -26,18 +26,7 @@ namespace build2 } inline target_state target:: - atomic_state () const - { - switch (task_count) - { - case target::count_unexecuted: return target_state::unknown; - case target::count_executed: return synchronized_state (); - default: return target_state::busy; - } - } - - inline target_state target:: - synchronized_state () const + state () const { // We go an extra step and short-circuit to the target state even if the // raw state is not group provided the recipe is group_recipe. @@ -57,6 +46,28 @@ namespace build2 return state_; } + inline target_state target:: + synchronized_state (bool fail) const + { + target_state r (state ()); + + if (fail && r == target_state::failed) + throw failed (); + + return r; + } + + inline target_state target:: + atomic_state (bool fail) const + { + switch (task_count) + { + case target::count_unexecuted: return target_state::unknown; + case target::count_executed: return synchronized_state (fail); + default: return target_state::busy; + } + } + // prerequisite_member // inline prerequisite prerequisite_member:: diff --git a/build2/test/operation.cxx b/build2/test/operation.cxx index 7f74323..87e3083 100644 --- a/build2/test/operation.cxx +++ b/build2/test/operation.cxx @@ -24,8 +24,10 @@ namespace build2 "test", "test", "testing", + "tested", "has nothing to test", // We cannot "be tested". execution_mode::first, + 1, &test_pre, nullptr }; diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index 1ce6efb..eac9203 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -291,7 +291,7 @@ namespace build2 { build2::match (ml, a, *it); - if (it->synchronized_state () == target_state::unchanged) //@@ TM? + if (it->unchanged ()) //@@ TM? { unmatch (a, *it); it = nullptr; @@ -304,7 +304,7 @@ namespace build2 { build2::match (ml, a, *ot); - if (ot->synchronized_state () == target_state::unchanged) //@@ MT? + if (ot->unchanged ()) //@@ MT? { unmatch (a, *ot); ot = nullptr; diff --git a/build2/test/script/parser b/build2/test/script/parser index fce372e..d5e721f 100644 --- a/build2/test/script/parser +++ b/build2/test/script/parser @@ -161,11 +161,7 @@ namespace build2 // public: void - execute (script& s, runner& r) - { - if (!s.empty ()) - execute (s, s, r); - } + execute (script& s, runner& r); void execute (scope&, script&, runner&); diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx index d4e318b..9d926fa 100644 --- a/build2/test/script/parser.cxx +++ b/build2/test/script/parser.cxx @@ -6,6 +6,7 @@ #include <sstream> +#include <build2/context> // keep_going #include <build2/scheduler> #include <build2/test/script/lexer> @@ -2789,6 +2790,21 @@ namespace build2 // void parser:: + execute (script& s, runner& r) + { + assert (s.state == scope_state::unknown); + + auto g ( + make_exception_guard ( + [&s] () {s.state = scope_state::failed;})); + + if (!s.empty ()) + execute (s, s, r); + + s.state = scope_state::passed; + } + + void parser:: execute (scope& sc, script& s, runner& r) { path_ = nullptr; // Set by replays. @@ -2824,13 +2840,19 @@ namespace build2 scheduler::atomic_count task_count (0); - for (unique_ptr<scope>& chain: g->scopes) + // Start asynchronous execution of inner scopes keeping track of how + // many we have handled. + // + auto i (g->scopes.begin ()); + for (auto e (g->scopes.end ()); i != e; ++i) { + unique_ptr<scope>& chain (*i); + // Check if this scope is ignored (e.g., via config.test). // if (!runner_->test (*chain)) { - chain.reset (); + chain = nullptr; continue; } @@ -2909,43 +2931,47 @@ namespace build2 // scope_ = chain.get (); // exec_scope_body (); // scope_ = os; + + // If the scope was executed synchronously, check the status and + // bail out if we weren't asked to keep going. // - sched.async (task_count, - [] (scope& s, script& scr, runner& r) - { - try - { - parser p; - p.execute (s, scr, r); - s.state = scope_state::passed; - } - catch (const failed&) - { - s.state = scope_state::failed; - } - }, - ref (*chain), - ref (*script_), - ref (*runner_)); + if (!sched.async (task_count, + [] (scope& s, script& scr, runner& r) + { + try + { + parser p; + p.execute (s, scr, r); + s.state = scope_state::passed; + } + catch (const failed&) + { + s.state = scope_state::failed; + } + }, + ref (*chain), + ref (*script_), + ref (*runner_))) + { + if (chain->state == scope_state::failed && !keep_going) + { + ++i; + break; + } + } } } - sched.wait (task_count); - for (unique_ptr<scope>& chain: g->scopes) + // Re-examine the scopes we have executed collecting their state. + // + for (auto j (g->scopes.begin ()); j != i; ++j) { + const unique_ptr<scope>& chain (*j); + if (chain == nullptr) continue; - // @@ Currently we simply re-throw though the default mode should - // probably be "keep going". While we could already do it at - // the testscript level, there is no support for continuing - // testing targets at the build2 level. This will probably all - // fall into place when we add support for parallel builds. - // - // At that stage we should also probably think about the "stop - // on first error" mode (which is what we have now). - // switch (chain->state) { case scope_state::passed: break; |