Skip to content

fix(libc): avoid coarse Windows C++ condition waits - #7735

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:codex/fix-libc-cnd-wait-precision
Oct 20, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:codex/fix-libc-cnd-wait-precision

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

On Windows, route the Dinkumware _Cnd_wait and _Cnd_timedwait shim through native condition variables and a CRITICAL_SECTION, avoiding winpthreads' GetTickCount64 timeout loop for C++ condition waits. Keep the existing standard-library path on POSIX. Timed waits round positive remaining durations up to milliseconds and recheck the absolute system-clock deadline after a timeout. The regression test measures the median of 31 one-millisecond waits with the same timer-resolution request used by AnyPS5.

Related report: #1471 documents Windows std::condition_variable::wait_until overshooting by about 15.5 ms for a 1 ms timeout, while SleepConditionVariableSRW(1) returned in 1.1–1.2 ms on the reported setup. This PR addresses the libc Dinkumware condition wait path only; it does not close the broader timing issue.

Tested

  • Linux x86-64: compiled the changed Threads.cpp and GuestLibcCndTimedWait.cpp together with GCC 12.2 and ran the focused test successfully. This covers existing timed timeout, timed signal wake, untimed wake, and the unchanged POSIX implementation.
  • Windows: both changed sources compile to assembly with MinGW GCC 12.2. A Windows binary/runtime test was not available here; the local MinGW assembler cannot assemble the production translation unit's exception metadata with this project configuration. The full CMake configure was also unavailable because the SDL2 submodule is uninitialized and XRandR detection fails.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 1835 core/libs/prx/libc/src/Threads.cpp
PR 5021 core/libs/prx/libc/src/Threads.cpp
PR 6044 core/libs/tests/GuestLibcCndTimedWait.cpp
PR 6879 core/libs/prx/libc/src/Threads.cpp
PR 7201 core/libs/prx/libc/src/Threads.cpp

@MarcZX10

Copy link
Copy Markdown
Contributor Author

I checked the overlap bot's uncompared files manually. #1835, #5021, #6879 and #7201 each change the tv_nsec → system_clock::duration deadline conversion in Threads.cpp; the conversion hunk overlaps this PR, while the native Windows condition-variable wait is distinct. #6044 does not touch GuestLibcCndTimedWait.cpp.

The rounded system_clock::duration conversion is retained in this PR's Windows path. These branches are currently dirty against main, and #7735 has no functional dependency on them. If one rebases or merges first, this small conversion hunk can be reconciled while preserving the rounding.

@github-actions github-actions Bot added the libc label Oct 19, 2026
@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@harmsluis

Copy link
Copy Markdown

Windows run (Windows 11 build 26300, non-elevated, WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, BUILD_TESTING=ON). main 965dc2d78 has not changed Threads.cpp or this test since the PR's merge-base 676d42396; git merge-tree reports no conflicts.

  • main 965dc2d78, the test as it is on main: guest_libc_cnd_timed_wait passes 3/3. That build does not include the new 1 ms check.
  • Control, this PR's GuestLibcCndTimedWait.cpp on main's libc: the new check aborts (0xc0000409) 3/3. A local print of the 31 samples (not part of the PR, removed before the run below) gave medians of 15.516, 15.839 and 15.998 ms.
  • This PR (31483262c) on 965dc2d78: Threads.cpp builds with no new warnings. The same local print gave medians of 1.503, 1.505 and 1.504 ms; one run's 31 samples ran from 1.005 to 2.529 ms. The PR's test, without that print, passes 5/5 (ctest --repeat until-fail:5).

An earlier run today on main 85029a00f, same PR head, matched this: unfixed medians 15.970, 15.951 and 15.510 ms (abort 3/3), fixed medians 1.494, 1.508 and 1.496 ms, and the unmodified test passed 5/5.

Not covered: the full build, and any title run.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the controlled Windows comparison. The baseline test passes while the new timing assertion reproduces the coarse wait failure 3/3; with the PR it passes 5/5, and the measured medians drop from roughly 15.5–16.0 ms to roughly 1.5 ms. I’ll cite this as focused Windows validation and keep the full-build and title-run limitations explicit.

@adriadam10
adriadam10 merged commit 54c289b into boykopovar:main Oct 20, 2026
18 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the Windows _Cnd_wait/_Cnd_timedwait paths (native CONDITION_VARIABLE + CRITICAL_SECTION, ms round-up, deadline recheck on ERROR_TIMEOUT, signal/broadcast) and the POSIX path (unchanged), and that the new 31-sample 1 ms median test and the untimed-wait case pin the behaviour; the full Linux and Windows Build covers the Windows runtime. Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants