diff options
| -rw-r--r-- | libbuild2/context.hxx | 3 | ||||
| -rw-r--r-- | libbuild2/install/operation.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/module.cxx | 16 | ||||
| -rw-r--r-- | libbuild2/operation.cxx | 159 | ||||
| -rw-r--r-- | libbuild2/operation.hxx | 7 | ||||
| -rw-r--r-- | libbuild2/scheduler.cxx | 29 | ||||
| -rw-r--r-- | libbuild2/scheduler.hxx | 47 | ||||
| -rw-r--r-- | libbuild2/test/operation.cxx | 2 |
8 files changed, 167 insertions, 100 deletions
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx index 0a2deea..e720321 100644 --- a/libbuild2/context.hxx +++ b/libbuild2/context.hxx @@ -131,6 +131,9 @@ namespace build2 // future we can do save/restore but then we would need some indication that // we have switched to another task). // + // Note that sharing the same scheduler between multiple top-level contexts + // can currently be problematic due to operation-specific scheduler tuning. + // // The loaded_modules state (module.hxx) is shared among all the contexts // (there is no way to have multiple shared library loading "contexts") and // is protected by loaded_modules_lock. A nested context should normally diff --git a/libbuild2/install/operation.cxx b/libbuild2/install/operation.cxx index a2ad7d0..4515f14 100644 --- a/libbuild2/install/operation.cxx +++ b/libbuild2/install/operation.cxx @@ -39,7 +39,7 @@ namespace build2 "installed", "has nothing to install", // We cannot "be installed". execution_mode::first, - 0, + 0 /* concurrency */, // Run serially. &install_pre, nullptr }; @@ -62,7 +62,7 @@ namespace build2 "uninstalled", "is not installed", execution_mode::last, - 0, + 0 /* concurrency */, // Run serially &install_pre, nullptr }; diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx index 45cbd30..b73ddb3 100644 --- a/libbuild2/module.cxx +++ b/libbuild2/module.cxx @@ -243,11 +243,25 @@ namespace build2 diag_frame::stack_guard diag_cutoff (nullptr); auto df = make_diag_frame ( - [&loc, &mod](const diag_record& dr) + [&loc, &mod] (const diag_record& dr) { dr << info (loc) << "while loading build system module " << mod; }); + // Un-tune the scheduler. + // + // Note that we can only do this if we are running serially because + // otherwise we cannot guarantee the scheduler is idle (we could + // have waiting threads from the outer context). This is fine for + // now since the only two tuning level we use are serial and full + // concurrency (turns out currently we don't really need this: we + // will always be called during load or match phases and we always + // do parallel match; but let's keep it in case things change). + // + auto sched_tune (ctx.sched.serial () + ? scheduler::tune_guard (ctx.sched, 0) + : scheduler::tune_guard ()); + // Note that for now we suppress progress since it would clash with // the progress of what we are already doing (maybe in the future we // can do save/restore but then we would need some sort of diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index 0bf87d5..a9b6107 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -271,100 +271,105 @@ namespace build2 if (ctx.current_mode == execution_mode::last) reverse (ts.begin (), ts.end ()); - // Tune the scheduler. - // - switch (ctx.current_inner_oif->concurrency) - { - case 0: ctx.sched.tune (1); break; // Run serially. - case 1: break; // Run as is. - default: assert (false); // Not yet supported. - } - phase_lock pl (ctx, run_phase::execute); // Never switched. - // Set the dry-run flag. - // - ctx.dry_run = ctx.dry_run_option; - - // Setup progress reporting if requested. - // - string what; // Note: must outlive monitor_guard. - scheduler::monitor_guard mg; - - if (prog && show_progress (1 /* max_verb */)) { - size_t init (ctx.target_count.load (memory_order_relaxed)); - size_t incr (init > 100 ? init / 100 : 1); // 1%. + // Tune the scheduler. + // + using tune_guard = scheduler::tune_guard; + tune_guard sched_tune; - if (init != incr) + switch (ctx.current_inner_oif->concurrency) { - what = "% of targets " + diag_did (ctx, a); + case 0: sched_tune = tune_guard (ctx.sched, 1); break; // Run serially. + case 1: break; // Run as is. + default: assert (false); // Not supported. + } - mg = ctx.sched.monitor ( - ctx.target_count, - init - incr, - [init, incr, &what, &ctx] (size_t c) -> size_t - { - size_t p ((init - c) * 100 / init); - size_t s (ctx.skip_count.load (memory_order_relaxed)); + // Set the dry-run flag. + // + ctx.dry_run = ctx.dry_run_option; - diag_progress_lock pl; - diag_progress = ' '; - diag_progress += to_string (p); - diag_progress += what; + // Setup progress reporting if requested. + // + string what; // Note: must outlive monitor_guard. + scheduler::monitor_guard mg; - if (s != 0) - { - diag_progress += " ("; - diag_progress += to_string (s); - diag_progress += " skipped)"; - } + if (prog && show_progress (1 /* max_verb */)) + { + size_t init (ctx.target_count.load (memory_order_relaxed)); + size_t incr (init > 100 ? init / 100 : 1); // 1%. - return c - incr; - }); - } - } + if (init != incr) + { + what = "% of targets " + diag_did (ctx, a); - // Similar logic to execute_members(): first start asynchronous execution - // of all the top-level targets. - // - { - atomic_count task_count (0); - wait_guard wg (ctx, task_count); + mg = ctx.sched.monitor ( + ctx.target_count, + init - incr, + [init, incr, &what, &ctx] (size_t c) -> size_t + { + size_t p ((init - c) * 100 / init); + size_t s (ctx.skip_count.load (memory_order_relaxed)); + + diag_progress_lock pl; + diag_progress = ' '; + diag_progress += to_string (p); + diag_progress += what; + + if (s != 0) + { + diag_progress += " ("; + diag_progress += to_string (s); + diag_progress += " skipped)"; + } + + return c - incr; + }); + } + } - for (const action_target& at: ts) + // Similar logic to execute_members(): first start asynchronous + // execution of all the top-level targets. + // { - const target& t (at.as_target ()); + atomic_count task_count (0); + wait_guard wg (ctx, task_count); - l5 ([&]{trace << diag_doing (a, t);}); + for (const action_target& at: ts) + { + const target& t (at.as_target ()); - target_state s (execute_async (a, t, 0, task_count, false)); + l5 ([&]{trace << diag_doing (a, t);}); - // Bail out if the target has failed and we weren't instructed to keep - // going. - // - if (s == target_state::failed && !ctx.keep_going) - break; - } + target_state s (execute_async (a, t, 0, task_count, false)); - wg.wait (); - } + // Bail out if the target has failed and we weren't instructed to + // keep going. + // + if (s == target_state::failed && !ctx.keep_going) + break; + } - // We are now running serially. - // + wg.wait (); + } - ctx.sched.tune (0); // Restore original scheduler settings. + // We are now running serially. + // - // Clear the dry-run flag. - // - ctx.dry_run = false; + // Clear the dry-run flag. + // + ctx.dry_run = false; - // Clear the progress if present. - // - if (mg) - { - diag_progress_lock pl; - diag_progress.clear (); + // Clear the progress if present. + // + if (mg) + { + diag_progress_lock pl; + diag_progress.clear (); + } + + // Restore original scheduler settings. } // Print skip count if not zero. Note that we print it regardless of the @@ -582,7 +587,7 @@ namespace build2 "", "", execution_mode::first, - 1, + 1 /* concurrency */, nullptr, nullptr }; @@ -606,7 +611,7 @@ namespace build2 "updated", "is up to date", execution_mode::first, - 1, + 1 /* concurrency */, nullptr, nullptr }; @@ -620,7 +625,7 @@ namespace build2 "cleaned", "is clean", execution_mode::last, - 1, + 1 /* concurrency */, nullptr, nullptr }; diff --git a/libbuild2/operation.hxx b/libbuild2/operation.hxx index 520b37b..125c692 100644 --- a/libbuild2/operation.hxx +++ b/libbuild2/operation.hxx @@ -221,8 +221,11 @@ namespace build2 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). + // This is the operation's concurrency multiplier. 0 means run serially, 1 + // means run at hardware concurrency (or the concurrency specified by the + // user). + // + // Note: 0 and 1 are currently the only valid values. // const size_t concurrency; diff --git a/libbuild2/scheduler.cxx b/libbuild2/scheduler.cxx index 3c06176..bd2694c 100644 --- a/libbuild2/scheduler.cxx +++ b/libbuild2/scheduler.cxx @@ -125,7 +125,7 @@ namespace build2 else if (active_ == 0 && external_ == 0) { // Note that we tried to handle this directly in this thread but that - // wouldn't work for the phase lock case where we cal; deactivate and + // wouldn't work for the phase lock case where we call deactivate and // then go wait on a condition variable: we would be doing deadlock // detection while holding the lock that prevents other threads from // making progress! So it has to be a separate monitoring thread. @@ -391,24 +391,35 @@ namespace build2 dead_thread_ = thread (deadlock_monitor, this); } - void scheduler:: + size_t scheduler:: tune (size_t max_active) { // Note that if we tune a parallel scheduler to run serially, we will // still have the deadlock monitoring thread running. + // With multiple initial active threads we will need to make changes to + // max_active_ visible to other threads and which we currently say can be + // accessed between startup and shutdown without a lock. + // + assert (init_active_ == 1); + if (max_active == 0) max_active = orig_max_active_; - assert (max_active >= init_active_ && - max_active <= orig_max_active_); + if (max_active != max_active_) + { + assert (max_active >= init_active_ && + max_active <= orig_max_active_); + + // The scheduler must not be active though some threads might still be + // comming off from finishing a task. So we busy-wait for them. + // + lock l (wait_idle ()); - // The scheduler must not be active though some threads might still be - // comming off from finishing a task. So we busy-wait for them. - // - lock l (wait_idle ()); + swap (max_active_, max_active); + } - max_active_ = max_active; + return max_active == orig_max_active_ ? 0 : max_active; } auto scheduler:: diff --git a/libbuild2/scheduler.hxx b/libbuild2/scheduler.hxx index 0107753..4495cb9 100644 --- a/libbuild2/scheduler.hxx +++ b/libbuild2/scheduler.hxx @@ -209,17 +209,46 @@ namespace build2 // Tune a started up scheduler. // - // Currently one cannot increase the number of max_active. Pass 0 to - // restore the initial value. + // Currently one cannot increase the number of (initial) max_active, only + // decrease it. Pass 0 to restore the initial value. Returns the old + // value (0 if it is initial). // // Note that tuning can only be done while the scheduler is inactive, that - // is, no threads are executing a task or are suspended. For example, in a + // is, no threads are executing or waiting on a task. For example, in a // setup with a single initial active thread that would be after a return - // from the top-level wait() call. + // from the top-level wait() call. Tuning the scheduler with more than one + // initial active threads is currently not supported. // - void + size_t tune (size_t max_active); + struct tune_guard + { + tune_guard (): s_ (nullptr) {} + tune_guard (scheduler& s, size_t ma): s_ (&s), o_ (s_->tune (ma)) {} + tune_guard (tune_guard&& x): s_ (x.s_), o_ (x.o_) {x.s_ = nullptr;} + tune_guard& operator= (tune_guard&& x) + { + if (&x != this) + { + s_ = x.s_; + o_ = x.o_; + x.s_ = nullptr; + } + return *this; + } + + ~tune_guard () + { + if (s_ != nullptr) + s_->tune (o_); + } + + private: + scheduler* s_; + size_t o_; + }; + // Return true if the scheduler is configured to run tasks serially. // // Note: can only be called from threads that have observed startup. @@ -415,8 +444,10 @@ namespace build2 // active <= max_active // (init_active + helpers) <= max_threads (soft; see activate_helper()) // - // Note that the first three are immutable between startup() and - // shutdown() so can be accessed without a lock (but see join()). + // Note that the first three are immutable between the startup() and + // shutdown() calls so can be accessed without a lock (but see join() and + // except for max_active_ which can be changed by tune() but only when the + // scheduler is idle). // size_t init_active_ = 0; // Initially active threads. size_t max_active_ = 0; // Maximum number of active threads. @@ -430,7 +461,7 @@ namespace build2 // size_t active_ = 0; // Active master threads executing a task. size_t idle_ = 0; // Idle helper threads waiting for a task. - size_t waiting_ = 0; // Suspended master threads waiting for their tasks. + size_t waiting_ = 0; // Suspended master threads waiting on their tasks. size_t ready_ = 0; // Ready master thread waiting to become active. size_t starting_ = 0; // Helper threads starting up. diff --git a/libbuild2/test/operation.cxx b/libbuild2/test/operation.cxx index 0ca8da0..061b2cc 100644 --- a/libbuild2/test/operation.cxx +++ b/libbuild2/test/operation.cxx @@ -33,7 +33,7 @@ namespace build2 "tested", "has nothing to test", // We cannot "be tested". execution_mode::first, - 1, + 1 /* concurrency */, &test_pre, nullptr }; |