Skip to content

Windows: use high-resolution timers for Event timed waits - #538

Open
pretend7979 wants to merge 2 commits into
Eglantine-Community:nx2from
pretend7979:review/windows-high-resolution-waits
Open

pretend7979 wants to merge 2 commits into
Eglantine-Community:nx2from
pretend7979:review/windows-high-resolution-waits

Conversation

@pretend7979

@pretend7979 pretend7979 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Replace Windows Event::WaitFor polling with a blocking wait on a high-resolution timer and a signal event. This aims to reduce CPU usage while avoiding the whole-millisecond timeout rounding of the earlier WaitOnAddress proposal.

Each waiting host thread owns a reusable timer, preventing concurrent waits from overwriting each other’s deadlines. Set() interrupts the wait, and a successful wait consumes the signal. Blocking fallbacks remain available when high-resolution timers or handles cannot be used.

Non-Windows wait behaviour is unchanged. This also includes the missing <cstdlib> include for the sirit dependency when building with Clang.

Type of change

  • Bug fix
  • Game compatibility / game-support
  • Performance
  • New feature
  • Refactor / cleanup
  • CI / build
  • Documentation

Related issues

Separated from #532 following review feedback about emulator-wide timing changes.

Testing

  • Tested on Windows
  • Tested on Linux
  • Tested on macOS
  • Tested on NVIDIA
  • Tested on AMD
  • Tested on Intel

The candidate Windows implementation compiled locally with Clang/MinGW.

Six Event regression cases passed, with 23 assertions covering:

  • Existing signals and signal consumption.
  • Nonpositive and short timeouts.
  • Early signalling and reset.
  • Independent concurrent deadlines.
  • Repeated timed/untimed handoffs.

Games tested:

Full emulator/game testing of this high-resolution implementation is pending. Earlier Star Fox and Mario Kart testing used the previous timer implementation.

Checklist

  • I built it locally and it compiles without new warnings.
  • I did not add any copyrighted game data, keys or firmware.
  • Docs updated if needed.
  • I added AI Disclosure if one was used.

Screenshots / logs

Idle Windows benchmark, 1,000 waits per duration. The baseline is the previous WaitOnAddress prototype.

Requested wait WaitOnAddress median / p95 High-resolution median / p95
250 µs 1,015 / 1,514 µs 499 / 517 µs
500 µs 1,015 / 1,517 µs 1,039 / 1,109 µs
1,000 µs 1,498 / 2,502 µs 1,514 / 1,586 µs

Neither blocking implementation returned early in these measurements. Sampled thread CPU usage was approximately 0–3% of one logical CPU, with accounting-granularity limitations.

The results show reduced latency or jitter in some cases, but do not establish microsecond-accurate wake-ups. CoreTiming also drives audio, vsync, syncpoint polling and guest timeouts, so broader testing is needed before merging.

AI disclosure

Assisted-by: OpenAI Codex (implementation, investigation, tests and draft text)

Summary by CodeRabbit

  • Bug Fixes
    • Timed event waits now report timeouts accurately instead of treating them as successful signals.
    • Waits respond more consistently when an event is signaled or reset, including when multiple threads are waiting concurrently.
    • Improved timed-wait behavior across Windows and other supported platforms, including more reliable handling of signals that arrive near a timeout.
    • Non-positive timeouts now consume an existing signal when available and otherwise return without waiting.

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)
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Timed event waits now use deadline-based waiting and return whether they consumed a signal. Windows uses wait handles with a condition-variable fallback; other platforms use condition variables. The Sirit dependency applies a patch that adds <cstdlib> to src/stream.h.

Changes: Timed Event Waits

Layer / File(s) Summary
Event wake support
src/common/thread.h, src/common/thread.cpp
On Windows, Set() signals timed waiters when the event changes from unset to set. The destructor closes the shared timed-wait event handle.
Deadline-based waits and validation
src/common/thread.h, src/common/thread.cpp, src/tests/CMakeLists.txt, src/tests/common/event.cpp
WaitFor uses a steady-clock deadline. Windows uses a per-thread timer and a shared event, with a condition-variable fallback; other platforms use a condition variable. Tests cover timeouts, signal consumption, resets, and concurrent waits.

Sirit Include Patch

Layer / File(s) Summary
Sirit include patch wiring
.patch/sirit/0001-include-cstdlib.patch, cpmfile.json
The Sirit dependency remains at v1.0.5 and applies a patch that adds <cstdlib> to src/stream.h.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Waiter
  participant Event
  participant WakeEvent
  Waiter->>Event: Call WaitFor
  Event->>WakeEvent: Wait using the shared wake event
  Event->>WakeEvent: Set signals the event
  WakeEvent-->>Event: Wake timed waiter
  Event-->>Waiter: Return true and consume signal
Loading

Possibly related PRs

  • Djihads80/Eglantine#532: Changes the same Windows Common::Event::WaitFor behavior, including timeout handling and signal consumption.

Suggested labels: bug

Suggested reviewers: crueter, camillelavey

Merge Risk: Minimal · up to 3d35a

No specific issue is established that would prevent merging after normal validation. Broader emulator and game testing of the Windows wait implementation remains pending.

Security architecture risk: Low · up to a51de

The inspected callers retain their existing scope, and no introduced security vulnerability was established. The main residual risk is Windows wait and shutdown behavior under concurrency or operating-system failures, which was reviewed statically rather than exercised here.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated impact is host-process timing and audio shutdown. The inspected change does not establish newly granted authority or a new cross-process interface; this bounded assessment does not establish safety for unexamined external consumers.

Trust Boundaries and Controls

  • inferred — The changed wait mechanics preserve the existing process-local synchronization boundary. Signal ownership is controlled by the shared predicate and mutex, rather than by possession of the native wake state; no introduced attacker-to-privileged-sink path was established in the inspected callers.

Resilience and Maintainability Implications

  • inferred — Static transition analysis found no demonstrated loss of signal ownership or cross-thread deadline overwrite through normal waits, timeout, repetition, or inspected fallback branches. CoreTiming requests stop before waking and joining its worker. Concurrent Event destruction remains outside the supported object-lifetime assumption; Windows API failure recovery was not fault-injected.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: using high-resolution timers for Windows Event timed waits.
Docstring Coverage Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 3 files.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b4267707-e455-4d8d-b062-2b19c3695752
Commits

Reviewing files that changed from the base of the PR and between a51de16 and 3d35a7d.

Files selected for processing: 2
  • src/common/thread.cpp
  • src/common/thread.h
Files skipped from review as they are similar to previous changes: 2
  • src/common/thread.h
  • src/common/thread.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

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)
mollie2999 pushed a commit to mollie2999/Eglantine that referenced this pull request Nov 5, 2026
Keep the existing monitor-based waiting policy, but check a fixed saturating deadline after every wake. Bound and round cycle budgets before narrowing to the 32-bit MWAITX counter. Preserve pending-signal consumption for nonpositive durations.

Retain the six Event regressions from Eglantine Eglantine-Community#538 (a51de16) without importing its high-resolution timer policy. Add arithmetic, reset-wake and extreme-duration coverage plus a Windows QPC verification probe. The extreme-duration test also exposes condition_variable wait_for overflow on Linux; use the same saturating deadline with wait_until there.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Underfloww pushed a commit to Underfloww/Elganflow that referenced this pull request Nov 5, 2026
…ntine-Community#120)

* common: retry unsignalled MWAITX wakes until the Event deadline

Keep the existing monitor-based waiting policy, but check a fixed saturating deadline after every wake. Bound and round cycle budgets before narrowing to the 32-bit MWAITX counter. Preserve pending-signal consumption for nonpositive durations.

Retain the six Event regressions from Eglantine Eglantine-Community#538 (a51de16) without importing its high-resolution timer policy. Add arithmetic, reset-wake and extreme-duration coverage plus a Windows QPC verification probe. The extreme-duration test also exposes condition_variable wait_for overflow on Linux; use the same saturating deadline with wait_until there.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* common: make steady clock authoritative for MWAITX timeout expiry

The hardware clock uses a sampled TSC frequency rounded to 100 kHz; its nanoseconds can advance faster than Windows QPC. The prior deadline could therefore expire early on steady_clock even though the MWAITX tick budget rounded upward. Use steady_clock for both deadline construction and expiry, retaining the TSC estimate only for bounded hardware wait durations.

Keep the existing concurrent timeout regression unchanged and add repeated 100 ms and 500 ms steady-clock checks.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
kraibse pushed a commit that referenced this pull request Nov 6, 2026
* common: retry unsignalled MWAITX wakes until the Event deadline

Keep the existing monitor-based waiting policy, but check a fixed saturating deadline after every wake. Bound and round cycle budgets before narrowing to the 32-bit MWAITX counter. Preserve pending-signal consumption for nonpositive durations.

Retain the six Event regressions from Eglantine #538 (a51de16) without importing its high-resolution timer policy. Add arithmetic, reset-wake and extreme-duration coverage plus a Windows QPC verification probe. The extreme-duration test also exposes condition_variable wait_for overflow on Linux; use the same saturating deadline with wait_until there.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* common: make steady clock authoritative for MWAITX timeout expiry

The hardware clock uses a sampled TSC frequency rounded to 100 kHz; its nanoseconds can advance faster than Windows QPC. The prior deadline could therefore expire early on steady_clock even though the MWAITX tick budget rounded upward. Use steady_clock for both deadline construction and expiry, retaining the TSC estimate only for bounded hardware wait durations.

Keep the existing concurrent timeout regression unchanged and add repeated 100 ms and 500 ms steady-clock checks.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
@PastaLaPate

Copy link
Copy Markdown
Collaborator

can you fix conflicts ?

adamhahnHU pushed a commit to adamhahnHU/EglantineAMD that referenced this pull request Nov 7, 2026
…ntine-Community#120)

* common: retry unsignalled MWAITX wakes until the Event deadline

Keep the existing monitor-based waiting policy, but check a fixed saturating deadline after every wake. Bound and round cycle budgets before narrowing to the 32-bit MWAITX counter. Preserve pending-signal consumption for nonpositive durations.

Retain the six Event regressions from Eglantine Eglantine-Community#538 (a51de16) without importing its high-resolution timer policy. Add arithmetic, reset-wake and extreme-duration coverage plus a Windows QPC verification probe. The extreme-duration test also exposes condition_variable wait_for overflow on Linux; use the same saturating deadline with wait_until there.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* common: make steady clock authoritative for MWAITX timeout expiry

The hardware clock uses a sampled TSC frequency rounded to 100 kHz; its nanoseconds can advance faster than Windows QPC. The prior deadline could therefore expire early on steady_clock even though the MWAITX tick budget rounded upward. Use steady_clock for both deadline construction and expiry, retaining the TSC estimate only for bounded hardware wait durations.

Keep the existing concurrent timeout regression unchanged and add repeated 100 ms and 500 ms steady-clock checks.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
(cherry picked from commit 687a494)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJNfzVjvfnqqaEjKL3CyAB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants