Merge pull request #12117 from JosJuice/config-callback-cpu

Don't call RunAsCPUThread in config callbacks

Source/Core/Core/CMakeLists.txt

@@ -59,6 +59,8 @@ add_library(core
   Core.h
   CoreTiming.cpp
   CoreTiming.h
+  CPUThreadConfigCallback.cpp
+  CPUThreadConfigCallback.h
   Debugger/CodeTrace.cpp
   Debugger/CodeTrace.h
   Debugger/DebugInterface.h

Source/Core/Core/CPUThreadConfigCallback.cpp

@@ -0,0 +1,75 @@
+// Copyright 2023 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "Core/CPUThreadConfigCallback.h"
+
+#include <atomic>
+
+#include "Common/Assert.h"
+#include "Common/Config/Config.h"
+#include "Core/Core.h"
+
+namespace
+{
+std::atomic<bool> s_should_run_callbacks = false;
+
+static std::vector<
+    std::pair<CPUThreadConfigCallback::ConfigChangedCallbackID, Config::ConfigChangedCallback>>
+    s_callbacks;
+
+static size_t s_next_callback_id = 0;
+
+void RunCallbacks()
+{
+  for (const auto& callback : s_callbacks)
+    callback.second();
+}
+
+void OnConfigChanged()
+{
+  if (Core::IsCPUThread())
+  {
+    s_should_run_callbacks.store(false, std::memory_order_relaxed);
+    RunCallbacks();
+  }
+  else
+  {
+    s_should_run_callbacks.store(true, std::memory_order_relaxed);
+  }
+}
+
+};  // namespace
+
+namespace CPUThreadConfigCallback
+{
+ConfigChangedCallbackID AddConfigChangedCallback(Config::ConfigChangedCallback func)
+{
+  static auto s_config_changed_callback_id = Config::AddConfigChangedCallback(&OnConfigChanged);
+
+  const ConfigChangedCallbackID callback_id{s_next_callback_id};
+  ++s_next_callback_id;
+  s_callbacks.emplace_back(std::make_pair(callback_id, std::move(func)));
+  return callback_id;
+}
+
+void RemoveConfigChangedCallback(ConfigChangedCallbackID callback_id)
+{
+  for (auto it = s_callbacks.begin(); it != s_callbacks.end(); ++it)
+  {
+    if (it->first == callback_id)
+    {
+      s_callbacks.erase(it);
+      return;
+    }
+  }
+}
+
+void CheckForConfigChanges()
+{
+  DEBUG_ASSERT(Core::IsCPUThread());
+
+  if (s_should_run_callbacks.exchange(false, std::memory_order_relaxed))
+    RunCallbacks();
+}
+
+};  // namespace CPUThreadConfigCallback

Source/Core/Core/CPUThreadConfigCallback.h

@@ -0,0 +1,30 @@
+// Copyright 2023 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include "Common/Config/Config.h"
+
+// This file lets you register callbacks like in Common/Config/Config.h, with the difference that
+// callbacks registered here are guaranteed to run on the CPU thread. Callbacks registered here may
+// run with a slight delay compared to regular config callbacks.
+
+namespace CPUThreadConfigCallback
+{
+struct ConfigChangedCallbackID
+{
+  size_t id = -1;
+
+  bool operator==(const ConfigChangedCallbackID&) const = default;
+  bool operator!=(const ConfigChangedCallbackID&) const = default;
+};
+
+// returns an ID that can be passed to RemoveConfigChangedCallback()
+ConfigChangedCallbackID AddConfigChangedCallback(Config::ConfigChangedCallback func);
+
+void RemoveConfigChangedCallback(ConfigChangedCallbackID callback_id);
+
+// Should be called regularly from the CPU thread
+void CheckForConfigChanges();
+
+};  // namespace CPUThreadConfigCallback

Source/Core/Core/Core.cpp

@@ -41,6 +41,7 @@
 #include "Core/AchievementManager.h"
 #include "Core/Boot/Boot.h"
 #include "Core/BootManager.h"
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Config/MainSettings.h"
 #include "Core/ConfigManager.h"
 #include "Core/CoreTiming.h"
@@ -511,6 +512,9 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
   DeclareAsCPUThread();
   s_frame_step = false;
 
+  // If settings have changed since the previous run, notify callbacks.
+  CPUThreadConfigCallback::CheckForConfigChanges();
+
   // Switch the window used for inputs to the render window. This way, the cursor position
   // is relative to the render window, instead of the main window.
   ASSERT(g_controller_interface.IsInit());

Source/Core/Core/CoreTiming.cpp

@@ -16,6 +16,7 @@
 #include "Common/Logging/Log.h"
 #include "Common/SPSCQueue.h"
 
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Config/MainSettings.h"
 #include "Core/Core.h"
 #include "Core/PowerPC/PowerPC.h"
@@ -88,8 +89,8 @@ void CoreTimingManager::UnregisterAllEvents()
 
 void CoreTimingManager::Init()
 {
-  m_registered_config_callback_id = Config::AddConfigChangedCallback(
-      [this]() { Core::RunAsCPUThread([this]() { RefreshConfig(); }); });
+  m_registered_config_callback_id =
+      CPUThreadConfigCallback::AddConfigChangedCallback([this]() { RefreshConfig(); });
   RefreshConfig();
 
   m_last_oc_factor = m_config_oc_factor;
@@ -118,7 +119,7 @@ void CoreTimingManager::Shutdown()
   MoveEvents();
   ClearPendingEvents();
   UnregisterAllEvents();
-  Config::RemoveConfigChangedCallback(m_registered_config_callback_id);
+  CPUThreadConfigCallback::RemoveConfigChangedCallback(m_registered_config_callback_id);
 }
 
 void CoreTimingManager::RefreshConfig()
@@ -311,11 +312,13 @@ void CoreTimingManager::MoveEvents()
 
 void CoreTimingManager::Advance()
 {
-  auto& power_pc = m_system.GetPowerPC();
-  auto& ppc_state = power_pc.GetPPCState();
+  CPUThreadConfigCallback::CheckForConfigChanges();
 
   MoveEvents();
 
+  auto& power_pc = m_system.GetPowerPC();
+  auto& ppc_state = power_pc.GetPPCState();
+
   int cyclesExecuted = m_globals.slice_length - DowncountToCycles(ppc_state.downcount);
   m_globals.global_timer += cyclesExecuted;
   m_last_oc_factor = m_config_oc_factor;

Source/Core/Core/CoreTiming.h

@@ -23,6 +23,7 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/SPSCQueue.h"
+#include "Core/CPUThreadConfigCallback.h"
 
 class PointerWrap;
 
@@ -182,7 +183,7 @@ private:
 
   EventType* m_ev_lost = nullptr;
 
-  size_t m_registered_config_callback_id = 0;
+  CPUThreadConfigCallback::ConfigChangedCallbackID m_registered_config_callback_id;
   float m_config_oc_factor = 0.0f;
   float m_config_oc_inv_factor = 0.0f;
   bool m_config_sync_on_skip_idle = false;

Source/Core/Core/FifoPlayer/FifoPlayer.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "Common/Assert.h"
+#include "Common/Config/Config.h"
 #include "Core/FifoPlayer/FifoDataFile.h"
 #include "Core/PowerPC/CPUCoreBase.h"
 #include "VideoCommon/CPMemory.h"
@@ -189,7 +190,7 @@ private:
 
   CallbackFunc m_FileLoadedCb = nullptr;
   CallbackFunc m_FrameWrittenCb = nullptr;
-  size_t m_config_changed_callback_id;
+  Config::ConfigChangedCallbackID m_config_changed_callback_id;
 
   std::unique_ptr<FifoDataFile> m_File;
 

Source/Core/Core/FreeLookConfig.cpp

@@ -2,6 +2,8 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "Core/FreeLookConfig.h"
+
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Config/FreeLookSettings.h"
 #include "Core/ConfigManager.h"
 #include "Core/Core.h"
@@ -37,7 +39,7 @@ void Config::Refresh()
 {
   if (!s_has_registered_callback)
   {
-    ::Config::AddConfigChangedCallback([] { Core::RunAsCPUThread([] { s_config.Refresh(); }); });
+    CPUThreadConfigCallback::AddConfigChangedCallback([] { s_config.Refresh(); });
     s_has_registered_callback = true;
   }
 

Source/Core/Core/HW/CPU.cpp

@@ -10,6 +10,7 @@
 #include "AudioCommon/AudioCommon.h"
 #include "Common/CommonTypes.h"
 #include "Common/Event.h"
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Core.h"
 #include "Core/Host.h"
 #include "Core/PowerPC/GDBStub.h"
@@ -75,6 +76,7 @@ void CPUManager::Run()
   {
     m_state_cpu_cvar.wait(state_lock, [this] { return !m_state_paused_and_locked; });
     ExecutePendingJobs(state_lock);
+    CPUThreadConfigCallback::CheckForConfigChanges();
 
     Common::Event gdb_step_sync_event;
     switch (m_state)
@@ -113,6 +115,7 @@ void CPUManager::Run()
       // Wait for step command.
       m_state_cpu_cvar.wait(state_lock, [this, &state_lock, &gdb_step_sync_event] {
         ExecutePendingJobs(state_lock);
+        CPUThreadConfigCallback::CheckForConfigChanges();
         state_lock.unlock();
         if (GDBStub::IsActive() && GDBStub::HasControl())
         {

Source/Core/Core/HW/Wiimote.cpp

@@ -30,7 +30,7 @@ static std::array<u8, MAX_BBMOTES> s_last_connect_request_counter;
 namespace
 {
 static std::array<std::atomic<WiimoteSource>, MAX_BBMOTES> s_wiimote_sources;
-static std::optional<size_t> s_config_callback_id = std::nullopt;
+static std::optional<Config::ConfigChangedCallbackID> s_config_callback_id = std::nullopt;
 
 WiimoteSource GetSource(unsigned int index)
 {

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "Common/Common.h"
+#include "Common/Config/Config.h"
 
 #include "Core/HW/WiimoteCommon/WiimoteReport.h"
 
@@ -343,6 +344,6 @@ private:
 
   IMUCursorState m_imu_cursor_state;
 
-  size_t m_config_changed_callback_id;
+  Config::ConfigChangedCallbackID m_config_changed_callback_id;
 };
 }  // namespace WiimoteEmu

Source/Core/Core/HW/WiimoteReal/WiimoteReal.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "Common/Common.h"
+#include "Common/Config/Config.h"
 #include "Common/Event.h"
 #include "Common/Flag.h"
 #include "Common/SPSCQueue.h"
@@ -157,7 +158,7 @@ private:
   bool m_speaker_enabled_in_dolphin_config = false;
   int m_balance_board_dump_port = 0;
 
-  size_t m_config_changed_callback_id;
+  Config::ConfigChangedCallbackID m_config_changed_callback_id;
 };
 
 class WiimoteScannerBackend

Source/Core/Core/IOS/SDIO/SDIOSlot0.cpp

@@ -15,6 +15,7 @@
 #include "Common/Logging/Log.h"
 #include "Common/SDCardUtil.h"
 
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Config/MainSettings.h"
 #include "Core/Config/SessionSettings.h"
 #include "Core/Core.h"
@@ -32,27 +33,23 @@ SDIOSlot0Device::SDIOSlot0Device(EmulationKernel& ios, const std::string& device
   if (!Config::Get(Config::MAIN_ALLOW_SD_WRITES))
     INFO_LOG_FMT(IOS_SD, "Writes to SD card disabled by user");
 
-  m_config_callback_id = Config::AddConfigChangedCallback([this] { RefreshConfig(); });
+  m_config_callback_id =
+      CPUThreadConfigCallback::AddConfigChangedCallback([this] { RefreshConfig(); });
   m_sd_card_inserted = Config::Get(Config::MAIN_WII_SD_CARD);
 }
 
 SDIOSlot0Device::~SDIOSlot0Device()
 {
-  Config::RemoveConfigChangedCallback(m_config_callback_id);
+  CPUThreadConfigCallback::RemoveConfigChangedCallback(m_config_callback_id);
 }
 
 void SDIOSlot0Device::RefreshConfig()
 {
-  if (m_sd_card_inserted != Config::Get(Config::MAIN_WII_SD_CARD))
+  const bool sd_card_inserted = Config::Get(Config::MAIN_WII_SD_CARD);
+  if (m_sd_card_inserted != sd_card_inserted)
   {
-    Core::RunAsCPUThread([this] {
-      const bool sd_card_inserted = Config::Get(Config::MAIN_WII_SD_CARD);
-      if (m_sd_card_inserted != sd_card_inserted)
-      {
-        m_sd_card_inserted = sd_card_inserted;
-        EventNotify();
-      }
-    });
+    m_sd_card_inserted = sd_card_inserted;
+    EventNotify();
   }
 }
 

Source/Core/Core/IOS/SDIO/SDIOSlot0.h

@@ -10,6 +10,7 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/IOFile.h"
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/IOS/Device.h"
 #include "Core/IOS/IOS.h"
 
@@ -166,7 +167,7 @@ private:
 
   File::IOFile m_card;
 
-  size_t m_config_callback_id;
+  CPUThreadConfigCallback::ConfigChangedCallbackID m_config_callback_id;
   bool m_sd_card_inserted = false;
 };
 }  // namespace IOS::HLE

Source/Core/Core/PowerPC/JitCommon/JitBase.cpp

@@ -7,6 +7,8 @@
 #include "Common/CommonTypes.h"
 #include "Common/MemoryUtil.h"
 #include "Common/Thread.h"
+
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/Config/MainSettings.h"
 #include "Core/ConfigManager.h"
 #include "Core/Core.h"
@@ -64,14 +66,14 @@ JitBase::JitBase(Core::System& system)
     : m_code_buffer(code_buffer_size), m_system(system), m_ppc_state(system.GetPPCState()),
       m_mmu(system.GetMMU())
 {
-  m_registered_config_callback_id = Config::AddConfigChangedCallback(
-      [this] { Core::RunAsCPUThread([this] { RefreshConfig(); }); });
+  m_registered_config_callback_id =
+      CPUThreadConfigCallback::AddConfigChangedCallback([this] { RefreshConfig(); });
   RefreshConfig();
 }
 
 JitBase::~JitBase()
 {
-  Config::RemoveConfigChangedCallback(m_registered_config_callback_id);
+  CPUThreadConfigCallback::RemoveConfigChangedCallback(m_registered_config_callback_id);
 }
 
 void JitBase::RefreshConfig()

Source/Core/Core/PowerPC/JitCommon/JitBase.h

@@ -10,6 +10,7 @@
 #include "Common/BitSet.h"
 #include "Common/CommonTypes.h"
 #include "Common/x64Emitter.h"
+#include "Core/CPUThreadConfigCallback.h"
 #include "Core/ConfigManager.h"
 #include "Core/MachineContext.h"
 #include "Core/PowerPC/CPUCoreBase.h"
@@ -129,7 +130,7 @@ protected:
   PPCAnalyst::CodeBuffer m_code_buffer;
   PPCAnalyst::PPCAnalyzer analyzer;
 
-  size_t m_registered_config_callback_id;
+  CPUThreadConfigCallback::ConfigChangedCallbackID m_registered_config_callback_id;
   bool bJITOff = false;
   bool bJITLoadStoreOff = false;
   bool bJITLoadStorelXzOff = false;

Source/Core/Core/PowerPC/PPCCache.h

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "Common/CommonTypes.h"
+#include "Common/Config/Config.h"
 
 class PointerWrap;
 
@@ -61,7 +62,7 @@ struct Cache
 
 struct InstructionCache : public Cache
 {
-  std::optional<size_t> m_config_callback_id = std::nullopt;
+  std::optional<Config::ConfigChangedCallbackID> m_config_callback_id = std::nullopt;
 
   bool m_disable_icache = false;