From f03435f1a99304ef81d87c9ed9b2dd017623aa31 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Wed, 20 Sep 2023 22:22:32 +0200 Subject: [PATCH] Get rid of mutexes on all critical code paths. --- src/common/AudioQueue.cxx | 9 ++++----- src/common/AudioQueue.hxx | 10 ++-------- src/common/Lock.hxx | 26 ++++++++++++++++++++++++++ src/common/Logger.cxx | 3 ++- src/common/Logger.hxx | 4 ++-- src/common/StaggeredLogger.cxx | 10 ++++++++++ src/common/StaggeredLogger.hxx | 7 +++++-- src/emucore/Event.hxx | 9 +++++---- src/emucore/EventHandler.cxx | 3 +++ 9 files changed, 59 insertions(+), 22 deletions(-) create mode 100644 src/common/Lock.hxx diff --git a/src/common/AudioQueue.cxx b/src/common/AudioQueue.cxx index 436f44832..3ffe10937 100644 --- a/src/common/AudioQueue.cxx +++ b/src/common/AudioQueue.cxx @@ -17,7 +17,6 @@ #include "AudioQueue.hxx" -using std::mutex; using std::lock_guard; // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -54,7 +53,7 @@ uInt32 AudioQueue::capacity() const // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - uInt32 AudioQueue::size() const { - const lock_guard guard(myLock); + const lock_guard guard(myLock); return mySize; } @@ -74,7 +73,7 @@ uInt32 AudioQueue::fragmentSize() const // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Int16* AudioQueue::enqueue(Int16* fragment) { - const lock_guard guard(myLock); + const lock_guard guard(myLock); Int16* newFragment = nullptr; @@ -105,7 +104,7 @@ Int16* AudioQueue::enqueue(Int16* fragment) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Int16* AudioQueue::dequeue(Int16* fragment) { - const lock_guard guard(myLock); + const lock_guard guard(myLock); if (mySize == 0) return nullptr; @@ -128,7 +127,7 @@ Int16* AudioQueue::dequeue(Int16* fragment) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void AudioQueue::closeSink(Int16* fragment) { - const lock_guard guard(myLock); + const lock_guard guard(myLock); if (myFirstFragmentForDequeue && fragment) throw runtime_error("attempt to return unknown buffer on closeSink"); diff --git a/src/common/AudioQueue.hxx b/src/common/AudioQueue.hxx index cabc61f96..981b6961e 100644 --- a/src/common/AudioQueue.hxx +++ b/src/common/AudioQueue.hxx @@ -18,9 +18,8 @@ #ifndef AUDIO_QUEUE_HXX #define AUDIO_QUEUE_HXX -#include - #include "bspf.hxx" +#include "Lock.hxx" #include "StaggeredLogger.hxx" #ifdef RTSTELLA @@ -40,11 +39,6 @@ */ class AudioQueue { - #ifdef RTSTELLA - using Lock = Spinlock; - #else - using Lock = std::Mutex; - #endif public: /** @@ -129,7 +123,7 @@ class AudioQueue uInt32 myNextFragment{0}; // We need a mutex for thread safety. - mutable Lock myLock; + mutable MutexOrSpinlock myLock; // The first (empty) enqueue call returns this fragment. diff --git a/src/common/Lock.hxx b/src/common/Lock.hxx new file mode 100644 index 000000000..6e8605ed8 --- /dev/null +++ b/src/common/Lock.hxx @@ -0,0 +1,26 @@ +//============================================================================ +// +// SSSS tt lll lll +// SS SS tt ll ll +// SS tttttt eeee ll ll aaaa +// SSSS tt ee ee ll ll aa +// SS tt eeeeee ll ll aaaaa -- "An Atari 2600 VCS Emulator" +// SS SS tt ee ll ll aa aa +// SSSS ttt eeeee llll llll aaaaa +// +// Copyright (c) 1995-2023 by Bradford W. Mott, Stephen Anthony +// and the Stella Team +// +// See the file "License.txt" for information on usage and redistribution of +// this file, and for a DISCLAIMER OF ALL WARRANTIES. +//============================================================================ + +#ifdef RTSTELLA +#include "Spinlock.hxx" + +using MutexOrSpinlock = Spinlock; +#else +#include + +using MutexOrSpinlock = std::mutex; +#endif diff --git a/src/common/Logger.cxx b/src/common/Logger.cxx index 3290def1a..e474b17dd 100644 --- a/src/common/Logger.cxx +++ b/src/common/Logger.cxx @@ -16,6 +16,7 @@ //============================================================================ #include "Logger.hxx" +#include // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Logger& Logger::instance() @@ -51,7 +52,7 @@ void Logger::debug(string_view message) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Logger::logMessage(string_view message, Level level) { - const std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); if(level == Logger::Level::ERR) { diff --git a/src/common/Logger.hxx b/src/common/Logger.hxx index c9ed1ae69..263df170d 100644 --- a/src/common/Logger.hxx +++ b/src/common/Logger.hxx @@ -19,9 +19,9 @@ #define LOGGER_HXX #include -#include #include "bspf.hxx" +#include "Lock.hxx" class Logger { @@ -63,7 +63,7 @@ class Logger { // The list of log messages string myLogMessages; - std::mutex mutex; + MutexOrSpinlock mutex; private: void logMessage(string_view message, Level level); diff --git a/src/common/StaggeredLogger.cxx b/src/common/StaggeredLogger.cxx index 28159f640..a5d08267f 100644 --- a/src/common/StaggeredLogger.cxx +++ b/src/common/StaggeredLogger.cxx @@ -45,6 +45,7 @@ StaggeredLogger::StaggeredLogger(string_view message, Logger::Level level) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - StaggeredLogger::~StaggeredLogger() { +#ifndef RTSTELLA myTimer->clear(myTimerId); // make sure that the worker thread joins before continuing with the destruction @@ -52,16 +53,23 @@ StaggeredLogger::~StaggeredLogger() // the worker thread has joined and there will be no more reentrant calls -> // continue with destruction +#endif } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void StaggeredLogger::log() { +#ifdef RTSTELLA + Logger::log(myMessage, myLevel); +#else const std::lock_guard lock(myMutex); _log(); +#endif } +#ifndef RTSTELLA + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void StaggeredLogger::_log() { @@ -143,3 +151,5 @@ void StaggeredLogger::onTimerExpired(uInt32 timerCallbackId) myLastIntervalEndTimestamp = high_resolution_clock::now(); } + +#endif diff --git a/src/common/StaggeredLogger.hxx b/src/common/StaggeredLogger.hxx index 590a87c7e..5c208df39 100644 --- a/src/common/StaggeredLogger.hxx +++ b/src/common/StaggeredLogger.hxx @@ -44,7 +44,7 @@ class StaggeredLogger void log(); private: - +#ifndef RTSTELLA void _log(); void onTimerExpired(uInt32 timerCallbackId); @@ -56,10 +56,13 @@ class StaggeredLogger void decreaseInterval(); void logLine(); +#endif string myMessage; Logger::Level myLevel; +#ifndef RTSTELLA + uInt32 myCurrentEventCount{0}; bool myIsCurrentlyCollecting{false}; @@ -83,7 +86,7 @@ class StaggeredLogger // returns. This id is unique per timer and is used to return from the callback // early in case the time is stale. uInt32 myTimerCallbackId{0}; - +#endif private: // Following constructors and assignment operators not supported StaggeredLogger(const StaggeredLogger&) = delete; diff --git a/src/emucore/Event.hxx b/src/emucore/Event.hxx index a5411cdcd..cd7f3c9cd 100644 --- a/src/emucore/Event.hxx +++ b/src/emucore/Event.hxx @@ -22,6 +22,7 @@ #include #include "bspf.hxx" +#include "Lock.hxx" /** @author Stephen Anthony, Christian Speckner, Thomas Jentzsch @@ -206,7 +207,7 @@ class Event Get the value associated with the event of the specified type. */ Int32 get(Type type) const { - std::lock_guard lock(myMutex); + std::lock_guard lock(myMutex); return myValues[type]; } @@ -215,7 +216,7 @@ class Event Set the value associated with the event of the specified type. */ void set(Type type, Int32 value) { - std::lock_guard lock(myMutex); + std::lock_guard lock(myMutex); myValues[type] = value; } @@ -225,7 +226,7 @@ class Event */ void clear() { - std::lock_guard lock(myMutex); + std::lock_guard lock(myMutex); myValues.fill(Event::NoType); } @@ -253,7 +254,7 @@ class Event // Array of values associated with each event type std::array myValues; - mutable std::mutex myMutex; + mutable MutexOrSpinlock myMutex; private: // Following constructors and assignment operators not supported diff --git a/src/emucore/EventHandler.cxx b/src/emucore/EventHandler.cxx index 1ab4ca0e5..861d7690b 100644 --- a/src/emucore/EventHandler.cxx +++ b/src/emucore/EventHandler.cxx @@ -151,11 +151,14 @@ void EventHandler::reset(EventHandlerState state) #endif myFryingFlag = false; +#ifndef RTSTELLA // Reset events almost immediately after starting emulation mode // We wait a little while (0.5s), since 'hold' events may be present, // and we want time for the ROM to process them if(state == EventHandlerState::EMULATION) myOSystem.timer().setTimeout([&ev = myEvent]() { ev.clear(); }, 500); +#endif + // Toggle 7800 mode set7800Mode(); }