Skip to content

perf(os): wake WaitAny only for objects a multi-wait watches - #1353

Merged
NotRexed merged 6 commits into
NotRexed:mainfrom
klimovich008:host-multiwait-targeted-wake
Nov 10, 2026
Merged

perf(os): wake WaitAny only for objects a multi-wait watches#1353
NotRexed merged 6 commits into
NotRexed:mainfrom
klimovich008:host-multiwait-targeted-wake

Conversation

@klimovich008

@klimovich008 klimovich008 commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this fixes

In offline OoT, unrelated event, timer, semaphore and queue changes repeatedly wake online-service threads in WaitAny, consuming CPU while their events remain idle.

Why merge

  • Filters only changes to objects no multi-wait watches. State updates and individual notifications stay unchanged.
  • Counts follow guest addresses across reset and teardown; Link/Unlink avoid target lookups.
  • Keeps the 5 ms recheck. Allocation failure restores broadcasts; HOS_MW_WAKE=all forces the old policy.

Change

  • host/nn_os_sync.c: track watched addresses under mw_lock and maintain holder counts.
  • tests/test_multiwait_wake.py: check lifetimes, notification filtering and allocation failure.
  • tests/run_all.py: register the check.

Tested

  • python -I -B tests/test_multiwait_wake.py --cxx clang++ compiles the real table, poke and holder operations with counting stubs and injected allocation failure. Fails on the old code, passes now.
  • v2.0.11 Hyrule, 32:9, three alternating runs per arm, same executable, HOS_MW_WAKE=all control: CPU ms/frame −6.4%; median fps 55.5 → 57.3, every pair higher.
  • Windows, RX 9070 XT: Hyrule/Kokiri/Temple, 96–97 seconds each; expected geometry, original saves unchanged, no fatal markers. Existing VUID-VkImageCreateInfo-imageCreateMaxMipLevels-02251 remains.

Not covered

  • Tested on AMD Windows only; Linux and smaller CPUs untested.
  • Native scene pass precedes the allocation fallback. No save-write/reload cycle or long soak.

klimovich008 and others added 3 commits November 9, 2026 21:30
@klimovich008 @claude
perf(os): WaitAny woken only by objects a multi-wait links
d40d1a4 
Every event signal, semaphore release, queue change and timer start
poked one global condition variable that every WaitAny sleeps on. So each
change anywhere in the game woke every multi-wait waiter to re-check its
holders. OoT's three nn.npln threads wait on their own events, which are
never set offline, and woke ~8,500 times a second each: 57% of the
game's context switches and ~0.4 of a logical CPU in a WPR capture.

Each event, timer, semaphore and queue now counts the holders that link
it into a multi-wait (LinkMultiWaitHolder / UnlinkMultiWaitHolder), and
poke_multiwait(o) wakes WaitAny only when that count is non-zero. The
signal stores and then reads the count with a full barrier; a link
increments it with one before the WaitAny reads the state, so one side
always sees the other. Thread and native-handle holders were never woken
by a poke. HOS_MW_WAKE=all restores the old wake-everything behaviour.

tests/test_multiwait_wake.py compiles the real poke_multiwait with a
counting harness and checks every poke site and the link/unlink count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@klimovich008 @claude
fix(os): UnlinkAllMultiWaitHolder lowers its objects' link counts
34dd8c3 
Review: UnlinkAll cleared the multi-wait's links without decrementing the
watched objects' counts, so after Link -> UnlinkAll those objects kept
phantom watchers and their changes woke every WaitAny again. MoveAll
moves holders between multi-waits and leaves the counts as they are.
(A holder finalized while still linked keeps its count: more wake-ups,
as before this change, not fewer; the 5 ms re-check stays a latency
backstop.)

tests/test_multiwait_wake.py: the real Link/Unlink/UnlinkAll/MoveAll and
holder_target compiled together, two multi-waits, two holders over one
event, one over another and a thread holder; each object's count is
checked after every step. Fails on the previous commit at UnlinkAll.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@klimovich008 @claude
fix(os): holder_target looks objects up without a scheduling point
d36db14 
Review: holder_target runs under mw_lock (Link, Unlink, UnlinkAll), and
obj() is a virtual-core scheduling point (hos_vcore_checkpoint): with
HOS_VCORE on and a kick pending it could give its core away while
holding mw_lock, and a higher-priority thread then blocking on mw_lock
waits until the 250 ms escape. obj_lookup is the same lookup without the
checkpoint (the HLE entries already checkpoint before taking the lock).

tests/test_multiwait_wake.py: the link fixture counts obj() calls made
while mw_lock is held and fails on any (the previous commit made 8).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@klimovich008

klimovich008 commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, both lifetime issues were real. Fixed in f0fe694 by moving the count off the object.

  • Re-init: the watched addresses and their holder counts now live in a small table under mw_lock, keyed by the guest address the holder links. fresh() and the tombstone memset no longer touch the count. The fixture re-initializes a linked event, signals it, and checks that it still wakes.
  • Lookups at teardown: Link, Unlink and UnlinkAll no longer look the target up at all; they use the holder's address. poke_multiwait checks the table under mw_lock, which it already took before broadcasting, and only when a WaitAny is waiting. A link before the check is seen; a link after it is followed by a scan that sees the stored state. The fixture finalizes an event before unlinking it and checks that nothing is resurrected and that no object lookup happens in link, unlink or poke.
  • Nit: right, wait_any / holder_ready already call obj under mw_lock. That's pre-existing, and this PR no longer adds any lookup under the lock or claims there are none.

The fixture is still single-threaded. The cross-thread argument is that every read and write of the table happens under mw_lock.

klimovich008 and others added 3 commits November 10, 2026 00:48
@klimovich008 @claude
fix(os): count multi-wait watchers by guest address, not on the object
f0fe694 
Review of NotRexed#1353: the count lived on the target Obj, so re-initializing
the target (fresh() -> the tombstone path's memset, which several
SystemEvent getters do on every call) zeroed it while holders still
linked it, and Unlink/UnlinkAll looked the target up with obj_lookup,
which creates objects: after a finalize it brought the tombstone back,
or turned a reused address's live object into an event.

The watched addresses and their holder counts now live in a small table
under mw_lock. Link/Unlink/UnlinkAll update it by the holder's guest
address and never look the target up; poke_multiwait checks it under
mw_lock (taken there before too) only when a WaitAny is waiting, so a
link before the check is seen and a link after it is followed by a scan
that sees the stored state.

tests/test_multiwait_wake.py compiles the real table, poke_multiwait,
holder_watch and Link/Unlink/UnlinkAll/MoveAll: counts after every step,
a re-initialized watched event still wakes, finalize-before-unlink
resurrects nothing, and no object lookup happens in link, unlink or poke.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@klimovich008 @claude
fix(os): mw_ names for the watched-address table
706ef7c 
f0fe694 named its table `watched`, and nn_os_sync.c already has a
`watched()` (the mutex watch log): the host did not compile. The table,
its type and helpers are now MwWatched / mw_watched / mw_watched_at_locked
/ mw_watch_add_locked / mw_watch_remove_locked. Checked by a syntax-only
compile of the whole file, not only the test's extracted functions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@klimovich008 @claude
fix(os): keep waking every WaitAny once a watch could not be recorded
7b9f02f 
If growing the watched-address table fails, that holder's address goes
uncounted, so its changes would wake WaitAny only on the 5 ms re-check,
and unlinking it could drop the count of another holder of the same
address. A sticky flag now falls back to the old wake-everything policy
for the rest of the process once any registration fails. The test
injects the allocation failure and checks both cases. (Review by Codex.)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@NotRexed

NotRexed commented Nov 10, 2026

Copy link
Copy Markdown
Owner

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

@NotRexed NotRexed closed this Nov 10, 2026
@NotRexed

NotRexed commented Nov 10, 2026

Copy link
Copy Markdown
Owner

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

@NotRexed NotRexed reopened this Nov 10, 2026
@NotRexed
NotRexed merged commit 4396756 into NotRexed:main Nov 10, 2026
20 checks passed
@klimovich008 klimovich008 mentioned this pull request Nov 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@klimovich008 @NotRexed