fix(vcores): honour mask 0 and keep the guest's core cache correct #327
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
beanstalk: approved @ 1697603
beanstalk: approved @ 2c5a62e
beanstalk: approved @ 4fd82b7
beanstalk: approved @ 64d6d09
beanstalk: approved @ 6f91fae
beanstalk: approved @ 70a78f9
beanstalk: approved @ 8d361dc
beanstalk: approved @ 9d8356e
beanstalk: approved @ abf2227
beanstalk: approved @ d82f202
beanstalk: approved @ e5f1e40
beanstalk: approved @ ee8ab5f
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!327
Loading…
Reference in a new issue
No description provided.
Delete branch "DuckNintendo/Longshot:fix/vcore-mask0-cache"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Mask 0 is the guest's "no affinity" value, but the host treats it as one core: a mask-0
thread cannot use another core even when the core it started on is busy. This change makes
mask 0 mean every host core, keeps the guest's cached core as the first choice, and rewrites
the guest's TLS core cell whenever the thread is placed elsewhere.
What changed
host/hos_thread.c:hos_thread_affinity()stores mask 0 instead of ignoring it.vc_allowed()treats mask 0 as "every host core" rather than pinning to one.host/hos_thread.c:vc_pref_core()andvc_free_for()prefer the core in the guest's TLS cell, then the thread's last core, then its ideal/lowest core, and request a remask when the cached core is busy.host/hos_thread.c:vc_note_mask_change()andvc_yield()force a move when a new mask excludes the held core, even with no waiter.host/hos_thread.c:vc_take()/vc_sync_tls()write the guest's core cell on every take:core + 1for a 0/single-bit mask, 0 for a multi-bit mask, and on a same-core mask change. The core slot is discovered from the guest's ownAllocateTlsSlotcall, not assumed.tests/test_vcore_mask0.py: new host-only fixture for the above.tests/test_vcore_escape.py: stubs for the new core-cache helpers.Flags and settings
None. No switch, menu row or ini key is added or changed.
Measurements
main:
16047f55this PR: fix/vcore-mask0-cache @
86b51d71Hardware: x86-64 host, Vulkan. These are CPU scheduling counters; no GPU path is involved.
Scene:
HyruleField_Childat a saved spot, default settings, 1280x720, 60 fps cap.Method: main, PR, main, PR; 10 s warm-up and 60 s sampling per run; the same save, settings
and movement pattern. FPS and draws are sampled once a second; the scheduling counters are
cumulative from process start and identical for both. The raw values of the two runs are in
the row labels.
How to reproduce: load the saved spot and hold a run for 60 s. The counter instrumentation
is in the closed testing-only PR #412 (do not merge), which contains this PR's head plus
the counters: #412
It is not part of this PR.
16047f5586b51d71Rebase: the branch was rebased through
0caf0618onto5e072fc8(head96107450). Thenew commits are Vulkan/actor-motion changes; they do not touch the mask-0/TLS paths
measured here (
vc_allowed,vc_pref_core,vc_take,vc_sync_tls). The counters stilldescribe this code, and the scene remains cap-bound.
Main pins a mask-0 thread to one core, so it takes a core other than the guest's cached
core only twice in 60 s and the cell after the take is wrong twice. This PR uses the free
cores: 129 such takes in the median run, and the cell after the take is never wrong because
vc_take()rewrites it. FPS is cap-bound at 60 in both and the draw counts are comparable:this is not a frame-rate win, it is the cache maintenance that lets mask 0 use the other
cores.
Testing
Tested:
python -I -B tests/run_all.py --jobs 8on the rebased head: 497 passed,2 failed, 63 skipped. The failures are
tests/test_module_ids.pyandtests/test_pad_layout.py; both also fail on untouched5e072fc8. The newtests/test_vcore_mask0.pyand the updatedtests/test_vcore_escape.pypass; the touched host files compile with the build's flags.Four Hyrule Field runs (main, PR, main, PR) at the saved spot; no crash; every run stayed
on
HyruleField_Child, and the counters are above. The no-save title-idle (100 s) → NewGame → intro route was also run twice per build with the same counters: both reached
Kokiri Forest; main's mask-0 off-cache takes were 1 and 1, with 1 wrong cell after the
take each; this PR's were 83 and 97, with 0 wrong cells each. The same route was run
twice more per build without the control bridge (
HOS_INPUT_SCRIPT); all four runsreached Kokiri Forest as well.
Not tested: no longer route than the saved spot; no scene where the cached core is
heavily contended; no uncapped frame rate or higher resolution, so whether the extra
mask-0 placements help is not shown. The real
nn::os::Mutex+ second-threadSetMaskcase belongs to the separate lock-held change, not this PR.
Needs other testers:
in Hyrule Field, Kokiri Forest and the Market for 5-10 minutes, including transitions and
the pause menu; look for hangs, crashes or wrong per-core data and compare the frame
rate, expecting no hang or crash and no worse frame rate (T-5HRS). The request is open
but needs the
testing wantedlabel before testers can claim it.Screenshots
Nothing on screen.
Risk and rollback
Mask-0 threads move between cores more than before (129 off-cache takes in the median run
against 2). If the cell sync were wrong, the guest would use per-core data from the wrong
core. The fixture covers the cell values and the scene run never left a wrong cell after a
take. Reverting the commit restores the pinning; there is no flag.
Reviewer notes
Start with
vc_pref_core(),vc_free_for(),vc_take()/vc_sync_tls()andvc_note_mask_change()inhost/hos_thread.c; the fixture names each case. The lock-helddeferral is a separate follow-up and is not part of this PR.
Issues
None.
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).e29caaa75e86b51d7174Hyrule Field measurement added to the description. Two interleaved runs each, 10 s warm-up and 60 s sampling at the same saved spot:
So this is the cache maintenance that lets mask 0 use the other cores, not a frame-rate win. The earlier 90 s/two-core counts were from this PR's own policy, not a main-vs-PR bug count; the description now uses the scene comparison.
86b51d7174f55b29d6ecRebased onto current
main0caf0618; head isf55b29d6. The suite on the rebased head is 497 passed / 2 failed / 63 skipped; both failures (test_module_ids.py,test_pad_layout.py) also fail on untouched0caf0618. The Hyrule Field table was taken at16047f55/86b51d71; the rebase changes thehos_thread_ofcurrent-thread fast path and condition-variable signaling, not the mask-0/TLS paths measured.Counters are in the closed testing-only PR #412 (do not merge): #412
It is #327's head plus the PERF counters and the report call. Closed so it cannot be merged; the diff stays available on the PR page.
WIP: fix(vcores): honour mask 0 and keep the guest's core cache correctto fix(vcores): honour mask 0 and keep the guest's core cache correctAdditional no-save route with the counters: title idle 100 s, then New Game and the intro video, two runs per build.
Same pattern as Hyrule Field: main pins the thread, so its cell stays aligned; the PR uses the free cores and rewrites the cell at every take.
Rebased through
0caf0618onto currentmain5e072fc8; head is96107450. Suite on the rebased head: 497 passed / 2 failed / 63 skipped; both failures (test_module_ids.py,test_pad_layout.py) also fail on untouched5e072fc8. The new main commits are Vulkan/actor-motion changes, not the mask-0/TLS paths.f55b29d6ec96107450acTest request T-5HRS is open for CPU-limited systems: Steam Deck, 4-core x86 or a low-TDP laptop; Hyrule Field, Kokiri Forest and the Market, comparing main with this PR and looking for hangs, crashes, wrong per-core data and frame-rate regressions.
The request cannot be claimed yet: #327 is not from a collaborator and has no
testing wantedlabel. Could a maintainer add that label? The token I have gets 403 when it tries.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.