From 30c5b24ec5e77306a0737ba361aefe351eaa26b1 Mon Sep 17 00:00:00 2001
From: Andrew Belt <andrewpbelt@gmail.com>
Date: Fri, 10 May 2019 05:41:49 -0400
Subject: [PATCH] Add Engine::yieldWorker() which turns worker spinlocks into
 mutex locks. Fix race condition in EngineWorker::run() when changing number
 of threads.

---
 CHANGELOG.md                |  3 ++
 include/engine/Engine.hpp   |  4 +++
 src/Core/AudioInterface.cpp |  2 ++
 src/app/Scene.cpp           |  2 +-
 src/engine/Engine.cpp       | 70 ++++++++++++++++++++++++++++++-------
 5 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 662dba2b..981dc002 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,8 @@
 - Fix draw order of cable plugs and wires
 - Make Gamepad MIDI driver generate MIDI CC instead of MIDI notes for buttons
 - Fix Unicode user directories on Windows
+- Add ability to change cable colors in `settings.json`
+- Add `-p X` flag for dumping a screenshot of each available module
 
 - Core
 	- Add Core CV-MIDI, CV-CC, and CV-Gate for sending MIDI to external devices
@@ -35,6 +37,7 @@
 	- Add polyphony to Core MIDI-CV
 	- Add MPE mode to Core MIDI-CV
 	- Add "Panic" button to all MIDI modules to reset performance state
+	- Add Core Audio 16
 
 - API
 	- Add [`simd.hpp`](include/dsp/simd.hpp) for generically handling arithmetic and math functions for vectors of floats, accelerated with SSE
diff --git a/include/engine/Engine.hpp b/include/engine/Engine.hpp
index 0f293f58..b15bc036 100644
--- a/include/engine/Engine.hpp
+++ b/include/engine/Engine.hpp
@@ -25,6 +25,10 @@ struct Engine {
 	float getSampleRate();
 	/** Returns the inverse of the current sample rate. */
 	float getSampleTime();
+	/** Causes worker threads to block on a mutex instead of spinlock.
+	Call this in your Module::step() method to hint that the operation will take more than ~0.1 ms.
+	*/
+	void yieldWorkers();
 
 	// Modules
 	/** Adds a module to the rack engine.
diff --git a/src/Core/AudioInterface.cpp b/src/Core/AudioInterface.cpp
index dcb3f879..093ed7a3 100644
--- a/src/Core/AudioInterface.cpp
+++ b/src/Core/AudioInterface.cpp
@@ -132,6 +132,7 @@ struct AudioInterface : Module {
 		if (port.active && port.numInputs > 0) {
 			// Wait until inputs are present
 			// Give up after a timeout in case the audio device is being unresponsive.
+			APP->engine->yieldWorkers();
 			std::unique_lock<std::mutex> lock(port.engineMutex);
 			auto cond = [&] {
 				return (!port.inputBuffer.empty());
@@ -181,6 +182,7 @@ struct AudioInterface : Module {
 			if (outputBuffer.full()) {
 				// Wait until enough outputs are consumed
 				// Give up after a timeout in case the audio device is being unresponsive.
+				APP->engine->yieldWorkers();
 				std::unique_lock<std::mutex> lock(port.engineMutex);
 				auto cond = [&] {
 					return (port.outputBuffer.size() < (size_t) port.blockSize);
diff --git a/src/app/Scene.cpp b/src/app/Scene.cpp
index cee6b416..4a093253 100644
--- a/src/app/Scene.cpp
+++ b/src/app/Scene.cpp
@@ -148,7 +148,7 @@ void Scene::onHoverKey(const event::HoverKey &e) {
 			case GLFW_KEY_F11: {
 				APP->window->setFullScreen(!APP->window->isFullScreen());
 				e.consume(this);
-			}
+			} break;
 		}
 	}
 }
diff --git a/src/engine/Engine.cpp b/src/engine/Engine.cpp
index 61c4d058..6b14c05f 100644
--- a/src/engine/Engine.cpp
+++ b/src/engine/Engine.cpp
@@ -96,6 +96,50 @@ struct SpinBarrier {
 };
 
 
+/** Spinlocks until all `total` threads are waiting.
+If `yield` is set to true at any time, all threads will switch to waiting on a mutex instead.
+All threads must return before beginning a new phase. Alternating between two barriers solves this problem.
+*/
+struct HybridBarrier {
+	std::atomic<int> count {0};
+	int total = 0;
+
+	std::mutex mutex;
+	std::condition_variable cv;
+
+	std::atomic<bool> yield {false};
+
+	void wait() {
+		int id = ++count;
+
+		// End and reset phase if this is the last thread
+		if (id == total) {
+			count = 0;
+			if (yield) {
+				std::unique_lock<std::mutex> lock(mutex);
+				cv.notify_all();
+				yield = false;
+			}
+			return;
+		}
+
+		// Spinlock
+		while (!yield) {
+			if (count == 0)
+				return;
+		}
+
+		// Wait on mutex
+		{
+			std::unique_lock<std::mutex> lock(mutex);
+			cv.wait(lock, [&]{
+				return count == 0;
+			});
+		}
+	}
+};
+
+
 struct EngineWorker {
 	Engine *engine;
 	int id;
@@ -117,7 +161,6 @@ struct EngineWorker {
 	}
 
 	void run();
-	void step();
 };
 
 
@@ -146,8 +189,8 @@ struct Engine::Internal {
 	bool realTime = false;
 	int threadCount = 1;
 	std::vector<EngineWorker> workers;
-	SpinBarrier engineBarrier;
-	SpinBarrier workerBarrier;
+	HybridBarrier engineBarrier;
+	HybridBarrier workerBarrier;
 	std::atomic<int> workerModuleIndex;
 };
 
@@ -417,6 +460,10 @@ float Engine::getSampleTime() {
 	return internal->sampleTime;
 }
 
+void Engine::yieldWorkers() {
+	internal->workerBarrier.yield = true;
+}
+
 void Engine::addModule(Module *module) {
 	assert(module);
 	VIPLock vipLock(internal->vipMutex);
@@ -698,17 +745,14 @@ void EngineWorker::run() {
 	system::setThreadName("Engine worker");
 	system::setThreadRealTime(engine->internal->realTime);
 	disableDenormals();
-	while (running) {
-		step();
-	}
-}
 
-void EngineWorker::step() {
-	engine->internal->engineBarrier.wait();
-	if (!running)
-		return;
-	Engine_stepModules(engine, id);
-	engine->internal->workerBarrier.wait();
+	while (1) {
+		engine->internal->engineBarrier.wait();
+		if (!running)
+			return;
+		Engine_stepModules(engine, id);
+		engine->internal->workerBarrier.wait();
+	}
 }