Common: Update BlockingLoop to only use one atomic.

This merges two atomic<bool> into one atomic<int>.
We did move the bit from one bool to another, now we can use operator--.

2 files changed, 88 insertions, 38 deletions

diff --git a/Source/Core/Common/BlockingLoop.h b/Source/Core/Common/BlockingLoop.h
index d67e07d90e..8071de1a15 100644
--- a/Source/Core/Common/BlockingLoop.h
+++ b/Source/Core/Common/BlockingLoop.h
@@ -17,6 +17,7 @@ namespace Common
 // This class provides a synchronized loop.
 // It's a thread-safe way to trigger a new iteration without busy loops.
 // It's optimized for high-usage iterations which usually are already running while it's triggered often.
+// Be careful on using Wait() and Wakeup() at the same time. Wait() may block forever while Wakeup() is called regulary.
 class BlockingLoop
 {
 public:
@@ -34,12 +35,17 @@ public:
 	// This function will never block and is designed to finish as fast as possible.
 	void Wakeup()
 	{
-		// already running, so no need for a wakeup
-		if (m_is_running.IsSet())
+		// Already running, so no need for a wakeup.
+		// This is the common case, so try to get this as fast as possible.
+		if (m_running_state.load() >= STATE_NEED_EXECUTION)
 			return;
 
-		m_is_running.Set();
-		m_is_pending.Set();
+		// Mark that new data is available. If the old state will rerun the payload
+		// itself, we don't have to set the event to interrupt the worker.
+		if (m_running_state.exchange(STATE_NEED_EXECUTION) != STATE_SLEEPING)
+			return;
+
+		// Else as the worker thread may sleep now, we have to set the event.
 		m_new_work_event.Set();
 	}
 
@@ -47,10 +53,8 @@ public:
 	// If stopped, this returns immediately.
 	void Wait()
 	{
-		// We have to give the loop a chance to exit.
-		m_may_sleep.Set();
-
-		if (m_stopped.IsSet() || (!m_is_running.IsSet() && !m_is_pending.IsSet()))
+		// already done
+		if (m_stopped.IsSet() || m_running_state.load() <= STATE_DONE)
 			return;
 
 		// notifying this event will only wake up one thread, so use a mutex here to
@@ -58,15 +62,19 @@ public:
 		// but for the first thread for free
 		std::lock_guard<std::mutex> lk(m_wait_lock);
 
-		while (!m_stopped.IsSet() && (m_is_running.IsSet() || m_is_pending.IsSet()))
+		// Wait for the worker thread to finish.
+		while (!m_stopped.IsSet() && m_running_state.load() > STATE_DONE)
 		{
-			m_may_sleep.Set();
 			m_done_event.Wait();
 		}
+
+		// As we wanted to wait for the other thread, there is likely no work remaining.
+		// So there is no need for a busy loop any more.
+		m_may_sleep.Set();
 	}
 
 	// Half start the worker.
-	// So this object is in running state and Wait() will block until the worker calls Run().
+	// So this object is in a running state and Wait() will block until the worker calls Run().
 	// This may be called from any thread and is supposed to call at least once before Wait() is used.
 	void Prepare()
 	{
@@ -76,47 +84,80 @@ public:
 
 		if (!m_stopped.TestAndClear())
 			return;
-		m_is_pending.Set();
+		m_running_state.store(STATE_LAST_EXECUTION); // so the payload will only be executed once without any Wakeup call
 		m_shutdown.Clear();
 		m_may_sleep.Set();
 	}
 
 	// Mainloop of this object.
 	// The payload callback is called at least as often as it's needed to match the Wakeup() requirements.
-	template<class F> void Run(F payload)
+	// The optional timeout parameters is a timeout how periodicly the payload should be called.
+	// Use timeout = 0 to run without a timeout at all.
+	template<class F> void Run(F payload, int64_t timeout = 0)
 	{
+		// Asserts that Prepare is called at least once before we enter the loop.
+		// But a good implementation should call this before already.
 		Prepare();
 
 		while (!m_shutdown.IsSet())
 		{
 			payload();
 
-			m_is_pending.Clear();
-			m_done_event.Set();
-
-			if (m_is_running.IsSet())
+			switch (m_running_state.load())
 			{
-				if (m_may_sleep.IsSet())
-				{
-					m_is_pending.Set();
-					m_is_running.Clear();
-
-					// We'll sleep after the next iteration now,
-					// so clear this flag now and we won't sleep another times.
-					m_may_sleep.Clear();
-				}
+				case STATE_NEED_EXECUTION:
+					// We won't get notified while we are in the STATE_NEED_EXECUTION state, so maybe Wakeup was called.
+					// So we have to assume on finishing the STATE_NEED_EXECUTION state, that there may be some remaining tasks.
+					// To process this tasks, we call the payload again within the STATE_LAST_EXECUTION state.
+					m_running_state--;
+					break;
+
+				case STATE_LAST_EXECUTION:
+					// If we're still in the STATE_LAST_EXECUTION state, than Wakeup wasn't called within the last
+					// execution of payload. This means we should be ready now.
+					// But bad luck, Wakeup might have be called right now. So break and rerun the payload
+					// if the state was touched right now.
+					if (m_running_state-- != STATE_LAST_EXECUTION)
+						break;
+
+					// Else we're likely in the STATE_DONE state now, so wakeup the waiting threads right now.
+					// However, if we're not in the STATE_DONE state any more, the event should also be
+					// triggered so that we'll skip the next waiting call quite fast.
+					m_done_event.Set();
+
+				case STATE_DONE:
+					// We're done now. So time to check if we want to sleep or if we want to stay in a busy loop.
+					if (m_may_sleep.TestAndClear())
+					{
+						// Try to set the sleeping state.
+						if (m_running_state-- != STATE_DONE)
+							break;
+					}
+					else
+					{
+						// Busy loop.
+						break;
+					}
+
+				case STATE_SLEEPING:
+					// Just relax
+					if (timeout > 0)
+					{
+						m_new_work_event.WaitFor(std::chrono::milliseconds(timeout));
+					}
+					else
+					{
+						m_new_work_event.Wait();
+					}
+					break;
 			}
-			else
-			{
-				m_new_work_event.WaitFor(std::chrono::milliseconds(100));
-			}
-
 		}
 
-		m_is_running.Clear();
-		m_is_pending.Clear();
+		// Shutdown down, so get a safe state
+		m_running_state.store(STATE_DONE);
 		m_stopped.Set();
 
+		// Wake up the last Wait calls.
 		m_done_event.Set();
 	}
 
@@ -129,6 +170,8 @@ public:
 			return;
 
 		m_shutdown.Set();
+
+		// We have to interrupt the sleeping call to let the worker shut down soon.
 		Wakeup();
 
 		if (block)
@@ -140,6 +183,8 @@ public:
 		return !m_stopped.IsSet() && !m_shutdown.IsSet();
 	}
 
+	// This functions should be triggered by regulary by time. So we will fall back from
+	// the busy loop to the sleeping way.
 	void AllowSleep()
 	{
 		m_may_sleep.Set();
@@ -149,14 +194,19 @@ private:
 	std::mutex m_wait_lock;
 	std::mutex m_prepare_lock;
 
-	Flag m_stopped; // This one is set, Wait() shall not block.
+	Flag m_stopped;  // This one is set, Wait() shall not block.
 	Flag m_shutdown; // If this one is set, the loop shall be quit.
 
 	Event m_new_work_event;
-	Flag m_is_running; // If this one is set, the loop will be called at least once again.
-
 	Event m_done_event;
-	Flag m_is_pending; // If this one is set, there might still be work to do.
+
+	enum RUNNING_TYPE {
+		STATE_SLEEPING = 0,
+		STATE_DONE = 1,
+		STATE_LAST_EXECUTION = 2,
+		STATE_NEED_EXECUTION = 3
+	};
+	std::atomic<int> m_running_state; // must be of type RUNNING_TYPE
 
 	Flag m_may_sleep; // If this one is set, we fall back from the busy loop to an event based synchronization.
 };
diff --git a/Source/Core/VideoCommon/Fifo.cpp b/Source/Core/VideoCommon/Fifo.cpp
index 5e482f3670..048916f28c 100644
--- a/Source/Core/VideoCommon/Fifo.cpp
+++ b/Source/Core/VideoCommon/Fifo.cpp
@@ -368,7 +368,7 @@ void RunGpuLoop()
 			// don't release the GPU running state on sync GPU waits
 			fifo.isGpuReadingData = !run_loop;
 		}
-	});
+	}, 100);
 
 	AsyncRequests::GetInstance()->SetEnable(false);
 	AsyncRequests::GetInstance()->SetPassthrough(true);