From a51de161ed2aa2264c1d1412f552c175b027904a Mon Sep 17 00:00:00 2001 From: PreTenD Date: Wed, 4 Nov 2026 10:35:15 -0600 Subject: [PATCH 1/2] Windows: use high-resolution timers for Event timed waits Block on a high-resolution timer and a signal event instead of polling or rounding every timeout to milliseconds. Give each waiting host thread its own reusable timer and retain blocking fallbacks for unavailable handles. Add Event regression coverage and the Clang sirit header include fix. Assisted-by: OpenAI Codex (implementation, investigation, tests and draft text) --- .patch/sirit/0001-include-cstdlib.patch | 11 ++ cpmfile.json | 5 +- src/common/thread.cpp | 177 +++++++++++------------- src/common/thread.h | 12 ++ src/tests/CMakeLists.txt | 1 + src/tests/common/event.cpp | 103 ++++++++++++++ 6 files changed, 212 insertions(+), 97 deletions(-) create mode 100644 .patch/sirit/0001-include-cstdlib.patch create mode 100644 src/tests/common/event.cpp diff --git a/.patch/sirit/0001-include-cstdlib.patch b/.patch/sirit/0001-include-cstdlib.patch new file mode 100644 index 0000000000..cafe690ac8 --- /dev/null +++ b/.patch/sirit/0001-include-cstdlib.patch @@ -0,0 +1,11 @@ +diff --git a/src/stream.h b/src/stream.h +--- a/src/stream.h ++++ b/src/stream.h +@@ -10,6 +10,7 @@ + #include + #include + #include ++#include + #include + #include + #include diff --git a/cpmfile.json b/cpmfile.json index 6e55707f36..ac6f229d51 100644 --- a/cpmfile.json +++ b/cpmfile.json @@ -279,7 +279,10 @@ "SIRIT_USE_SYSTEM_SPIRV_HEADERS ON" ], "repo": "eden-emulator/sirit", - "version": "v1.0.5" + "version": "v1.0.5", + "patches": [ + "0001-include-cstdlib.patch" + ] }, "sirit-ci": { "ci": true, diff --git a/src/common/thread.cpp b/src/common/thread.cpp index 7b4dec3dce..3148d589ec 100644 --- a/src/common/thread.cpp +++ b/src/common/thread.cpp @@ -5,8 +5,8 @@ // SPDX-FileCopyrightText: 2014 Citra Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later +#include #include -#include #include #include @@ -22,7 +22,6 @@ #elif defined(_WIN32) #include #include "common/string_util.h" -#include "common/windows/timer_resolution.h" #else #if defined(__FreeBSD__) #include @@ -437,16 +436,6 @@ void RememberCurrentThreadNice(pid_t tid, s32 nice_value) { } // Anonymous namespace #endif -#include "common/cpu_features.h" -#ifdef ARCHITECTURE_x86_64 -#ifdef _MSC_VER -#include -#else -#include -#endif -#include "common/x64/rdtsc.h" -#endif - namespace Common { void SetCurrentThreadPriority(ThreadPriority new_priority) { @@ -629,102 +618,98 @@ void PollThreadPolicies() { #endif } -#ifdef ARCHITECTURE_x86_64 -// On Linux and UNIX systems, a futex would nominally be used to cover the costs -// the idea is that it's intuitivelly cheaper to use a direct instruction as opposed to a full futex call -// the underlying libc++ implementation uses pthread_cond_timedwait which MAY invoke a futex -// Let's pretend the OS is too expensive to jump into, and avoid ANY context switches -// this should *IN THEORY* lower CPU usage while just waiting for stuff effectively -// For windows the minimal quanta resolution is about 500us, and normal CRT cond var is 1.5ms(?) -// so may as well avoid that too -// Let's just give ALL platforms the same mechanisms (almost) for when they have umonitor OR waitpkg -#ifdef __clang__ -__attribute__((target("waitpkg,mwaitx"))) -#elif defined(__GNUC__) -#pragma GCC target("waitpkg") -#pragma GCC target("mwaitx") +#ifdef _WIN32 +Event::~Event() { + if (timed_wait_event) { + CloseHandle(timed_wait_event); + } +} + +void Event::WakeTimedWaiters() { + if (timed_wait_event) { + SetEvent(timed_wait_event); + } +} #endif + bool Event::WaitFor(const std::chrono::nanoseconds time) { + std::unique_lock lk{mutex}; #ifdef _WIN32 - auto const start = Common::X64::FencedRDTSC(); - auto const& caps = Common::g_cpu_caps; - [[maybe_unused]] auto const end = start + Common::g_wall_clock.NsToTicks(time); - if (caps.monitorx) { - while (true) { - // Armed monitor, as per manual, MWAITX must be conditional if the condition isn't satisfied - // to prevent a race condition. - _mm_monitorx(reinterpret_cast(std::addressof(is_set)), 0, 0); - if (!is_set.load()) { - // RDTSC may be fenced here due to atomic load -#ifdef _MSC_VER - auto const now = __rdtsc(); -#else - auto const now = _rdtsc(); -#endif - if (end > now) { - u32 const cycles = std::min((std::numeric_limits::max)(), s64(end) - s64(now)); - // See here: https://github.com/torvalds/linux/blob/948a64995aca6820abefd17f1a4258f5835c5ad9/arch/x86/lib/delay.c#L93 - // MWAITX accepts a 32-bit input timer which determines the total number of cycles to wait for - // NOT THE TOTAL ABSOLUTE TSC VALUE, it's just a delta - // BIT[1] = use a timer - // Hint = 0: Use C1 state when sleepy (means slower wakeup but better savings) - _mm_mwaitx(1 << 1, 0u, cycles); - if (!is_set.load()) - return false; - } else - return false; //timeout + using Clock = std::chrono::steady_clock; + const auto now = Clock::now(); + const auto deadline = time > Clock::time_point::max() - now + ? Clock::time_point::max() + : now + time; + while (!is_set.load()) { + if (Clock::now() >= deadline) { + return false; + } + + // Each waiting host thread owns its timer, so concurrent waits cannot rearm + // each other's deadlines. Reuse the handle across the host timing loop. + struct Timer { + Timer() { + handle = CreateWaitableTimerExW(nullptr, nullptr, + CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_MODIFY_STATE | SYNCHRONIZE); + if (!handle) { + handle = CreateWaitableTimerExW(nullptr, nullptr, 0, + TIMER_MODIFY_STATE | SYNCHRONIZE); + } + } + ~Timer() { + if (handle) { + CloseHandle(handle); + } } - bool expected = true; - if (is_set.compare_exchange_weak(expected, false, std::memory_order_release)) - return true; + HANDLE handle{}; + }; + static thread_local Timer timer; + if (!timed_wait_event) { + timed_wait_event = CreateEventW(nullptr, TRUE, FALSE, nullptr); } - } else if (caps.waitpkg) { - // #UD If CPUID.7.0:ECX.WAITPKG[bit 5]=0. - while (true) { - _umonitor(std::addressof(is_set)); - if (!is_set.load() && !_umwait(0, end)) //umwait is absolute time!!! - return false; - bool expected = true; - if (is_set.compare_exchange_weak(expected, false, std::memory_order_release)) - return true; + const auto remaining = deadline - Clock::now(); + if (remaining <= Clock::duration::zero()) { + return false; + } + + // The event is reset while holding the predicate lock. Set() cannot race + // between this reset and the wait without leaving a persistent wake signal. + if (timed_wait_event) { + ResetEvent(timed_wait_event); + using TimerTicks = std::chrono::duration>; + LARGE_INTEGER due{}; + due.QuadPart = -std::chrono::ceil(remaining).count(); + const bool armed = timer.handle && + SetWaitableTimerEx(timer.handle, &due, 0, nullptr, nullptr, nullptr, 0); + const HANDLE handles[]{timed_wait_event, timer.handle}; + const auto milliseconds = std::chrono::ceil(remaining); + const DWORD timeout = static_cast( + (std::min)(milliseconds.count(), INFINITE - 1)); + lk.unlock(); + const DWORD result = armed ? WaitForMultipleObjects(2, handles, FALSE, INFINITE) + : WaitForSingleObject(timed_wait_event, timeout); + if (armed) { + CancelWaitableTimer(timer.handle); + } + lk.lock(); + if (result != WAIT_FAILED) { + continue; + } + } + + // Retain a blocking fallback if Windows cannot allocate or wait on handles. + const auto fallback = std::chrono::ceil(deadline - Clock::now()); + if (fallback > std::chrono::milliseconds::zero()) { + condvar.wait_for(lk, fallback, [this] { return is_set.load(); }); } - } else { -#ifdef _MSC_VER - while (!is_set.load() && end > __rdtsc()) - Common::Windows::SleepForOneTick(); -#else - while (!is_set.load() && end > _rdtsc()) - Common::Windows::SleepForOneTick(); -#endif - if (is_set.load()) - Reset(); - return true; } #else - std::unique_lock lk{mutex}; - if (!condvar.wait_for(lk, time, [this] { return is_set.load(); })) + if (!condvar.wait_for(lk, time, [this] { return is_set.load(); })) { return false; - is_set = false; - return true; + } #endif -} -#else -bool Event::WaitFor(const std::chrono::nanoseconds time) { -#ifdef _WIN32 - auto const end = Common::g_wall_clock.GetTimeNS() + time; - while (!is_set.load() && end > Common::g_wall_clock.GetTimeNS()) - Common::Windows::SleepForOneTick(); - if (is_set.load()) - Reset(); - return true; -#else - std::unique_lock lk{mutex}; - if (!condvar.wait_for(lk, time, [this] { return is_set.load(); })) - return false; is_set = false; return true; -#endif } -#endif } // namespace Common diff --git a/src/common/thread.h b/src/common/thread.h index 4a36295931..69eaa4c0e9 100644 --- a/src/common/thread.h +++ b/src/common/thread.h @@ -20,10 +20,16 @@ namespace Common { class Event { public: +#ifdef _WIN32 + ~Event(); +#endif void Set() { std::scoped_lock lk{mutex}; if (!is_set) { is_set = true; +#ifdef _WIN32 + WakeTimedWaiters(); +#endif condvar.notify_one(); } } @@ -57,9 +63,15 @@ class Event { } private: +#ifdef _WIN32 + void WakeTimedWaiters(); +#endif alignas(64) std::atomic is_set{false}; std::condition_variable condvar; std::mutex mutex; +#ifdef _WIN32 + void* timed_wait_event{}; +#endif }; class Barrier { diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 998a62821a..38a72a6c97 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -8,6 +8,7 @@ add_executable(tests common/bit_field.cpp common/cityhash.cpp common/container_hash.cpp + common/event.cpp common/fibers.cpp common/game_presets.cpp common/host_memory.cpp diff --git a/src/tests/common/event.cpp b/src/tests/common/event.cpp new file mode 100644 index 0000000000..e5b026cb1b --- /dev/null +++ b/src/tests/common/event.cpp @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include +#include +#include +#include "common/thread.h" + +using namespace std::chrono_literals; + +TEST_CASE("Event timed waits consume an existing signal", "[common][event]") { + Common::Event event; + CHECK_FALSE(event.WaitFor(0ns)); + CHECK_FALSE(event.WaitFor(-1ns)); + event.Set(); + CHECK(event.WaitFor(0ns)); + CHECK_FALSE(event.IsSet()); + CHECK_FALSE(event.WaitFor(0ns)); +} + +TEST_CASE("Event short timeouts do not return early", "[common][event]") { + Common::Event event; + for (const auto timeout : {200us, 500us, 2000us}) { + const auto start = std::chrono::steady_clock::now(); + CHECK_FALSE(event.WaitFor(timeout)); + CHECK(std::chrono::steady_clock::now() - start >= timeout); + } +} + +TEST_CASE("Event Set interrupts a long timed wait", "[common][event]") { + Common::Event event; + std::jthread setter([&] { + std::this_thread::sleep_for(5ms); + event.Set(); + }); + CHECK(event.WaitFor(2s)); + setter.join(); + CHECK_FALSE(event.WaitFor(0ns)); +} + +TEST_CASE("Event reset discards a prior timed-wait signal", "[common][event]") { + Common::Event event; + CHECK_FALSE(event.WaitFor(100us)); + event.Set(); + event.Reset(); + CHECK_FALSE(event.WaitFor(1ms)); + event.Set(); + CHECK(event.WaitFor(0ns)); +} + +TEST_CASE("Concurrent Event timed waits have independent deadlines", "[common][event]") { + Common::Event event; + std::barrier ready(3); + bool short_result{}, long_result{}; + std::chrono::steady_clock::duration short_elapsed{}, long_elapsed{}; + std::jthread short_wait([&] { + ready.arrive_and_wait(); + const auto start = std::chrono::steady_clock::now(); + short_result = event.WaitFor(20ms); + short_elapsed = std::chrono::steady_clock::now() - start; + }); + std::jthread long_wait([&] { + ready.arrive_and_wait(); + const auto start = std::chrono::steady_clock::now(); + long_result = event.WaitFor(500ms); + long_elapsed = std::chrono::steady_clock::now() - start; + }); + ready.arrive_and_wait(); + short_wait.join(); + long_wait.join(); + CHECK_FALSE(short_result); + CHECK_FALSE(long_result); + CHECK(short_elapsed >= 20ms); + CHECK(short_elapsed < 400ms); + CHECK(long_elapsed >= 500ms); +} + +TEST_CASE("Event signalling across timed and untimed waits remains consumable", "[common][event]") { + Common::Event event; + Common::Event acknowledgement; + std::atomic completed{true}; + std::jthread worker([&] { + for (int i = 0; i < 100; ++i) { + if (!event.WaitFor(2s)) { + completed = false; + acknowledgement.Set(); + return; + } + acknowledgement.Set(); + } + }); + for (int i = 0; i < 100; ++i) { + event.Set(); + acknowledgement.Wait(); + if (!completed) { + break; + } + } + worker.join(); + CHECK(completed.load()); + CHECK_FALSE(event.WaitFor(0ns)); +} From 3d35a7de2128237d245f02fc904a693bf93161ea Mon Sep 17 00:00:00 2001 From: PreTenD Date: Wed, 4 Nov 2026 11:56:16 -0600 Subject: [PATCH 2/2] common: document Event signal and timed-wait behavior Add Doxygen comments for the changed Event and per-thread timer functions, including signal consumption, timeout results and handle ownership. Assisted-by: OpenAI Codex (documentation, investigation and validation) --- src/common/thread.cpp | 9 +++++++++ src/common/thread.h | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/src/common/thread.cpp b/src/common/thread.cpp index 3148d589ec..d424d194be 100644 --- a/src/common/thread.cpp +++ b/src/common/thread.cpp @@ -619,12 +619,14 @@ void PollThreadPolicies() { } #ifdef _WIN32 +/// @brief Closes the Windows wake handle; all waiters must have finished before destruction. Event::~Event() { if (timed_wait_event) { CloseHandle(timed_wait_event); } } +/// @brief Signals the Windows wake handle if allocated. The caller must hold mutex. void Event::WakeTimedWaiters() { if (timed_wait_event) { SetEvent(timed_wait_event); @@ -632,6 +634,11 @@ void Event::WakeTimedWaiters() { } #endif +/** + * @brief Waits for and consumes a signal, returning false on timeout. + * Windows uses a per-thread timer and shared wake event, with blocking fallbacks. + * An existing signal is consumed even when time is zero or negative. + */ bool Event::WaitFor(const std::chrono::nanoseconds time) { std::unique_lock lk{mutex}; #ifdef _WIN32 @@ -648,6 +655,7 @@ bool Event::WaitFor(const std::chrono::nanoseconds time) { // Each waiting host thread owns its timer, so concurrent waits cannot rearm // each other's deadlines. Reuse the handle across the host timing loop. struct Timer { + /// @brief Creates a high-resolution timer, falling back to a regular timer. Timer() { handle = CreateWaitableTimerExW(nullptr, nullptr, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_MODIFY_STATE | SYNCHRONIZE); @@ -656,6 +664,7 @@ bool Event::WaitFor(const std::chrono::nanoseconds time) { TIMER_MODIFY_STATE | SYNCHRONIZE); } } + /// @brief Closes the allocated timer when this host thread exits. ~Timer() { if (handle) { CloseHandle(handle); diff --git a/src/common/thread.h b/src/common/thread.h index 69eaa4c0e9..e3bbd2ce72 100644 --- a/src/common/thread.h +++ b/src/common/thread.h @@ -23,6 +23,7 @@ class Event { #ifdef _WIN32 ~Event(); #endif + /// @brief Sets a pending signal and notifies waiters; repeated sets coalesce until consumed. void Set() { std::scoped_lock lk{mutex}; if (!is_set) { @@ -40,6 +41,11 @@ class Event { is_set = false; } + /** + * @brief Waits for a signal and consumes it on success. + * @param time Relative timeout; non-positive values only consume an existing signal. + * @return True if a signal was consumed, or false if the wait timed out. + */ bool WaitFor(const std::chrono::nanoseconds time); template