From 7fdb87143c7e2141b65f47d727783b533e72c717 Mon Sep 17 00:00:00 2001 From: Zygo Blaxell Date: Tue, 15 Nov 2022 16:24:25 -0500 Subject: [PATCH] task: get rid of the separate Barrier and BarrierLock Make one class Barrier which is copiable, so we don't have to have users making shared Barrier all the time. Signed-off-by: Zygo Blaxell --- include/crucible/task.h | 32 +++++++++----------------- lib/task.cc | 32 +++++++------------------- test/task.cc | 50 ++++++++++++++++++++++------------------- 3 files changed, 46 insertions(+), 68 deletions(-) diff --git a/include/crucible/task.h b/include/crucible/task.h index b767155..63feaa3 100644 --- a/include/crucible/task.h +++ b/include/crucible/task.h @@ -99,36 +99,26 @@ namespace crucible { static void cancel(); }; - // Barrier executes waiting Tasks once the last BarrierLock - // is released. Multiple unique Tasks may be scheduled while - // BarrierLocks exist and all will be run() at once upon - // release. If no BarrierLocks exist, Tasks are executed - // immediately upon insertion. - class BarrierState; - class BarrierLock { - shared_ptr m_barrier_state; - BarrierLock(shared_ptr pbs); - friend class Barrier; - public: - // Release this Lock immediately and permanently - void release(); - }; - + /// Barrier delays the execution of one or more Tasks. + /// The Tasks are executed when the last shared reference to the + /// BarrierState is released. Copies of Barrier objects refer + /// to the same Barrier state. class Barrier { shared_ptr m_barrier_state; - Barrier(shared_ptr pbs); public: Barrier(); - // Prevent execution of tasks behind barrier until - // BarrierLock destructor or release() method is called. - BarrierLock lock(); - - // Schedule a task for execution when no Locks exist + /// Schedule a task for execution when last Barrier is released. void insert_task(Task t); + + /// Release this reference to the barrier state. + /// Last released reference executes the task. + /// Barrier can only be released once, after which the + /// object can no longer be used. + void release(); }; // Exclusion provides exclusive access to a ExclusionLock. diff --git a/lib/task.cc b/lib/task.cc index 6e9f64c..0a971f1 100644 --- a/lib/task.cc +++ b/lib/task.cc @@ -788,16 +788,6 @@ namespace crucible { void insert_task(Task t); }; - Barrier::Barrier(shared_ptr pbs) : - m_barrier_state(pbs) - { - } - - Barrier::Barrier() : - m_barrier_state(make_shared()) - { - } - void BarrierState::release() { @@ -813,17 +803,6 @@ namespace crucible { release(); } - BarrierLock::BarrierLock(shared_ptr pbs) : - m_barrier_state(pbs) - { - } - - void - BarrierLock::release() - { - m_barrier_state.reset(); - } - void BarrierState::insert_task(Task t) { @@ -831,16 +810,21 @@ namespace crucible { m_tasks.insert(t); } + Barrier::Barrier() : + m_barrier_state(make_shared()) + { + } + void Barrier::insert_task(Task t) { m_barrier_state->insert_task(t); } - BarrierLock - Barrier::lock() + void + Barrier::release() { - return BarrierLock(m_barrier_state); + m_barrier_state.reset(); } class ExclusionState { diff --git a/test/task.cc b/test/task.cc index e688a49..ddc8db4 100644 --- a/test/task.cc +++ b/test/task.cc @@ -90,47 +90,51 @@ test_barrier(size_t count) mutex mtx; condition_variable cv; + bool done_flag = false; unique_lock lock(mtx); - auto b = make_shared(); + Barrier b; // Run several tasks in parallel for (size_t c = 0; c < count; ++c) { - auto bl = b->lock(); ostringstream oss; oss << "task #" << c; + auto b_hold = b; Task t( oss.str(), - [c, &task_done, &mtx, bl]() mutable { - // cerr << "Task #" << c << endl; + [c, &task_done, &mtx, b_hold]() mutable { + // ostringstream oss; + // oss << "Task #" << c << endl; unique_lock lock(mtx); + // cerr << oss.str(); task_done.at(c) = true; - bl.release(); + b_hold.release(); } ); t.run(); } + // Need completed to go out of local scope so it will release b + { + Task completed( + "Waiting for Barrier", + [&mtx, &cv, &done_flag]() { + unique_lock lock(mtx); + // cerr << "Running cv notify" << endl; + done_flag = true; + cv.notify_all(); + } + ); + b.insert_task(completed); + } + // Get current status - ostringstream oss; - TaskMaster::print_queue(oss); - TaskMaster::print_workers(oss); + // TaskMaster::print_queue(cerr); + // TaskMaster::print_workers(cerr); - bool done_flag = false; - - Task completed( - "Waiting for Barrier", - [&mtx, &cv, &done_flag]() { - unique_lock lock(mtx); - // cerr << "Running cv notify" << endl; - done_flag = true; - cv.notify_all(); - } - ); - b->insert_task(completed); - - b.reset(); + // Release our b + b.release(); while (true) { size_t tasks_done = 0; @@ -139,7 +143,7 @@ test_barrier(size_t count) ++tasks_done; } } - // cerr << "Tasks done: " << tasks_done << " done_flag " << done_flag << endl; + cerr << "Tasks done: " << tasks_done << " done_flag " << done_flag << endl; if (tasks_done == count && done_flag) { break; }