Merge pull request #14061 from jordan-woyak/config-info-spin-mutex

ConfigInfo: Cleanups and change mutex to a spin lock.

5 files changed, 86 insertions, 44 deletions

diff --git a/Source/Core/Common/CMakeLists.txt b/Source/Core/Common/CMakeLists.txt
index 69ab3baf6a..e8bd917175 100644
--- a/Source/Core/Common/CMakeLists.txt
+++ b/Source/Core/Common/CMakeLists.txt
@@ -111,6 +111,7 @@ add_library(common
   MinizipUtil.h
   MsgHandler.cpp
   MsgHandler.h
+  Mutex.h
   NandPaths.cpp
   NandPaths.h
   Network.cpp
diff --git a/Source/Core/Common/Config/Config.h b/Source/Core/Common/Config/Config.h
index ea6058a8d3..e3e3d3df3b 100644
--- a/Source/Core/Common/Config/Config.h
+++ b/Source/Core/Common/Config/Config.h
@@ -72,10 +72,10 @@ T Get(const Info<T>& info)
     cached.value = GetUncached(info);
     cached.config_version = config_version;
 
-    info.SetCachedValue(cached);
+    info.TryToSetCachedValue(cached);
   }
 
-  return cached.value;
+  return std::move(cached.value);
 }
 
 template <typename T>
diff --git a/Source/Core/Common/Config/ConfigInfo.h b/Source/Core/Common/Config/ConfigInfo.h
index e8d904fae2..32f5db069f 100644
--- a/Source/Core/Common/Config/ConfigInfo.h
+++ b/Source/Core/Common/Config/ConfigInfo.h
@@ -4,12 +4,12 @@
 #pragma once
 
 #include <mutex>
-#include <shared_mutex>
 #include <string>
 #include <utility>
 
 #include "Common/CommonTypes.h"
 #include "Common/Config/Enums.h"
+#include "Common/Mutex.h"
 #include "Common/TypeUtils.h"
 
 namespace Config
@@ -35,73 +35,58 @@ template <typename T>
 class Info
 {
 public:
-  constexpr Info(const Location& location, const T& default_value)
-      : m_location{location}, m_default_value{default_value}, m_cached_value{default_value, 0}
+  constexpr Info(Location location, T default_value)
+      : m_location{std::move(location)}, m_default_value{default_value},
+        m_cached_value{std::move(default_value), 0}
   {
   }
 
-  Info(const Info<T>& other) { *this = other; }
-
-  // Not thread-safe
-  Info(Info<T>&& other) { *this = std::move(other); }
+  Info(const Info<T>& other)
+      : m_location{other.m_location}, m_default_value{other.m_default_value},
+        m_cached_value(other.GetCachedValue())
+  {
+  }
 
   // Make it easy to convert Info<Enum> into Info<UnderlyingType<Enum>>
   // so that enum settings can still easily work with code that doesn't care about the enum values.
   template <Common::TypedEnum<T> Enum>
   Info(const Info<Enum>& other)
+      : m_location{other.GetLocation()}, m_default_value{static_cast<T>(other.GetDefaultValue())},
+        m_cached_value(other.template GetCachedValueCasted<T>())
   {
-    *this = other;
   }
 
-  Info<T>& operator=(const Info<T>& other)
-  {
-    m_location = other.GetLocation();
-    m_default_value = other.GetDefaultValue();
-    m_cached_value = other.GetCachedValue();
-    return *this;
-  }
-
-  // Not thread-safe
-  Info<T>& operator=(Info<T>&& other)
-  {
-    m_location = std::move(other.m_location);
-    m_default_value = std::move(other.m_default_value);
-    m_cached_value = std::move(other.m_cached_value);
-    return *this;
-  }
+  ~Info() = default;
 
-  // Make it easy to convert Info<Enum> into Info<UnderlyingType<Enum>>
-  // so that enum settings can still easily work with code that doesn't care about the enum values.
-  template <Common::TypedEnum<T> Enum>
-  Info<T>& operator=(const Info<Enum>& other)
-  {
-    m_location = other.GetLocation();
-    m_default_value = static_cast<T>(other.GetDefaultValue());
-    m_cached_value = other.template GetCachedValueCasted<T>();
-    return *this;
-  }
+  // Assignments after construction would require more locking to be thread safe.
+  // It seems unnecessary to have this functionality anyways.
+  Info& operator=(const Info&) = delete;
+  Info& operator=(Info&&) = delete;
+  // Moves are also unnecessary and would be thread unsafe without additional locking.
+  Info(Info&&) = delete;
 
   constexpr const Location& GetLocation() const { return m_location; }
   constexpr const T& GetDefaultValue() const { return m_default_value; }
 
   CachedValue<T> GetCachedValue() const
   {
-    std::shared_lock lock(m_cached_value_mutex);
+    std::lock_guard lk{m_cached_value_mutex};
     return m_cached_value;
   }
 
   template <typename U>
   CachedValue<U> GetCachedValueCasted() const
   {
-    std::shared_lock lock(m_cached_value_mutex);
-    return CachedValue<U>{static_cast<U>(m_cached_value.value), m_cached_value.config_version};
+    std::lock_guard lk{m_cached_value_mutex};
+    return {static_cast<U>(m_cached_value.value), m_cached_value.config_version};
   }
 
-  void SetCachedValue(const CachedValue<T>& cached_value) const
+  // Only updates if the provided config_version is newer.
+  void TryToSetCachedValue(CachedValue<T> new_value) const
   {
-    std::unique_lock lock(m_cached_value_mutex);
-    if (m_cached_value.config_version < cached_value.config_version)
-      m_cached_value = cached_value;
+    std::lock_guard lk{m_cached_value_mutex};
+    if (new_value.config_version > m_cached_value.config_version)
+      m_cached_value = std::move(new_value);
   }
 
 private:
@@ -109,6 +94,10 @@ private:
   T m_default_value;
 
   mutable CachedValue<T> m_cached_value;
-  mutable std::shared_mutex m_cached_value_mutex;
+
+  // In testing, this mutex is effectively never contested.
+  // The lock durations are brief and each `Info` object is mostly relevant to one thread.
+  // Common::SpinMutex is ~3x faster than std::shared_mutex when uncontested.
+  mutable Common::SpinMutex m_cached_value_mutex;
 };
 }  // namespace Config
diff --git a/Source/Core/Common/Mutex.h b/Source/Core/Common/Mutex.h
new file mode 100644
index 0000000000..22aebd08b1
--- /dev/null
+++ b/Source/Core/Common/Mutex.h
@@ -0,0 +1,51 @@
+// Copyright 2025 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include <atomic>
+
+namespace Common
+{
+namespace detail
+{
+template <bool UseAtomicWait>
+class AtomicMutexBase
+{
+public:
+  void lock()
+  {
+    while (m_lock.exchange(true, std::memory_order_acquire))
+    {
+      if constexpr (UseAtomicWait)
+        m_lock.wait(true, std::memory_order_relaxed);
+    }
+  }
+
+  bool try_lock()
+  {
+    bool expected = false;
+    return m_lock.compare_exchange_weak(expected, true, std::memory_order_acquire,
+                                        std::memory_order_relaxed);
+  }
+
+  // Unlike with std::mutex, this call may come from any thread.
+  void unlock()
+  {
+    m_lock.store(false, std::memory_order_release);
+    if constexpr (UseAtomicWait)
+      m_lock.notify_one();
+  }
+
+private:
+  std::atomic_bool m_lock{};
+};
+}  // namespace detail
+
+// Sometimes faster than std::mutex.
+using AtomicMutex = detail::AtomicMutexBase<true>;
+
+// Very fast to lock and unlock when uncontested (~3x faster than std::mutex).
+using SpinMutex = detail::AtomicMutexBase<false>;
+
+}  // namespace Common
diff --git a/Source/Core/DolphinLib.props b/Source/Core/DolphinLib.props
index 5049ac785f..32f76c8d01 100644
--- a/Source/Core/DolphinLib.props
+++ b/Source/Core/DolphinLib.props
@@ -145,6 +145,7 @@
     <ClInclude Include="Common\MemoryUtil.h" />
     <ClInclude Include="Common\MinizipUtil.h" />
     <ClInclude Include="Common\MsgHandler.h" />
+    <ClInclude Include="Common\Mutex.h" />
     <ClInclude Include="Common\NandPaths.h" />
     <ClInclude Include="Common\Network.h" />
     <ClInclude Include="Common\PcapFile.h" />