aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2021-05-12 10:46:21 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2021-05-12 16:30:12 +0200
commit88379eedeae654391711d8cdda17dfc2be6367ef (patch)
tree966ee03369b51013021600a2beea8334962ab2ec
parent8b858c642ccab43050dcff2d8f98db469ac6dc1b (diff)
Keep phase locked while working own queue
-rw-r--r--libbuild2/adhoc-rule-buildscript.cxx4
-rw-r--r--libbuild2/algorithm.cxx16
-rw-r--r--libbuild2/context.cxx30
-rw-r--r--libbuild2/context.hxx8
-rw-r--r--libbuild2/context.ixx8
-rw-r--r--libbuild2/scheduler.cxx15
-rw-r--r--libbuild2/scheduler.hxx14
-rw-r--r--libbuild2/scheduler.ixx35
8 files changed, 87 insertions, 43 deletions
diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index 168856c..73219b6 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -265,9 +265,7 @@ namespace build2
const target& pt (*p.target);
- const auto& tc (pt[a].task_count);
- if (tc.load (memory_order_acquire) >= busy)
- ctx.sched.wait (exec, tc, scheduler::work_none);
+ ctx.sched.wait (exec, pt[a].task_count, scheduler::work_none);
target_state s (pt.executed_state (a));
rs |= s;
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 5e93935..a19a6a0 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -236,8 +236,8 @@ namespace build2
// to switch the phase to load. Which would result in a deadlock
// unless we release the phase.
//
- phase_unlock ul (ct.ctx);
- e = ctx.sched.wait (busy - 1, task_count, *wq);
+ phase_unlock u (ct.ctx, true /* unlock */, true /* delay */);
+ e = ctx.sched.wait (busy - 1, task_count, u, *wq);
}
// We don't lock already applied or executed targets.
@@ -2015,9 +2015,7 @@ namespace build2
// If the target is still busy, wait for its completion.
//
- const auto& tc (mt[a].task_count);
- if (tc.load (memory_order_acquire) >= busy)
- ctx.sched.wait (exec, tc, scheduler::work_none);
+ ctx.sched.wait (exec, mt[a].task_count, scheduler::work_none);
r |= mt.executed_state (a);
@@ -2067,9 +2065,7 @@ namespace build2
const target& mt (*ts[i]);
- const auto& tc (mt[a].task_count);
- if (tc.load (memory_order_acquire) >= busy)
- ctx.sched.wait (exec, tc, scheduler::work_none);
+ ctx.sched.wait (exec, mt[a].task_count, scheduler::work_none);
r |= mt.executed_state (a);
@@ -2151,9 +2147,7 @@ namespace build2
const target& pt (*p.target);
- const auto& tc (pt[a].task_count);
- if (tc.load (memory_order_acquire) >= busy)
- ctx.sched.wait (exec, tc, scheduler::work_none);
+ ctx.sched.wait (exec, pt[a].task_count, scheduler::work_none);
target_state s (pt.executed_state (a));
rs |= s;
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index b4ba5df..3ae8a07 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -853,27 +853,35 @@ namespace build2
// phase_unlock
//
phase_unlock::
- phase_unlock (context& ctx, bool u)
- : l (u ? phase_lock_instance : nullptr)
+ phase_unlock (context& c, bool u, bool d)
+ : ctx (u ? &c : nullptr), lock (nullptr)
{
- if (u)
+ if (u && !d)
+ unlock ();
+ }
+
+ void phase_unlock::
+ unlock ()
+ {
+ if (ctx != nullptr && lock == nullptr)
{
- assert (&l->ctx == &ctx);
+ lock = phase_lock_instance;
+ assert (&lock->ctx == ctx);
- phase_lock_instance = nullptr; // Note: not l->prev.
- ctx.phase_mutex.unlock (l->phase);
+ phase_lock_instance = nullptr; // Note: not lock->prev.
+ ctx->phase_mutex.unlock (lock->phase);
- //text << this_thread::get_id () << " phase unlock " << l->p;
+ //text << this_thread::get_id () << " phase unlock " << lock->phase;
}
}
phase_unlock::
~phase_unlock () noexcept (false)
{
- if (l != nullptr)
+ if (lock != nullptr)
{
- bool r (l->ctx.phase_mutex.lock (l->phase));
- phase_lock_instance = l;
+ bool r (ctx->phase_mutex.lock (lock->phase));
+ phase_lock_instance = lock;
// Fail unless we are already failing. Note that we keep the phase
// locked since there will be phase_lock down the stack to unlock it.
@@ -881,7 +889,7 @@ namespace build2
if (!r && !uncaught_exception ())
throw failed ();
- //text << this_thread::get_id () << " phase lock " << l->p;
+ //text << this_thread::get_id () << " phase lock " << lock->phase;
}
}
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 0d401de..ad89f58 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -594,10 +594,14 @@ namespace build2
//
struct LIBBUILD2_SYMEXPORT phase_unlock
{
- phase_unlock (context&, bool unlock = true);
+ phase_unlock (context&, bool unlock = true, bool delay = false);
~phase_unlock () noexcept (false);
- phase_lock* l;
+ void
+ unlock ();
+
+ context* ctx;
+ phase_lock* lock;
};
// Assuming we have a lock on the current phase, temporarily switch to a
diff --git a/libbuild2/context.ixx b/libbuild2/context.ixx
index 73601d4..4f86c28 100644
--- a/libbuild2/context.ixx
+++ b/libbuild2/context.ixx
@@ -56,12 +56,8 @@ namespace build2
inline void wait_guard::
wait ()
{
- if (task_count->load (memory_order_acquire) > start_count)
- {
- phase_unlock u (*ctx, phase);
- ctx->sched.wait (start_count, *task_count);
- }
-
+ phase_unlock u (*ctx, phase, true /* delay */);
+ ctx->sched.wait (start_count, *task_count, u);
task_count = nullptr;
}
}
diff --git a/libbuild2/scheduler.cxx b/libbuild2/scheduler.cxx
index deb5399..43da681 100644
--- a/libbuild2/scheduler.cxx
+++ b/libbuild2/scheduler.cxx
@@ -50,16 +50,9 @@ namespace build2
scheduler_queue = q;
}
- size_t scheduler::
- wait (size_t start_count, const atomic_count& task_count, work_queue wq)
+ optional<size_t> scheduler::
+ wait_impl (size_t start_count, const atomic_count& task_count, work_queue wq)
{
- // Note that task_count is a synchronization point.
- //
- size_t tc;
-
- if ((tc = task_count.load (memory_order_acquire)) <= start_count)
- return tc;
-
assert (max_active_ != 1); // Serial execution, nobody to wait for.
// See if we can run some of our own tasks.
@@ -71,6 +64,8 @@ namespace build2
//
if (task_queue* tq = queue ())
{
+ size_t tc;
+
for (lock ql (tq->mutex); !tq->shutdown && !empty_back (*tq); )
{
pop_back (*tq, ql);
@@ -91,7 +86,7 @@ namespace build2
}
}
- return suspend (start_count, task_count);
+ return nullopt;
}
void scheduler::
diff --git a/libbuild2/scheduler.hxx b/libbuild2/scheduler.hxx
index e1bb715..9556caa 100644
--- a/libbuild2/scheduler.hxx
+++ b/libbuild2/scheduler.hxx
@@ -125,6 +125,16 @@ namespace build2
return wait (0, task_count, wq);
}
+ // As above but call lock.unlock() before suspending (can be used to
+ // unlock the phase).
+ //
+ template <typename L>
+ size_t
+ wait (size_t start_count,
+ const atomic_count& task_count,
+ L& lock,
+ work_queue = work_all);
+
// Mark the queue so that we don't work any tasks that may already be
// there. In the normal "bunch of acync() calls followed by wait()"
// cases this happens automatically but in special cases where async()
@@ -846,6 +856,10 @@ namespace build2
static void
queue (task_queue*) noexcept;
+
+ private:
+ optional<size_t>
+ wait_impl (size_t, const atomic_count&, work_queue);
};
}
diff --git a/libbuild2/scheduler.ixx b/libbuild2/scheduler.ixx
index f9f0f2e..4cf347c 100644
--- a/libbuild2/scheduler.ixx
+++ b/libbuild2/scheduler.ixx
@@ -3,6 +3,41 @@
namespace build2
{
+ inline size_t scheduler::
+ wait (size_t start_count, const atomic_count& task_count, work_queue wq)
+ {
+ // Note that task_count is a synchronization point.
+ //
+ size_t tc;
+ if ((tc = task_count.load (memory_order_acquire)) <= start_count)
+ return tc;
+
+ if (optional<size_t> r = wait_impl (start_count, task_count, wq))
+ return *r;
+
+ return suspend (start_count, task_count);
+ }
+
+ template <typename L>
+ inline size_t scheduler::
+ wait (size_t start_count,
+ const atomic_count& task_count,
+ L& lock,
+ work_queue wq)
+ {
+ // Note that task_count is a synchronization point.
+ //
+ size_t tc;
+ if ((tc = task_count.load (memory_order_acquire)) <= start_count)
+ return tc;
+
+ if (optional<size_t> r = wait_impl (start_count, task_count, wq))
+ return *r;
+
+ lock.unlock ();
+ return suspend (start_count, task_count);
+ }
+
inline scheduler::queue_mark::
queue_mark (scheduler& s)
: tq_ (s.queue ())