From 3273e8081e2b2ec4daad17b559f9fe2d0c26fd44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 17 Aug 2026 00:50:39 +0200 Subject: [PATCH] Push game and stepping events from the CPU thread instead of polling for them GameBroadcaster and SteppingBroadcaster ran per connection on the WebSocket thread, so every connected debugger was reading pc, the tick count, coreState, the UI state and the param SFO out from under the CPU thread on every lap of its loop - up to 1000 times a second in high-activity mode. Inverted: the CPU thread notices the transition once in WebSocketDebuggerTick(), formats the event there, and drops it into a per-connection mailbox that the connection's own thread drains and sends. Same events, same conditions, no core reads off the CPU thread, and no per-connection polling of emulator state. The tick hangs off Core_ProcessCPUQueue(), the one function reliably called on the CPU thread both in game (Core_RunLoopUntil) and at the menu (NativeFrame). It polls even with nothing connected, since skipping would let the "previous state" go stale and fire a bogus event at whoever connects next. Behavior preserved including the awkward bit: a debugger that connects while the CPU is already stopped still gets an immediate cpu.stepping, which used to fall out of SteppingBroadcaster's counter starting at 0. That's now an explicit per-connection prime instead of an accident. Part of removing the WebSocket debugger's lifecycleLock. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/Core.cpp | 5 ++ Core/Debugger/WebSocket.cpp | 86 +++++++++++++++++-- Core/Debugger/WebSocket.h | 5 ++ Core/Debugger/WebSocket/GameBroadcaster.cpp | 40 +++++---- Core/Debugger/WebSocket/GameBroadcaster.h | 22 ++--- .../WebSocket/SteppingBroadcaster.cpp | 42 ++++++--- Core/Debugger/WebSocket/SteppingBroadcaster.h | 25 +++--- 7 files changed, 160 insertions(+), 65 deletions(-) diff --git a/Core/Core.cpp b/Core/Core.cpp index d841f1ad2e..4c686dd95b 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -40,6 +40,7 @@ #include "Core/System.h" #include "Core/MemFault.h" #include "Core/Debugger/Breakpoints.h" +#include "Core/Debugger/WebSocket.h" #include "Core/MIPS/MIPS.h" #include "Core/MIPS/MIPSAnalyst.h" #include "Core/HLE/sceKernelModule.h" @@ -113,6 +114,10 @@ void Core_ProcessCPUQueue() { g_cpuThreadIdValid.store(true, std::memory_order_release); }); + // Piggybacking on the one function that's reliably called on the CPU thread both in game + // (Core_RunLoopUntil) and at the menu (NativeFrame) - see WebSocketDebuggerTick(). + WebSocketDebuggerTick(); + std::vector> tasks; { std::lock_guard guard(g_cpuQueueMutex); diff --git a/Core/Debugger/WebSocket.cpp b/Core/Debugger/WebSocket.cpp index 8aa1f00a4e..bd13775b5d 100644 --- a/Core/Debugger/WebSocket.cpp +++ b/Core/Debugger/WebSocket.cpp @@ -15,8 +15,10 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include #include #include +#include #include "Common/Thread/ThreadUtil.h" #include "Common/TimeUtil.h" @@ -102,6 +104,71 @@ static void UpdateConnected(int delta) { stopCond.notify_all(); } +// Per-connection mailbox for events the CPU thread produces (cpu.stepping, game.start, ...). +// +// These used to be polled per connection from the WebSocket thread, which meant every connected +// debugger was reading pc, the tick count, the UI state and the param SFO out from under the CPU +// thread on every lap of its loop. Now the CPU thread notices the transition once, formats the +// event, and drops it in here; the connection's own thread just drains and sends. +struct DebuggerEventSink { + std::mutex lock; + std::vector> pending; + // A debugger that connects while the CPU is already stopped still wants to hear about it. + bool needsSteppingPrime = true; + + void Push(const char *category, std::string json) { + std::lock_guard guard(lock); + pending.emplace_back(category, std::move(json)); + } + + void Take(std::vector> *out) { + std::lock_guard guard(lock); + out->swap(pending); + pending.clear(); + } +}; + +static std::mutex g_sinkLock; +static std::vector g_sinks; + +static void RegisterSink(DebuggerEventSink *sink) { + std::lock_guard guard(g_sinkLock); + g_sinks.push_back(sink); +} + +static void UnregisterSink(DebuggerEventSink *sink) { + std::lock_guard guard(g_sinkLock); + g_sinks.erase(std::remove(g_sinks.begin(), g_sinks.end(), sink), g_sinks.end()); +} + +void WebSocketDebuggerTick() { + // Poll unconditionally, even with nothing connected: these track transitions, and skipping them + // would let the "previous" state go stale and fire a bogus event at whoever connects next. + const std::string gameEvent = GameBroadcaster::PollChange(); + const std::string steppingEvent = SteppingBroadcaster::PollChange(); + + std::lock_guard guard(g_sinkLock); + if (g_sinks.empty()) + return; + + std::string steppingPrime; + for (DebuggerEventSink *sink : g_sinks) { + if (sink->needsSteppingPrime) { + sink->needsSteppingPrime = false; + // Only format it if somebody actually needs it. + if (steppingPrime.empty()) + steppingPrime = SteppingBroadcaster::CurrentState(); + if (!steppingPrime.empty()) + sink->Push("stepping", steppingPrime); + continue; + } + if (!gameEvent.empty()) + sink->Push("game", gameEvent); + if (!steppingEvent.empty()) + sink->Push("stepping", steppingEvent); + } +} + static void WebSocketNotifyLifecycle(CoreLifecycle stage) { switch (stage) { case CoreLifecycle::STARTING: @@ -153,10 +220,11 @@ void HandleDebuggerRequest(const http::ServerRequest &request) { WebSocketClientInfo client_info; auto& disallowed_config = client_info.disallowed; - GameBroadcaster game; LogBroadcaster logger; InputBroadcaster input; - SteppingBroadcaster stepping; + + DebuggerEventSink sink; + RegisterSink(&sink); DebuggerEventHandlerMap eventHandlers; std::vector subscriberData; @@ -213,13 +281,17 @@ void HandleDebuggerRequest(const http::ServerRequest &request) { // so we check the client settings first if (!disallowed_config["logger"]) logger.Broadcast(ws); - if (!disallowed_config["game"]) - game.Broadcast(ws); - if (!disallowed_config["stepping"]) - stepping.Broadcast(ws); if (!disallowed_config["input"]) input.Broadcast(ws); + // Whatever the CPU thread queued up for us since last lap. + std::vector> events; + sink.Take(&events); + for (const auto &ev : events) { + if (!disallowed_config[ev.first]) + ws->Send(ev.second); + } + for (size_t i = 0; i < subscribers.size(); ++i) { if (subscriberData[i]) { subscriberData[i]->Broadcast(ws); @@ -235,6 +307,8 @@ void HandleDebuggerRequest(const http::ServerRequest &request) { } } + UnregisterSink(&sink); + std::lock_guard guard(lifecycleLock); for (size_t i = 0; i < subscribers.size(); ++i) { delete subscriberData[i]; diff --git a/Core/Debugger/WebSocket.h b/Core/Debugger/WebSocket.h index f05856db20..ef6c823c8b 100644 --- a/Core/Debugger/WebSocket.h +++ b/Core/Debugger/WebSocket.h @@ -24,3 +24,8 @@ class ServerRequest; void HandleDebuggerRequest(const http::ServerRequest &request); // Note: blocks. void StopAllDebuggers(); + +// Notices emulator state changes (cpu.stepping, game.start, ...) and pushes the resulting events to +// connected debuggers, so their own threads never have to read that state themselves. CPU thread +// only; cheap, and safe to call with no debugger connected (it still has to track transitions). +void WebSocketDebuggerTick(); diff --git a/Core/Debugger/WebSocket/GameBroadcaster.cpp b/Core/Debugger/WebSocket/GameBroadcaster.cpp index 755f041728..f6a1cca51b 100644 --- a/Core/Debugger/WebSocket/GameBroadcaster.cpp +++ b/Core/Debugger/WebSocket/GameBroadcaster.cpp @@ -73,22 +73,28 @@ struct GameStatusEvent { // - id: string disc ID (such as ULUS12345.) // - version: string disc version. // - title: string game title. -void GameBroadcaster::Broadcast(net::WebSocketServer *ws) { - // TODO: This is ugly. Callbacks instead? - GlobalUIState state = GetUIState(); - if (prevState_ != state) { - if (state == UISTATE_PAUSEMENU) { - ws->Send(GameStatusEvent{"game.pause"}); - prevState_ = state; - } else if (state == UISTATE_INGAME && prevState_ == UISTATE_PAUSEMENU) { - ws->Send(GameStatusEvent{"game.resume"}); - prevState_ = state; - } else if (state == UISTATE_INGAME && PSP_GetBootState() == BootState::Complete) { - ws->Send(GameStatusEvent{"game.start"}); - prevState_ = state; - } else if (state == UISTATE_MENU && PSP_GetBootState() != BootState::Complete) { - ws->Send(GameStatusEvent{"game.quit"}); - prevState_ = state; - } +std::string GameBroadcaster::PollChange() { + // Tracked globally rather than per connection: this runs on the CPU thread, which owns the state + // being read, and the resulting event is then handed to every connected debugger. + static GlobalUIState prevState = GetUIState(); + + const GlobalUIState state = GetUIState(); + if (prevState == state) + return std::string(); + + const char *ev = nullptr; + if (state == UISTATE_PAUSEMENU) { + ev = "game.pause"; + } else if (state == UISTATE_INGAME && prevState == UISTATE_PAUSEMENU) { + ev = "game.resume"; + } else if (state == UISTATE_INGAME && PSP_GetBootState() == BootState::Complete) { + ev = "game.start"; + } else if (state == UISTATE_MENU && PSP_GetBootState() != BootState::Complete) { + ev = "game.quit"; } + if (!ev) + return std::string(); + + prevState = state; + return GameStatusEvent{ev}; } diff --git a/Core/Debugger/WebSocket/GameBroadcaster.h b/Core/Debugger/WebSocket/GameBroadcaster.h index 63ef3eb346..3539a523a7 100644 --- a/Core/Debugger/WebSocket/GameBroadcaster.h +++ b/Core/Debugger/WebSocket/GameBroadcaster.h @@ -17,20 +17,14 @@ #pragma once -#include "Core/System.h" +#include -namespace net { -class WebSocketServer; -} +namespace GameBroadcaster { -struct GameBroadcaster { -public: - GameBroadcaster() { - prevState_ = GetUIState(); - } +// Notices game start/quit/pause/resume transitions and returns the formatted event for one, or an +// empty string if nothing changed. CPU thread only - it reads the UI state and the param SFO, and +// is what lets connected debuggers hear about this without touching either from their own threads. +// Must be called even when no debugger is connected, so the transition state doesn't go stale. +std::string PollChange(); - void Broadcast(net::WebSocketServer *ws); - -private: - GlobalUIState prevState_; -}; +} // namespace GameBroadcaster diff --git a/Core/Debugger/WebSocket/SteppingBroadcaster.cpp b/Core/Debugger/WebSocket/SteppingBroadcaster.cpp index 0f4fe66fa8..76bef1edb0 100644 --- a/Core/Debugger/WebSocket/SteppingBroadcaster.cpp +++ b/Core/Debugger/WebSocket/SteppingBroadcaster.cpp @@ -56,19 +56,33 @@ private: // CPU has resumed from stepping (cpu.resume) // // Sent unexpectedly with no other properties. -void SteppingBroadcaster::Broadcast(net::WebSocketServer *ws) { - if (PSP_GetBootState() == BootState::Complete) { - int steppingCounter = Core_GetSteppingCounter(); - // We ignore CORE_POWERDOWN as a stepping state. - if (coreState == CORE_STEPPING_CPU && steppingCounter != lastCounter_) { - ws->Send(CPUSteppingEvent(Core_GetSteppingReason())); - } else if (prevState_ == CORE_STEPPING_CPU && coreState != CORE_STEPPING_CPU && Core_IsActive()) { - ws->Send(R"({"event":"cpu.resume"})"); - } - lastCounter_ = steppingCounter; - prevState_ = coreState; - } else { - lastCounter_ = -1; - prevState_ = CORE_POWERDOWN; +// Tracked globally rather than per connection: this runs on the CPU thread, which owns the state +// being read, and the resulting event is then handed to every connected debugger. +static CoreState g_prevState = CORE_POWERDOWN; +static int g_lastCounter = 0; + +std::string SteppingBroadcaster::PollChange() { + if (PSP_GetBootState() != BootState::Complete) { + g_lastCounter = -1; + g_prevState = CORE_POWERDOWN; + return std::string(); } + + std::string result; + const int steppingCounter = Core_GetSteppingCounter(); + // We ignore CORE_POWERDOWN as a stepping state. + if (coreState == CORE_STEPPING_CPU && steppingCounter != g_lastCounter) { + result = CPUSteppingEvent(Core_GetSteppingReason()); + } else if (g_prevState == CORE_STEPPING_CPU && coreState != CORE_STEPPING_CPU && Core_IsActive()) { + result = R"({"event":"cpu.resume"})"; + } + g_lastCounter = steppingCounter; + g_prevState = coreState; + return result; +} + +std::string SteppingBroadcaster::CurrentState() { + if (PSP_GetBootState() != BootState::Complete || coreState != CORE_STEPPING_CPU) + return std::string(); + return CPUSteppingEvent(Core_GetSteppingReason()); } diff --git a/Core/Debugger/WebSocket/SteppingBroadcaster.h b/Core/Debugger/WebSocket/SteppingBroadcaster.h index 4ca34f4552..1a697044e9 100644 --- a/Core/Debugger/WebSocket/SteppingBroadcaster.h +++ b/Core/Debugger/WebSocket/SteppingBroadcaster.h @@ -17,21 +17,18 @@ #pragma once -#include "Core/Core.h" +#include -namespace net { -class WebSocketServer; -} +namespace SteppingBroadcaster { -struct SteppingBroadcaster { -public: - SteppingBroadcaster() { - prevState_ = coreState; - } +// Notices the CPU entering or leaving stepping and returns the formatted event for one, or an empty +// string if nothing changed. CPU thread only - it reads pc and the tick count, which is exactly what +// connected debuggers must not do from their own threads. Must be called even when no debugger is +// connected, so the transition state doesn't go stale. +std::string PollChange(); - void Broadcast(net::WebSocketServer *ws); +// The cpu.stepping event describing the state right now, for a debugger that connected while the +// CPU was already stopped. Empty if it isn't stepping. CPU thread only. +std::string CurrentState(); -private: - CoreState prevState_; - int lastCounter_ = 0; -}; +} // namespace SteppingBroadcaster