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
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 project
No assignees
4 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. By then 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 Hookshot, at Lon Lon Ranch with the late pages' wait as on main, this switch took 54.2 fps to 59.8. On Longshot it hasn't been measured where the gain is claimed yet.
What changed
host/hos_thread.conly):HOS_VCORE_FIRST=<name part>,...(off by default): a thread whose name contains one of the parts is a "first thread". It ranks above every guest priority, for the virtual cores only (vc_prio: its guest priority - 64, so first threads keep their guest order among themselves). A part is a case-sensitive substring of the thread name. Spaces around a part are skipped (vc_first_named).HOS_VCORE_SLICE_US, 2 ms) and then asks (kick 2).vc_turn,vc_yield): on a YieldThread / SleepThread(0), or when a waiter as good by guest priority asks after the time slice, it gives its core to that waiter. Until it takes a core again, it then ranks by its guest priority (vc_turned, set invc_yield, cleared invc_take). So the waiter keeps the core for a time slice instead of being asked for it back at once. 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.hos_thread_set_name) and at creation (the boot thread's MainThread,Thread_<entry>). The flag is atomic, because another thread may name it.tests/test_vcore_first.py, a sibling oftest_vcore_escape.py. It cutsvc_first_namedand the virtual-core block out of the source and compiles them with a single-threaded harness, whose wait hook plays the holder answering. It pins these cases:docs/development.mdrow says how a part matches and what the turn rules are.docs/tested-configurations.mdgets dukenheimer's two Kokiri Forest rows.Flags and settings
HOS_VCORE_FIRSTPresentationfor OoT's Presentation Thread); parts are case-sensitive substrings, comma-separatedPresentation). It departs from console scheduling on purpose.Documented in
docs/development.mdbeside the otherHOS_thread switches.Measurements
This head (
68df5714) is not measured. Since the runs below, a first thread's turns changed: after it gives its core up by turn, it no longer asks for it back at once. That changes timing only when the Presentation Thread yields or uses up a slice with an equal waiter on its cores. The numbers below show the switch at19ad5cd8, and Lon Lon Ranch on Longshot, where the gain is claimed, is still to measure.Longshot (dukenheimer, comment)
Baseline: this PR @
19ad5cd8, unsetWhy not main: the switch is off by default, so one build with the switch off and on isolates it.
this PR: perf/vcore-first @
19ad5cd8(before this rebase), built as3.0.0-rc.3-17-g19ad5cd8; one full build, both arms from the same folderHardware: RTX 5090 32 GB, NVIDIA 621.14; Core i7-13700K (8P + 8E, 24 threads), 98 GB; Windows 11 Pro; display 5120x2160 at 165 Hz
Scene: Kokiri Forest from the autosave, a scripted moving route; 2048x1152 in a 1600x900 window. M1152: no frame generation, no game cap (
HOS_FPS_MAX=240), mailbox. MP165: DLSS with DLSS frame generation 2x, cap 165, FIFO (fps is the game's own frames).Method: 2 interleaved rounds (unset, then
Presentation, at each setting), 125 s a run. Frame times come from the renderer's frame log, 30–110 s after the first frame. CPU comes fromprof: threads in 10 s, and escapes from everyvcores:line.How to reproduce: any save in Kokiri Forest, the settings above,
HOS_VCORE_FIRST=Presentationagainst unset.19ad5cd8, unset19ad5cd8,HOS_VCORE_FIRST=Presentationvcores:escapes19ad5cd8, unset19ad5cd8,HOS_VCORE_FIRST=Presentationvcores:escapesReading: M1152's +1.4 fps comes entirely from round 1, and run-to-run spread is about ±1.5 fps, so this is no change. MP165 is flat. 0 escapes. This scene is paced by the main thread's jobs, on a CPU with no core shortage, so no gain was expected (dukenheimer's review shows the Presentation Thread waits 0.29 ms a frame for a core there).
Hookshot (the author's, before the move to Longshot)
Baseline: Hookshot @ 3434d990, unset
Why not main: measured in Hookshot, before the move to Longshot (the build carried a TEMP 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
HOS_VCORE_FIRST=PresentationRX_VK_LATE_FIX=0, Hookshot only): fpsTesting
Tested:
19ad5cd8(test request T-GZ9B, pass): RTX 5090 / i7-13700K, Windows 11 Pro, Kokiri Forest, 11 runs (warm-ups included). No crash, hang, device loss orVK_ERROR, and 0vcores:escapes. The luma gate showed 0 near-white shots on either side, and flashes stayed within main's usual range.68df5714): the turn fix, the first threads' order and the trimming were read, not run. No tests, checks or builds were run on the PC that made this push.tests/test_vcore_first.pywas written and traced by hand, not run. The CI's Python checks (ci.yml) run only on a manual dispatch, so they haven't run on this head either.Not tested:
HOS_CHAIN), where the gain is claimed.tests/run_all.pyand the new check, here or in CI.Needs other testers:
HOS_VCORE_FIRST=Presentationagainst unset, expecting 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 (with it unset,
vc_turnedis never set andvc_priois the guest priority, as on main). If it's set, an executor asked for its core gives it up at its next checkpoint. That delays its job batch by a few hundred µs (Hookshot's +0.2 ms median at 60 fps). A game thread better by guest priority than a first thread that shares its cores waits up to the time slice (2 ms) for it, where Horizon would preempt at once. A broad part names many threads (ThreadnamesMainThreadand every unnamedThread_<entry>). Unset the switch to turn it off.Reviewer notes
The vcore code is
vc_acquire,vc_yield,vc_prio,vc_turnandvc_first_namedinhost/hos_thread.c. The fix for review 1193's point 1 isvc_turned. ReplayPin and host affinity are unaffected: the virtual cores don't touch host affinity, and the replay runs with no core held.tests/test_vcore_escape.pycompiles the same block. It needed no change (the struct gains one field), but it hasn't been run on this head. Whether the switch should become OoT's default is a maintainer question. dukenheimer's idea of applying the first rank only against threads at or below the first thread's own guest priority would keep the console's preemptions. It is left for a follow-up once the Presentation Thread's guest priority and mask are known (HOS_TLS_TRACE=1).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): not run on this head. Nothing was run on the PC that pushed it, and CI's Python checks (ci.yml) run only on a manual dispatch. Left to that dispatch or the author.19ad5cd8exist so far. Lon Lon Ranch on Longshot, and this head, are unmeasured.third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run (dukenheimer's two Longshot settings; the Hookshot runs predate Longshot).mainand passing here:tests/test_vcore_first.pyadded. It hasn't been run yet (it should fail at 19ad5cd8's turn and order cases, and pass here).🤖 Generated with Claude Code
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: point 2's figures are now from a rerun at the usual 16:9 picture, 2048×1152, on the same PC, scene and settings (M1152) as the A/B below, with the switch unset. The first version quoted a run that drew 5:4 (1800×1440), because the PC's display mode had changed that hour. The figures moved a little, and I've added where the replay meets the main thread (the readbacks); the reading didn't change.
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, 2048×1152, uncapped, about 66 fps with the profiler on), withHOS_CHAINand a sampled profile, the switch unset:ReceiveLightMessageQueue). Its replay reaches the main thread's cycle in two places: the frame sync, which the main thread waits on in about one frame in eight (0.09 ms a frame on average), and the game's readbacks, where job workers wait for GPU work the replay submitted (1.1 ms a frame here).In a scene paced like this, the switch would move about 0.3 ms a frame of core time from the chain that paces the frame to the Presentation Thread, and the replay reaches the main thread only through those two waits. I'd expect no gain there, and possibly a small loss. The A/B below found no change outside run-to-run noise in this scene, which fits. Lon Lon Ranch's slow frame pairs may well be shaped differently.
The A/B below covers such a scene. For Lon Lon Ranch,
HOS_CHAIN's per-thread waits beside the vcores line would show where its slow frames wait.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.Measured on Longshot: RTX 5090 and i7-13700K, Windows 11,
HOS_VCORE_FIRST=Presentationagainst unsetNo regression, and 0 escapes. On this PC and scene the switch doesn't move fps or p99 outside run-to-run noise.
this PR:
19ad5cd8, built as3.0.0-rc.3-17-g19ad5cd8. One build, both arms from the same folder; only the switch differs.Baseline: this PR @
19ad5cd8withHOS_VCORE_FIRSTunset. Why not main: the switch is off by default, and an on/off pair in one build isolates it, as your Hookshot table did.Hardware: RTX 5090 32 GB, NVIDIA 621.14; Core i7-13700K (8P + 8E cores, 24 threads), 98 GB; Windows 11 Pro; display 5120x2160 at 165 Hz
Scene: Kokiri Forest from the autosave, a scripted moving route (running, circling and camera pans from 28 s). Output 2048x1152 in a 1600x900 window, two settings:
HOS_FPS_MAX=240), mailbox: every frame runs as fast as the critical path allows.Method:
nx2/make_game.py, shaders prepared), warm-up runs at both settings.Presentation, at each setting), 125 s a run.prof: threads in 10 slines: every thread at ≥5% of a core, summed over the lines ending at 40–110 s.vcores:line of each run.How to reproduce: any save in Kokiri Forest, the settings above,
HOS_VCORE_FIRST=Presentationagainst unset. The route is our bench's scripted input (HOS_INPUT_SCRIPT), not a repo script.19ad5cd8, unset19ad5cd8,HOS_VCORE_FIRST=Presentationsead::MainThread/ Presentation Thread, coresvcores:escapesPresentationvcores:escapesReading it:
vcores:lines show the same contention in both arms (~250k waits in 5 s, mean 21–24 µs). Your gain was at the ranch, with the late pages' wait as on main and frames over budget at a 60 cap. That case isn't reproduced here. I didn't runHOS_CHAIN, so I can't say how long the Presentation Thread waits for a core on this PC.Checked, not measured:
VK_ERRORline in the 11 runs (warm-ups included).Not tested:
RX_VK_LATE_FIX=0, andHOS_CHAIN.Code review only; nothing built or run. PR 249 at
19ad5cd8, the same head as the earlier review.Nothing has been pushed since the earlier review, so its must-change points still apply. I checked each one again against
19ad5cd8:Must change
vc_yield(F, 0),vc_turnhands F's core to an equal-guest-priority waiter J. F then goes intovc_acquirewithvc_prio(F) = -1. Its first loop finds J as a worse holder and setsvc_kick = 1without waiting for the slice. J's next checkpoint gives the core back (vc_yield(J, 1), since F is strictly better). So the 2 s escape is gone, but J gets one checkpoint of work per round trip. The comment onvc_turnand the docs row both say "yields and time slices still follow guest priority". That isn't true yet.tests/changed. A sibling oftests/test_vcore_escape.pyshould cover three cases: a first waiter asks a worse holder at once; a first holder hands its core over on a yield and after the slice; and after that hand-over, F doesn't take the core straight back.main" is ticked but false. The PR forks at2e1e2e8c, andmainis now16047f55, 12 commits ahead. It merges cleanly (git merge-tree), andmain's changes todocs/development.mdare in other hunks, but the box should be unticked until it's rebased.Should change
vc_betterfalls back to wait time andvc_turn(w, t)is true for any firstw. The comment onvc_first_namedstill says "no waiter takes theirs". It also says "~1.2 ms" where the description says 1.01 ms.main, and add thedocs/tested-configurations.mdrows. Lon Lon Ranch on Longshot, where the gain is claimed, is still unmeasured.19ad5cd8, runs 1786 (Python checks) and 1787 (third-party) are blocked, waiting for approval. Only the description check passed. Therun_all.pybox rightly stays unticked until the checks run, here or locally.The
HOS_VCORE_FIRST-unset path is still the same asmain. That part is fine.@ -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) |Still says "their yields and time slices still follow guest priority". At
19ad5cd8that isn't so: after a yield by turn,vc_acquireasks the new holder for the core back at once (see the earlier review's comment on the kick loop). Reword it once point 1 is fixed, and say how a part matches: a case-sensitive substring of the thread name.@ -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;The parts aren't trimmed, so
HOS_VCORE_FIRST=Presentation, Mainlooks for " Main", leading space included, and never matches it (dukenheimer's nit, still open). Skip spaces around each part, or say "comma-separated, no spaces, case-sensitive substring" in the docs row.19ad5cd861f20959eca2Pushed
f20959ec: rebased ontomain@e65cb1e0(no conflicts), plus one fix commit for review 1193 (and 1143). No tests, checks or builds were run on the PC that pushed it: the change and the new check were read and traced by hand.Per point:
Yields and slices follow guest priority. When a first thread gives its core up by turn (a yield, or the slice's ask from a waiter as good by guest priority),
vc_yieldsetsvc_turned, andvc_priothen ranks it at its guest priority untilvc_takeclears the flag. So its nextvc_acquiredoesn't ask the new holder at once (vc_kick = 1). It asks only after the time slice, and the waiter keeps the core for its slice. With the switch unset,vc_turnedis never set, andvc_yield's scan stops where it did.A check.
tests/test_vcore_first.pyis a sibling oftest_vcore_escape.py: it cutsvc_first_namedand the vcore block out of the source and compiles them with stubs and a wait hook. It pins these cases:It hasn't been run.
Rebased onto
e65cb1e0.Should-change:
vc_priois now the guest priority - 64 for a first thread, so two first threads keep their guest order invc_betterandvc_turn.19ad5cd8): in Measurements, and in twodocs/tested-configurations.mdrows.Still WIP:
run_all.py, with the new check, hasn't run: CI's Python checks need a manual dispatch.What's needed is a run of this head at the ranch (cap 60), with
HOS_VCORE_FIRST=Presentationagainst unset, ≥2 interleaved rounds, thevcores:lines andHOS_CHAIN. A run againstmainate65cb1e0would settle the Measured box.f20959eca268df571467Rebased again onto
main@b7a46d6b, which moved after the last push (an OoT input fix, nothing in these files): the head is now68df5714, with the same changes asf20959ec. The Measurements' baseline lines now name their columns exactly. Still no tests, checks or builds run on the PC that pushed it.Light re-review of the fix commit
68df5714only, against review 1193. I read and traced it by hand and ran nothing.The code points are resolved.
vc_yieldsetsvc_turned, so F waits at 16. F'svc_acquireasks J only after the slice (kick 2), and J's ownvc_yieldhands the core back (vc_turn16 ≤ 16).vc_takeclears the flag on every path that hands out a core: a free core, a grant, or a free core found mid-wait. A holder is never turned. So no thread stays ranked at guest priority, nothing waits that didn't wait before, and no wake-up is lost.vc_turnedis written and read only undervc_lock.any && !better, so a strict kick-1 yield to a better first thread leaves it down, as it should.vc_firstandvc_turnedstay 0, sovc_prioispriorityandvc_turnis<=. The new loop still stops at the first allowed waiter (!(any && !first)), with the sameanyas main's.priority - 64. It maps 0..63 to -64..-1, which is below every guest rank, and first threads keep their guest order invc_better,vc_turnand the worst-holder scan.tests/test_vcore_first.py. All four markers are in 68df5714's source, and they are cut the waytest_vcore_escape.pycuts them.getenvis macro'd after the includes, soHOS_VCORESand the slice fall back to their defaults.RUN_ALL_ARGSmatches its sibling's, sorun_allpicks it up. I traced cases 1–5 and the name cases, and they pass. Case 3 has J still inside its ownvc_acquirewhile it holds the core, but the end state comes out right.Two non-blocking nits are inline.
Still needed before ready:
run_all.py(or CI's dispatch) on this head, the Linux and llvm-mingw builds, and a run of68df5714at Lon Lon Ranch (cap 60) withHOS_VCORE_FIRST=Presentationagainst unset, at least 2 interleaved rounds, with thevcores:lines.@ -1657,6 +1702,7 @@ static void vc_take(HosThread *t, int c) {t->vc_held = c + 1;t->vc_last = c + 1;t->vc_kick = 0;t->vc_turned = 0; /* (a first thread that gave its core up by turn comes first again) */Nit:
vc_turnedisn't cleared onvc_acquire's escape path (line 1793, outside the diff), only here. A first thread whose wait after a turn escapes keeps the flag while it runs without a core. Its next wait, after its next block, then ranks at guest priority, untilvc_take. At->vc_turned = 0;besidet->vc_escaped = 1;keeps "until it takes one" exact. It only matters after a 2 s escape.@ -1791,0 +1840,4 @@/* a first thread that gives its core up by turn (to a waiter no better by vc_prio) asks for one by guest priorityuntil it takes one: otherwise its vc_acquire would ask the waiter for the core back at once (vc_kick 1), and thewaiter would run one checkpoint's work per turn instead of its time slice */if (any && !better && first) t->vc_turned = 1;Nit, only with two or more first threads: whether F gives its core up is decided by J, a waiter as good by guest priority.
vc_grantthen hands the core to the best waiter byvc_prio. That can be a first thread F2 that is worse by guest priority than F (guest 20 → -44, beating J's 16). F2 itself doesn't qualify (vc_turn(F2, F)is false), so F's turn goes to a thread it outranks by guest priority, and F waits at 16 for a slice. It doesn't happen withPresentationalone, so a line in the comment is enough.Superseded by the re-review at #249 (comment): the code points this review asked for are resolved. What's left is runs and boxes, listed there.
Superseded by the re-review at #249 (comment): the code points this review asked for are resolved. What's left is runs and boxes, listed there.
Read head
68df5714against maincf605c85, the complete diff, scheduler and checkpoint callers, all comments and all three earlier reviews; the new test and 11 related checks pass here, with GCC and Clang for all 10 compiled drivers. The Windows log-writer check was skipped, as was the UTF-8 check's Windows executable probe. No game or full build was run.What's good
Earlier review points
Against #249 (comment) and #249 (comment):
vc_turnedlowers the returning thread to guest priority untilvc_take. The new test executes both voluntary yielding and the checkpoint response to a slice request, and passes with both compilers.tests/test_vcore_first.py; it now has an actual passing run. The existing escape and invalidate checks also pass unchanged. Against main's source, the new test stops at the missingvc_first_namedmarker before compiling, so that control is not a behavioral reproduction of the old turn bug.19ad5cd8. The requested current-head ranch comparison, full suite and Linux/llvm-mingw builds remain open. The latest source-check jobs are skipped, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/2878.vc_turnedsurviving an escape are already recorded at #249 (comment) and #249 (comment); I am not raising them again as new findings.Must change
No additional merge-blocking code defect found. The outstanding readiness checks above still need to be completed before this leaves WIP.
Measurement and smaller points
docs/tested-configurations.md, lines 17-18, into separate run files underdocs/tested/. Main now freezes the old table. Give each measured run its own identifier and full context rather thanas above. The real merge check reports a conflict in this file; this is not just the forge's WIP mergeability flag. The rebased checkbox needs updating accordingly.Not checked
No game, full suite, full host build, Windows execution, AMD or lower-thread-count CPU, long-session or visual testing here. I checked the published run figures, not raw frame captures or screenshots. A current-head Lon Lon Ranch comparison at the 60 fps cap, plus a constrained-CPU run, would decide whether the shorter presentation wait outweighs the time taken from the job workers.
View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.