From 8edc597189360a73f5026b7ca95734345333c3b5 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Thu, 7 Jun 2018 23:38:14 +0200 Subject: [PATCH] Hook and fix up EmulationWorker -> threading works, pick'n'pile is happy. --- .vscode/settings.json | 3 +- src/common/AudioQueue.cxx | 2 +- src/emucore/EmulationWorker.cxx | 166 +++++++++++++++++++------------- src/emucore/EmulationWorker.hxx | 35 +++++-- src/emucore/OSystem.cxx | 39 ++++---- src/emucore/OSystem.hxx | 3 +- 6 files changed, 155 insertions(+), 93 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index a067b748d..effb7653b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -52,6 +52,7 @@ "new": "cpp", "typeinfo": "cpp", "__mutex_base": "cpp", - "mutex": "cpp" + "mutex": "cpp", + "condition_variable": "cpp" } } diff --git a/src/common/AudioQueue.cxx b/src/common/AudioQueue.cxx index 845c63032..237059816 100644 --- a/src/common/AudioQueue.cxx +++ b/src/common/AudioQueue.cxx @@ -101,7 +101,7 @@ Int16* AudioQueue::enqueue(Int16* fragment) if (mySize < capacity) mySize++; else { myNextFragment = (myNextFragment + 1) % capacity; - (cerr << "audio buffer overflow\n").flush(); + //(cerr << "audio buffer overflow\n").flush(); } return newFragment; diff --git a/src/emucore/EmulationWorker.cxx b/src/emucore/EmulationWorker.cxx index 9304e11b4..728ddd5c8 100644 --- a/src/emucore/EmulationWorker.cxx +++ b/src/emucore/EmulationWorker.cxx @@ -24,7 +24,7 @@ using namespace std::chrono; // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -EmulationWorker::EmulationWorker(TIA& tia) : myTia(tia) +EmulationWorker::EmulationWorker() : myPendingSignal(Signal::none), myState(State::initializing) { std::mutex mutex; std::unique_lock lock(mutex); @@ -34,7 +34,7 @@ EmulationWorker::EmulationWorker(TIA& tia) : myTia(tia) &EmulationWorker::threadMain, this, &threadInitialized, &mutex ); - // Wait until the thread has acquired myMutex and moved on + // Wait until the thread has acquired myWakeupMutex and moved on while (myState == State::initializing) threadInitialized.wait(lock); } @@ -43,11 +43,11 @@ EmulationWorker::~EmulationWorker() { // This has to run in a block in order to release the mutex before joining { - std::unique_lock lock(myMutex); + std::unique_lock lock(myWakeupMutex); if (myState != State::exception) { - myPendingSignal = Signal::quit; - mySignalCondition.notify_one(); + signalQuit(); + myWakeupCondition.notify_one(); } } @@ -69,75 +69,73 @@ void EmulationWorker::handlePossibleException() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void EmulationWorker::start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult) +void EmulationWorker::start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult, TIA* tia) { - // Optimization: run this in a block to unlock the mutex before notifying the thread; - // otherwise, the thread would immediatelly block again until we have released the mutex, - // sacrificing a timeslice - { - // Aquire the mutex -> wait until the thread is suspended - std::unique_lock lock(myMutex); + waitForSignalClear(); - // Pass on possible exceptions - handlePossibleException(); + // Aquire the mutex -> wait until the thread is suspended + std::unique_lock lock(myWakeupMutex); - // NB: The thread does not suspend execution in State::initialized - if (myState != State::waitingForResume) - throw runtime_error("start called on running or dead worker"); + // Pass on possible exceptions + handlePossibleException(); - // Store the parameters for emulation - myCyclesPerSecond = cyclesPerSecond; - myMaxCycles = maxCycles; - myMinCycles = minCycles; - myDispatchResult = dispatchResult; + // NB: The thread does not suspend execution in State::initialized + if (myState != State::waitingForResume) + fatal("start called on running or dead worker"); - // Set the signal... - myPendingSignal = Signal::resume; - } + // Store the parameters for emulation + myTia = tia; + myCyclesPerSecond = cyclesPerSecond; + myMaxCycles = maxCycles; + myMinCycles = minCycles; + myDispatchResult = dispatchResult; + + // Set the signal... + myPendingSignal = Signal::resume; // ... and wakeup the thread - mySignalCondition.notify_one(); + myWakeupCondition.notify_one(); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void EmulationWorker::stop() +uInt64 EmulationWorker::stop() { - // See EmulationWorker::start for the gory details - { - std::unique_lock lock(myMutex); - handlePossibleException(); + waitForSignalClear(); - // If the worker has stopped on its own, we return - if (myState == State::waitingForResume) return; + std::unique_lock lock(myWakeupMutex); + handlePossibleException(); - // NB: The thread does not suspend execution in State::initialized or State::running - if (myState != State::waitingForStop) - throw runtime_error("stop called on a dead worker"); + // If the worker has stopped on its own, we return + if (myState == State::waitingForResume) return 0; - myPendingSignal = Signal::stop; - } + // NB: The thread does not suspend execution in State::initialized or State::running + if (myState != State::waitingForStop) + fatal("stop called on a dead worker"); - mySignalCondition.notify_one(); + myPendingSignal = Signal::stop; + + myWakeupCondition.notify_one(); + + return myTotalCycles; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void EmulationWorker::threadMain(std::condition_variable* initializedCondition, std::mutex* initializationMutex) { - std::unique_lock lock(myMutex); + std::unique_lock lock(myWakeupMutex); try { - // Optimization: release the lock before notifying our parent -> saves a timeslice { // Wait until our parent releases the lock and sleeps std::lock_guard guard(*initializationMutex); // Update the state... myState = State::initialized; - } - // ... and wake up our parent to notifiy that we have initialized. From this point, the - // parent can safely assume that we are running while the mutex is locked. - initializedCondition->notify_one(); + // ... and wake up our parent to notifiy that we have initialized. From this point, the + // parent can safely assume that we are running while the mutex is locked. + initializedCondition->notify_one(); + } while (myPendingSignal != Signal::quit) handleWakeup(lock); } @@ -153,7 +151,7 @@ void EmulationWorker::handleWakeup(std::unique_lock& lock) switch (myState) { case State::initialized: myState = State::waitingForResume; - mySignalCondition.wait(lock); + myWakeupCondition.wait(lock); break; case State::waitingForResume: @@ -165,7 +163,7 @@ void EmulationWorker::handleWakeup(std::unique_lock& lock) break; default: - throw runtime_error("wakeup in invalid worker state"); + fatal("wakeup in invalid worker state"); } } @@ -174,20 +172,21 @@ void EmulationWorker::handleWakeupFromWaitingForResume(std::unique_lock& lock) uInt64 totalCycles = 0; do { - myTia.update(*myDispatchResult, totalCycles > 0 ? myMinCycles - totalCycles : myMaxCycles); + myTia->update(*myDispatchResult, totalCycles > 0 ? myMinCycles - totalCycles : myMaxCycles); + totalCycles += myDispatchResult->getCycles(); } while (totalCycles < myMinCycles && myDispatchResult->getStatus() == DispatchResult::Status::ok); + myTotalCycles += totalCycles; + + bool continueEmulating = false; + if (myDispatchResult->getStatus() == DispatchResult::Status::ok) { // If emulation finished successfully, we can go for another round duration timesliceSeconds(static_cast(totalCycles) / static_cast(myCyclesPerSecond)); myVirtualTime += duration_cast(timesliceSeconds); myState = State::waitingForStop; + continueEmulating = myVirtualTime > high_resolution_clock::now(); + } - if (myVirtualTime > high_resolution_clock::now()) - // If we can keep up with the emulation, we sleep - mySignalCondition.wait_until(lock, myVirtualTime); - else { - // If we are already lagging behind, we briefly relinquish control over the mutex - // and yield to scheduler, to make sure that the main thread has a chance to stop us - lock.release(); - std::this_thread::yield(); - lock.lock(); - } + if (continueEmulating) { + myState = State::waitingForStop; + myWakeupCondition.wait_until(lock, myVirtualTime); } else { - // If execution trapped, we stop immediatelly. myState = State::waitingForResume; - mySignalCondition.wait(lock); + myWakeupCondition.wait(lock); } } + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void EmulationWorker::clearSignal() +{ + std::unique_lock lock(mySignalChangeMutex); + myPendingSignal = Signal::none; + + mySignalChangeCondition.notify_one(); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void EmulationWorker::signalQuit() +{ + std::unique_lock lock(mySignalChangeMutex); + myPendingSignal = Signal::quit; + + mySignalChangeCondition.notify_one(); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void EmulationWorker::waitForSignalClear() +{ + std::unique_lock lock(mySignalChangeMutex); + + while (myPendingSignal != Signal::none && myPendingSignal != Signal::quit) + mySignalChangeCondition.wait(lock); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void EmulationWorker::fatal(string message) +{ + (cerr << "FATAL in emulation worker: " << message << std::endl).flush(); + throw runtime_error(message); +} diff --git a/src/emucore/EmulationWorker.hxx b/src/emucore/EmulationWorker.hxx index e2226d6e9..c1fbacfa8 100644 --- a/src/emucore/EmulationWorker.hxx +++ b/src/emucore/EmulationWorker.hxx @@ -43,13 +43,13 @@ class EmulationWorker public: - EmulationWorker(TIA& tia); + EmulationWorker(); ~EmulationWorker(); - void start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult); + void start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult, TIA* tia); - void stop(); + uInt64 stop(); private: @@ -58,6 +58,12 @@ class EmulationWorker // Passing references into a thread is awkward and requires std::ref -> use pointers here void threadMain(std::condition_variable* initializedCondition, std::mutex* initializationMutex); + void clearSignal(); + + void signalQuit(); + + void waitForSignalClear(); + void handleWakeup(std::unique_lock& lock); void handleWakeupFromWaitingForResume(std::unique_lock& lock); @@ -66,14 +72,20 @@ class EmulationWorker void dispatchEmulation(std::unique_lock& lock); + void fatal(string message); + private: - TIA& myTia; + TIA* myTia; std::thread myThread; - std::condition_variable mySignalCondition; - std::mutex myMutex; + std::condition_variable myWakeupCondition; + std::mutex myWakeupMutex; + + std::condition_variable mySignalChangeCondition; + std::mutex mySignalChangeMutex; + std::exception_ptr myPendingException; Signal myPendingSignal; // The initial access to myState is not synchronized -> make this atomic @@ -84,7 +96,18 @@ class EmulationWorker uInt32 myMinCycles; DispatchResult* myDispatchResult; + uInt64 myTotalCycles; std::chrono::time_point myVirtualTime; + + private: + + EmulationWorker(const EmulationWorker&) = delete; + + EmulationWorker(EmulationWorker&&) = delete; + + EmulationWorker& operator=(const EmulationWorker&) = delete; + + EmulationWorker& operator=(EmulationWorker&&) = delete; }; #endif // EMULATION_WORKER_HXX diff --git a/src/emucore/OSystem.cxx b/src/emucore/OSystem.cxx index 5fbb81b2b..65b9c09ac 100644 --- a/src/emucore/OSystem.cxx +++ b/src/emucore/OSystem.cxx @@ -54,6 +54,7 @@ #include "Version.hxx" #include "TIA.hxx" #include "DispatchResult.hxx" +#include "EmulationWorker.hxx" #include "OSystem.hxx" @@ -637,27 +638,35 @@ float OSystem::frameRate() const } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -double OSystem::dispatchEmulation(uInt32 cyclesPerSecond) +double OSystem::dispatchEmulation(EmulationWorker& emulationWorker) { if (!myConsole) return 0.; - Int64 totalCycles = 0; - const Int64 minCycles = myConsole->emulationTiming().minCyclesPerTimeslice(); - const Int64 maxCycles = myConsole->emulationTiming().maxCyclesPerTimeslice(); + TIA& tia(myConsole->tia()); + EmulationTiming& timing(myConsole->emulationTiming()); DispatchResult dispatchResult; - do { - myConsole->tia().update(dispatchResult, totalCycles > 0 ? minCycles - totalCycles : maxCycles); + bool framePending = tia.newFramePending(); + if (framePending) tia.renderToFrameBuffer(); - totalCycles += dispatchResult.getCycles(); - } while (totalCycles < minCycles && dispatchResult.getStatus() == DispatchResult::Status::ok); + emulationWorker.start( + timing.cyclesPerSecond(), + timing.maxCyclesPerTimeslice(), + timing.minCyclesPerTimeslice(), + &dispatchResult, + &tia + ); + + if (framePending) myFrameBuffer->updateInEmulationMode(); + + uInt64 totalCycles = emulationWorker.stop(); if (dispatchResult.getStatus() == DispatchResult::Status::debugger) myDebugger->start(); if (dispatchResult.getStatus() == DispatchResult::Status::ok && myEventHandler->frying()) myConsole->fry(); - return static_cast(totalCycles) / static_cast(cyclesPerSecond); + return static_cast(totalCycles) / static_cast(timing.cyclesPerSecond()); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -666,6 +675,7 @@ void OSystem::mainLoop() // Sleep-based wait: good for CPU, bad for graphical sync bool busyWait = mySettings->getString("timing") != "sleep"; time_point virtualTime = high_resolution_clock::now(); + EmulationWorker emulationWorker; for(;;) { @@ -674,14 +684,9 @@ void OSystem::mainLoop() double timesliceSeconds; - if (myEventHandler->state() == EventHandlerState::EMULATION) { - timesliceSeconds = dispatchEmulation(myConsole ? myConsole->emulationTiming().cyclesPerSecond() : 1); - - if (myConsole && myConsole->tia().newFramePending()) { - myConsole->tia().renderToFrameBuffer(); - myFrameBuffer->updateInEmulationMode(); - } - } else { + if (myEventHandler->state() == EventHandlerState::EMULATION) + timesliceSeconds = dispatchEmulation(emulationWorker); + else { timesliceSeconds = 1. / 30.; myFrameBuffer->update(); } diff --git a/src/emucore/OSystem.hxx b/src/emucore/OSystem.hxx index 0b9a3521c..85d23b9d6 100644 --- a/src/emucore/OSystem.hxx +++ b/src/emucore/OSystem.hxx @@ -38,6 +38,7 @@ class Settings; class Sound; class StateManager; class VideoDialog; +class EmulationWorker; #include "FSNode.hxx" #include "FrameBufferConstants.hxx" @@ -563,7 +564,7 @@ class OSystem void validatePath(string& path, const string& setting, const string& defaultpath); - double dispatchEmulation(uInt32 cyclesPerSecond); + double dispatchEmulation(EmulationWorker& emulationWorker); // Following constructors and assignment operators not supported OSystem(const OSystem&) = delete;