task: get rid of separate Exclusion and ExclusionState

Exclusion was generating a new Task every time a lock was contended.
That results in thousands of empty Task objects which contain a single
Task item.

Get rid of ExclusionState.  Exclusion is now a simple weak_ptr to a Task.
If the weak_ptr is expired, the Exclusion is unlocked.  If the weak_ptr
is not expired, it points to the Task which owns the Exclusion.

try_lock now appends the Task attempting to lock the Exclusion directly
to the owning Task, eliminating the need for Exclusion to have one.
This also removes the need to call insert_task separately, though
insert_task remains for other use cases.

With no ExclusionState there is no need for a string argument to
Exclusion's constructor, so get rid of that too.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>

include/crucible/task.h

@@ -3,6 +3,7 @@
 
 #include <functional>
 #include <memory>
+#include <mutex>
 #include <ostream>
 #include <string>
 
@@ -121,50 +122,40 @@ namespace crucible {
 		void release();
 	};
 
-	// Exclusion provides exclusive access to a ExclusionLock.
-	// One Task will be able to obtain the ExclusionLock; other Tasks
-	// may schedule themselves for re-execution after the ExclusionLock
-	// is released.
-
-	class ExclusionState;
-	class Exclusion;
-
 	class ExclusionLock {
-		shared_ptr<ExclusionState> m_exclusion_state;
-		ExclusionLock(shared_ptr<ExclusionState> pes);
-		ExclusionLock() = default;
+		shared_ptr<Task> m_owner;
+		ExclusionLock(shared_ptr<Task> owner);
 	friend class Exclusion;
 	public:
-		// Calls release()
-		~ExclusionLock();
+		/// Explicit default constructor because we have other kinds
+		ExclusionLock() = default;
 
-		// Release this Lock immediately and permanently
+		/// Release this Lock immediately and permanently
 		void release();
 
-		// Test for locked state
+		/// Test for locked state
 		operator bool() const;
 	};
 
 	class Exclusion {
-		shared_ptr<ExclusionState> m_exclusion_state;
+		mutex m_mutex;
+		weak_ptr<Task> m_owner;
 
-		Exclusion(shared_ptr<ExclusionState> pes);
 	public:
-		Exclusion(const string &title);
+		/// Attempt to obtain a Lock.  If successful, current Task
+		/// owns the Lock until the ExclusionLock is released
+		/// (it is the ExclusionLock that owns the lock, so it can
+		/// be passed to other Tasks or threads, but this is not
+		/// recommended practice).
+		/// If not successful, current Task is appended to the
+		/// task that currently holds the lock.  Current task is
+		/// expected to release any other ExclusionLock
+		/// objects it holds, and exit its Task function.
+		ExclusionLock try_lock(const Task &task);
 
-		// Attempt to obtain a Lock.  If successful, current Task
-		// owns the Lock until the ExclusionLock is released
-		// (it is the ExclusionLock that owns the lock, so it can
-		// be passed to other Tasks or threads, but this is not
-		// recommended practice).
-		// If not successful, current Task is expected to call
-		// insert_task(current_task()), release any ExclusionLock
-		// objects it holds, and exit its Task function.
-		ExclusionLock try_lock();
-
-		// Execute Task when Exclusion is unlocked (possibly
-		// immediately).
-		void insert_task(Task t = Task::current_task());
+		/// Execute Task when Exclusion is unlocked (possibly
+		/// immediately).
+		void insert_task(const Task &t);
 	};
 
 

lib/task.cc

@@ -827,110 +827,52 @@ namespace crucible {
 		m_barrier_state.reset();
 	}
 
-	class ExclusionState {
-		mutex		m_mutex;
-		bool		m_locked = false;
-		Task		m_task;
-
-	public:
-		ExclusionState(const string &title);
-		~ExclusionState();
-		void release();
-		bool try_lock();
-		void insert_task(Task t);
-	};
-
-	Exclusion::Exclusion(shared_ptr<ExclusionState> pbs) :
-		m_exclusion_state(pbs)
-	{
-	}
-
-	Exclusion::Exclusion(const string &title) :
-		m_exclusion_state(make_shared<ExclusionState>(title))
-	{
-	}
-
-	ExclusionState::ExclusionState(const string &title) :
-		m_task(title, [](){})
-	{
-	}
-
-	void
-	ExclusionState::release()
-	{
-		unique_lock<mutex> lock(m_mutex);
-		m_locked = false;
-		m_task.run();
-	}
-
-	ExclusionState::~ExclusionState()
-	{
-		release();
-	}
-
-	ExclusionLock::ExclusionLock(shared_ptr<ExclusionState> pbs) :
-		m_exclusion_state(pbs)
+	ExclusionLock::ExclusionLock(shared_ptr<Task> owner) :
+		m_owner(owner)
 	{
 	}
 
 	void
 	ExclusionLock::release()
 	{
-		if (m_exclusion_state) {
-			m_exclusion_state->release();
-			m_exclusion_state.reset();
-		}
-	}
-
-	ExclusionLock::~ExclusionLock()
-	{
-		release();
+		m_owner.reset();
 	}
 
 	void
-	ExclusionState::insert_task(Task task)
+	Exclusion::insert_task(const Task &task)
 	{
 		unique_lock<mutex> lock(m_mutex);
-		if (m_locked) {
+		const auto sp = m_owner.lock();
+		lock.unlock();
+		if (sp) {
 			// If Exclusion is locked then queue task for release;
-			m_task.append(task);
+			sp->append(task);
 		} else {
 			// otherwise, run the inserted task immediately
 			task.run();
 		}
 	}
 
-	bool
-	ExclusionState::try_lock()
+	ExclusionLock
+	Exclusion::try_lock(const Task &task)
 	{
 		unique_lock<mutex> lock(m_mutex);
-		if (m_locked) {
-			return false;
+		const auto sp = m_owner.lock();
+		if (sp) {
+			if (task) {
+				sp->append(task);
+			}
+			return ExclusionLock();
 		} else {
-			m_locked = true;
-			return true;
+			const auto rv = make_shared<Task>(task);
+			m_owner = rv;
+			return ExclusionLock(rv);
 		}
 	}
 
-	void
-	Exclusion::insert_task(Task t)
-	{
-		m_exclusion_state->insert_task(t);
-	}
-
 	ExclusionLock::operator bool() const
 	{
-		return !!m_exclusion_state;
+		return !!m_owner;
 	}
 
-	ExclusionLock
-	Exclusion::try_lock()
-	{
-		THROW_CHECK0(runtime_error, m_exclusion_state);
-		if (m_exclusion_state->try_lock()) {
-			return ExclusionLock(m_exclusion_state);
-		} else {
-			return ExclusionLock();
-		}
-	}
 }

test/task.cc

@@ -157,7 +157,7 @@ void
 test_exclusion(size_t count)
 {
 	mutex only_one;
-	auto excl = make_shared<Exclusion>("test_excl");
+	auto excl = make_shared<Exclusion>();
 
 	mutex mtx;
 	condition_variable cv;
@@ -178,9 +178,8 @@ test_exclusion(size_t count)
 			[c, &only_one, excl, &lock_success_count, &lock_failure_count, &pings, &tasks_running, &cv, &mtx]() mutable {
 				// cerr << "Task #" << c << endl;
 				(void)c;
-				auto lock = excl->try_lock();
+				auto lock = excl->try_lock(Task::current_task());
 				if (!lock) {
-					excl->insert_task(Task::current_task());
 					++lock_failure_count;
 					return;
 				}
@@ -200,7 +199,7 @@ test_exclusion(size_t count)
 		t.run();
 	}
 
-	// excl.reset();
+	excl.reset();
 
 	unique_lock<mutex> lock(mtx);
 	while (tasks_running) {