WIP: perf(host): HOS_VCORE_FIRST, named threads take a virtual core before any other (off by default) #249
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
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/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!249
Loading…
Reference in a new issue
No description provided.
Delete branch "GummiNaomi/Longshot:perf/vcore-first"
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
In OoT's busy frames, the Presentation Thread waits about 1 ms a frame for a "virtual core" before it can submit the frame to the GPU. The game's six BaseProcExecutors, woken by the same hand-off, have taken all six cores. The game is built for the Switch 2's six fixed cores, and on a PC that wait sits on the frame's critical path. In frames already over budget it also delays the GPU work enough to make the late-pages wait longer: at Lon Lon Ranch, with the late pages' wait as on main, this switch took 54.2 fps to 59.8.
What changed
host/hos_thread.conly):HOS_VCORE_FIRST=<name part>,...(off by default): threads whose name contains one of the parts rank above every guest priority, for the virtual cores only (vc_prio). A waiter asks a holder for its core at once, and no waiter takes theirs.vc_turn): its YieldThread / SleepThread(0) gives its core to a waiter as good by guest priority, and after the time slice such a waiter may ask it for the core. Without this, a first thread waiting by yielding for a job thread pinned to its own core kept that core until the 2 s escape (an audit found it).hos_thread_set_name) and at creation (the boot thread's MainThread,Thread_<entry>). The flag is atomic, because another thread may name it.Flags and settings
HOS_VCORE_FIRSTPresentationfor OoT's Presentation Thread)Presentation). It departs from console scheduling on purpose.Documented in
docs/development.mdbeside the otherHOS_thread switches.Measurements
WIP: the Longshot A/B (main @
2e1e2e8cagainst this PR withHOS_VCORE_FIRST=Presentation) is queued. The numbers below are from Hookshot: the same code, one build, the switch off and on.Baseline:
Hookshot test/game-cpu @ 3434d990, HOS_VCORE_FIRST unsetWhy not main: measured before the move to Longshot (the build carried a TEMP measuring commit that only logs).
Hardware: RTX 4070 SUPER 12 GB, NVIDIA 617.42, i7-14700K, 32 GB, Windows 11 Pro 26200
Scene: Lon Lon Ranch at noon (
ranch-day3), 2560x1440 DLAA, cap 60, unattendedMethod: frames.csv, presents 3480-3960, 3 interleaved runs a side; the critical path from HOS_CHAIN + HOS_SAMPLE_ALL (gc-chain-off / gc-chain-first)
HOS_VCORE_FIRST=PresentationRX_VK_LATE_FIX=0): fps (3 runs)The critical path of a 17.6 ms ranch frame: game code ~4.8 ms, the game's job polls ~2.8, the host scheduler ~1.0, host sync ~0.1, renderer and GPU ~8.8.
HOS_CORES=8changed nothing: the game pins its six executors to cores 0-5 itself.Testing
Tested:
host/hos_thread.c: 12 pass. 2 fail on this PC, and fail on main here too:test_invalidate_vcore,test_sdl_pad_diagnosticsdon't build with this PC's compiler.Not tested:
Needs other testers:
HOS_VCORE_FIRST=Presentationagainst unset. Expect no lower fps and 0vcores: ... escapes(request to open).Screenshots
Nothing on screen.
Risk and rollback
Off by default: nothing changes unless the switch is set. If it's set, an executor asked for its core gives it up at its next checkpoint, which delays its job batch by a few hundred µs (the +0.2 ms median seen at 60 fps). Unset the switch to turn it off.
Reviewer notes
The vcore code is
vc_acquire,vc_yield,vc_prioandvc_turninhost/hos_thread.c. Whether it should become OoT's default is a maintainer question.Issues
None.
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing: the checks readinghos_thread.cran. 2 fail here and fail on main here too (named above). The full run is still to do.third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run (with the Longshot A/B).Thanks. The change is small and easy to follow, and it is well contained. With
HOS_VCORE_FIRSTunset it is the same as main:vc_firststays 0, sovc_prioispriority.vc_turn(w, t)comes down to the oldw->priority <= t->priority. Invc_acquire's slice loop,vc_turn(t, owner)only runs when no holder is worse thant, so there it means the old==. The cost is one plain load per comparison, plus agetenvat each naming. Matching by name is cheap: it happens at naming and at creation, not in the scheduler. The flag is atomic, the same wayserialis. The escape log showingvc_priois a useful touch. It doesn't overlap with #156 or #157 in code. Both are innn_os_sync.c, and 156's hunk inhos_thread.cis insleep_ns, far from here. But 156 changes how a condition-variable signal wakes its waiter, which is the hand-off this PR measures. Whichever lands second should measure again on top of the other.Must change
vc_turnlets a waiter J take a first thread F's core when F yields, or after F's time slice, if J is as good by guest priority. That happens invc_yield(F, 0), thenvc_release, thenvc_grant. But F then goes straight intovc_acquire, and withvc_prio(F) = -1its first loop asks a non-first holder for its core withvc_kick = 1, without waiting for the slice. If J is the worst holder on F's allowed cores, F asks J. That is certain when F's mask is J's one core, which is exactly the case the audit found. J then gives the core back at its next checkpoint, its next nn::os call, SVC or polled loop header, because F is strictly better (vc_yield(J, 1)). So J no longer waits 2 s, but it runs only one checkpoint's worth of work per round trip throughvc_lockand two hand-offs. That is close to a livelock while F waits for J by yielding. When F's mask spans more cores, the same immediate ask lands on some other executor instead. Either way, the line in the description and the docs that "its yields and time slices still follow guest priority" doesn't hold.A fix: after F gives its core up by turn (in
vc_yieldwithanytrue, where the taker isn't strictly better byvc_prio), have F's nextvc_acquirerank by guest priority until it gets a core. For example, avc_turnedflag thatvc_takeclears, and thatvc_prioor the kick loop honours.tests/test_vcore_escape.pyalready compiles the whole block fromvc_locktohos_cv_sleepagainst a single-threaded harness. A sibling check in that style would pin three cases: a first waiter asks a worse holder at once; a first holder hands its core to an equal-priority waiter on a yield and after the slice; and, after case 1, F doesn't take that core back at once. The audit's hazard went into the code without a test, which is how case 1 got through.2e1e2e8cand main is atfbd42024. Nothing conflicts, since main's change is ingpu/vk, but the box is false. The fulltests/run_all.pyand the Linux build are still open. The PR says so itself, so it stays WIP.Should change
RX_VK_LATE_FIX=0, which doesn't exist in Longshot. With the late pages' fix, the comparable setup, it is 59.4 → 59.8 fps from one figure a side: p95 is 0.7 to 0.9 ms better, and the median is about 0.2 ms worse. The ~1 ms core wait comes from a TEMP logging commit that isn't here. The code comment says "~1.2 ms twice in every slow frame pair" and the description says 1.01 ms: pick one. The Longshot A/B the PR already plans should report the vcore line's waits, kicks and escapes. It should also cover a frame-capped scene where the executors are the bottleneck, since that is where taking a core from an executor costs the most. The Deck or a 6c/12t run is worth asking for, and the switch doesn't change the vcore count (6), so the host is more oversubscribed there.vc_betterbreaks the tie by wait time alone, andvc_turn(w, t)is true for any firstw, even one worse by guest priority. To fix it, compare(first, priority, since)invc_better. Matching is a case-sensitive substring, so a broad part names many threads at once.ThreadmatchesMainThreadand every unnamedThread_<entry>. The docs row should say "case-sensitive substring" and warn against broad parts.vc_first_namedsays "no waiter takes theirs", whichvc_turnnow contradicts. Bring it in line with the rule as it ends up after point 1.docs/development.mdrepeats the claim about yields and slices. Fix it with point 1. The nameHOS_VCORE_FIRSTfits the otherHOS_VCORE*switches, and its place in the table is right.Not checked: the Presentation Thread's guest core mask and priority, and whether it waits by yielding. Those come from the game. Point 1's cost depends on them, but the hazard is in the code either way. Nothing was built or run for this review.
@ -264,6 +264,7 @@ Pipeline-cache file ownership, temporary storage and failure behavior are descri| `RX_VK_NAN_GUARD=0` | No NaN guard on float colour outputs (on by default on every card: one NaN texel spreads through bloom into glowing blocks) || `RX_VK_FIFO=1`, `RX_VK_IMMEDIATE=1` | V-Sync On (FIFO) or Off (immediate) for one run, over the menu's V-Sync row || `HOS_VCORES=0`, `HOS_CORES=<1..8>`, `HOS_RESERVE_CORE=0` | No virtual cores; the cores the game sees (6); no core kept for the replay (one is, with 8 or more logical processors) || `HOS_VCORE_FIRST=<name part>,...` | The threads whose name holds one of the parts take a virtual core before any other thread (above every guest priority, for the virtual cores only; their yields and time slices still follow guest priority). For OoT, `Presentation`: its Presentation Thread no longer waits behind the six BaseProcExecutors (off by default) |"their yields and time slices still follow guest priority": not yet, see the comment on
vc_acquire's kick loop. Please also say it is a case-sensitive substring of the thread name.@ -653,1 +657,4 @@/* HOS_VCORE_FIRST=<name part>,... (experiment, off by default): the threads whose name holds one of the parts get avirtual core before any other, as if their priority were above every guest priority (-1; Horizon's are 0-63): awaiter asks a holder for its core at once, and no waiter takes theirs. The guest's own priority (GetThreadPriority)"no waiter takes theirs" no longer holds with
vc_turn. Also: "~1.2 ms twice in every slow frame pair" here, "1.01 ms" in the description. Which is it?@ -654,0 +669,4 @@const size_t n = end ? (size_t)(end - p) : strlen(p);char part[64];snprintf(part, sizeof part, "%.*s", (int)(n < sizeof part ? n : sizeof part - 1), p);if (part[0] && strstr(name, part)) return 1;This is a case-sensitive substring match, so a broad part names many threads:
ThreadmatchesMainThreadand every unnamedThread_<entry>. Worth saying in the docs row.@ -1641,0 +1667,4 @@/* w may take t's core in a turn (a yield of t's, or after the time slice): as good in that order, or, t being a firstthread, as good by guest priority: a first thread comes first, but its yields and slices stay as they were (a firstthread waiting by yielding for a job thread pinned to its own core kept that core until the 2 s escape) */static int vc_turn(const HosThread *w, const HosThread *t) {This makes the yield hand the core over, but the comment's promise that "its yields and slices stay as they were" fails one call later: see the comment on the kick loop in
vc_acquire. Also, when bothwandtare first threads, this is true whatever their guest priorities are, andvc_betterbreaks the tie by wait time alone. Comparing(first, priority, since)would keep the guest order among first threads.@ -1724,3 +1756,3 @@for (int k = 0; k < vc_cores(); k++) {HosThread *h = vc_owner[k];if ((m >> k & 1) && h && h->priority > t->priority && (!worst || h->priority > worst->priority)) worst = h;if ((m >> k & 1) && h && vc_prio(h) > vc_prio(t) && (!worst || vc_prio(h) > vc_prio(worst))) worst = h;A first thread that just gave its core up by turn (
vc_yieldthroughvc_turn) comes straight back here withvc_prio = -1and asks a non-first holder for its core withvc_kick = 1, without waiting for the slice. If that holder is the waiter it just yielded to (always the case when its mask is that one core), the waiter gives the core back at its next checkpoint (vc_yield(J, 1), F being strictly better). So the waiter gets one checkpoint of work per round trip, not its turn. Suggest that F's next acquire ranks by guest priority after a yield by turn (a flag thatvc_takeclears).Code review only; not built or run.
Edited: the Kokiri profile quoted below drew a 5:4 picture, 1800×1440, not 2048×1152 as I first wrote: the PC's display mode had changed that hour. At the usual 16:9 picture the same route runs at about 74 fps. A rerun of the profile there is queued, and I'll update the numbers if they move.
Thanks, it's a small and readable change. HumanBean's review covers the turn rules, the missing check, the order among first threads and the evidence. I have two additions: one behaviour change I didn't see raised, and data from a scene shaped differently from Lon Lon Ranch.
1. A thread better than a first thread by guest priority loses its preemption. In
vc_acquire's first loop, a waiter asks only holders worse than it byvc_prio, and a first thread is -1 to everyone. So a waiter with a better guest priority than the Presentation Thread never asks it for its core at once.HOS_VCORE_SLICE_US, 2 ms) and then gets a turn (kick 2). By HumanBean's point 1, it may then lose the core again at once.The PR departs from console scheduling on purpose, but this side of it isn't in the description: up to 2 ms of extra latency for any higher-priority game thread that shares the Presentation Thread's cores.
HumanBean listed the Presentation Thread's mask and priority as not checked.
HOS_TLS_TRACE=1logs each thread's ideal core, mask and priority when it starts, under itsThread_<entry>name before the game names it. That is enough to see which threads this touches, and whether the BaseProcExecutors share the Presentation Thread's guest priority or are above it. If they share it, the first rank could be applied only against threads at or below the first thread's own guest priority. That would keep the gain and the console's preemptions.2. Data from a main-and-jobs-paced scene. This is from Kokiri Forest on Longshot
1ef93e79(i7-13700K, RTX 5090, Windows, 1800×1440 (5:4), uncapped, about 79 fps), withHOS_CHAINand a sampled profile, the switch unset:ReceiveLightMessageQueue).In a scene paced like this, the switch would move about 0.2 ms a frame of core time from the chain that paces the frame to a thread that would have waited for the main thread anyway. I'd expect no gain there, and possibly a small loss. Lon Lon Ranch's slow frame pairs may well be shaped differently.
The planned A/B would read best with one uncapped, main-and-jobs-paced scene like this beside Lon Lon Ranch. It would also help to report
HOS_CHAIN's per-thread waits along with the vcores line.Nit: the parts aren't trimmed, so
HOS_VCORE_FIRST=Presentation, Mainlooks for " Main" with its leading space and never matches. Either trim the parts, or say "no spaces" in the docs row.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.