Repository navigation
fix(vk): with per-queue fences, a CPU read-back waits for the GPU work that writes it again - #2125
Merged
Merged
Conversation
… fences too A CPU readback of GPU results (invalidate) looks up the range's write-back value, which is known only once the work that writes it is recorded. With per-queue fences (the default since 8c28ed7) the readback skipped its catch-up, because a catch-up then forced stopped work past its waits. d6c5ddc made the catch-up let on only the met waits, but the readback kept skipping it: work stopped at a wait that was already met (a CPU event signalled, not applied yet) stayed unrecorded, its write-back unknown, and the CPU read the range before it. Now the readback settles first (sync_settle: the met waits only, never forced) and then waits for the write-back on the recordings' timeline, as before. RX_VK_PER_QUEUE_FENCES=0 hid it (issue NotRexed#1005) by waiting for every queue's work: on an RX 5500M in Hyrule Market the game's readbacks then waited 19-87 ms each (0.02-0.19 ms with per-queue fences), 51% of 5-second windows under 8 fps. tests/test_readback_settle.py reads the source: the readback settles before the write-back lookup.
…cyan The comment and the test's docstring named one-frame cyan flashes and the UI thrown about as what the skipped catch-up caused. That was not shown: the runs compared differed in Render resolution and session length too, and the cyan that reproduces (after a pause with the game in the background) comes from VRAM mirrors given back while idle. What is shown stays: the read-back waited for nothing, and RX_VK_PER_QUEUE_FENCES=0's wait for every queue cost 19-87 ms a read-back against ~1 ms with the settle.
elkarbeenus
pushed a commit
to elkarbeenus/LakuenRecompBuilder
that referenced
this pull request
Nov 11, 2026
…ly the first (NotRexed#2419) On AMD integrated GPUs (ROG Ally, Legion Go, Steam Deck) the picture breaks for one frame at a time: the HUD drawn stretched across the screen, grass flat white, the whole screen flashing. The game rewrites memory the GPU is still reading, because its fence waits return after the first part of a submission. ## Cause A submission longer than `RX_VK_CHUNK` draws (default 120), or one stopped at an event wait and finished later, is recorded in several recordings that all carry its value. `submit_recording` maps the value to its recording with `value_rec.emplace`, which keeps the **first** one: when the last part finishes the submission, the key is already there and nothing changes. So `wait_recorded` (the game's fence waits in `wait()`, and the exposure readback wait) returns once the first ~120 draws are done. With no VRAM mirror of the buffer pools (integrated GPUs), the later parts read guest memory in place, and the game has by then written the next frame's uniforms, vertices and exposure meter into it. The HUD is drawn late in the frame, so it gets the wrong transforms most often; the stale exposure readback gives the whole-screen flashes. myylkn found the same root cause on a Radeon 8060S (NotRexed#997, with a patch in a comment). This PR is a variant of that patch: - `find` and an update in place rather than `value_rec[v]`, so it builds against `tests/draw_stream_visibility_test.cpp`, which models `value_rec` as a `std::multimap`. - It keeps registering `r.value` when the recording does not signal (the parts split off before the last), raised to the latest seq. A wait on a stopped submission still finds its recording, instead of falling back to the timeline wait, which forces stopped work past its event waits. ## Change - `gpu/vk/vk_replay.cpp`, `submit_recording`: each value maps to the latest recording that carries it. - `tests/test_value_rec_last_part.py`: runs the statements cut from `submit_recording`, for a submission split in three parts and for a stopped submission finished in a later recording. It fails on `main` (`value 10 -> recording 1 (expected 3)`, `value 11 -> recording 4 (expected 5)`) and passes with the change. - `docs/tested-configurations.md`: the three runs below. ## Tested ROG Ally X (Ryzen Z1 Extreme, RADV PHOENIX, Mesa 26.2.3), Linux (Anatase, Fedora-based), counting one-frame luminance spikes in screen recordings (a frame brighter or darker than both its neighbours): | v2.0.13, Hyrule Field | One-frame spikes | 15/255 or more | `GPU waits in 5 s` | |---|---|---|---| | without the change (69 s) | 104 | 15 | 4–109 sync waits, 31–232 ms | | with the change (42 s) | 3 | 0 (the largest is a sword hit) | 103–137 sync waits, 1.4–1.8 s | Then about a day and a half of play with the change, rebuilt for v2.0.14 and v2.1.0 (the renderer alone, `RX_VK_ONLY=1`): no flashes or stray HUD seen in Hyrule Field, Death Mountain Trail and Goron City, in Desktop Mode and in Steam's Gaming Mode. A 57 s Gaming Mode recording on v2.0.14 had no one-frame spikes at all. **It costs frame rate on this device.** The FPS overlay in the field read roughly 45–70 before and 38–49 after (two recordings, not a controlled benchmark): part of the old frame rate was the game running ahead of the GPU. The waits now last as long as the GPU work they stand for. `python -I -B tests/run_all.py` with the Vulkan headers: the same results on a clean checkout of `main` with and without the change, apart from the new test, which passes. `test_draw_stream_visibility`, `test_capture_slot_ownership` and `test_parallel_rb_refresh`, which compile `submit_recording`, pass. **Not tested:** Windows, NVIDIA and discrete AMD cards (the early wait is there too, but their VRAM mirror is synced at the submit, which should hide it; not measured), `RX_VK_PARALLEL` segments (off by default; the same mapping applies to them), and the Steam Deck. ## Related - Part of NotRexed#997 (AMD handhelds and APUs tracker). - NotRexed#1795 copies uniform buffers on GPUs without mirrors for the same symptoms. The two can coexist: that one keeps a late write from reaching the GPU; this one makes the fence waits cover the work they stand for. An A/B of NotRexed#1795 with this change in and `RX_VK_UB_SNAPSHOT=0` would show how much of its effect was this wait. - NotRexed#2125 (merged) lets stopped work finish before a CPU readback. Complementary: the readback's wait that follows goes through `value_rec`, which this corrects. ## Screenshots Before (v2.0.13, Hyrule Field and Castle Town; each strip is the frame before, the broken frame, the frame after): <img width="1920" height="335" alt="before-1" src="https://github.com/user-attachments/assets/4d3ee32e-8575-462e-b972-b3bb16ec2f3e" /> <img width="1920" height="335" alt="before-2" src="https://github.com/user-attachments/assets/2231f72b-3394-46b0-b67d-647674712a29" /> <img width="1920" height="335" alt="before-3" src="https://github.com/user-attachments/assets/d2d645a7-bf2a-41c3-979a-19b2a36562ff" /> <img width="1920" height="335" alt="before-4" src="https://github.com/user-attachments/assets/366157ae-41e2-48a4-b4cd-7fc96f817eaa" /> After (v2.0.13 with the change, Hyrule Field, fights included; the last strip is the largest one-frame spike left, a sword hit): <img width="1920" height="333" alt="after-1" src="https://github.com/user-attachments/assets/9f025025-1034-47ab-bfe9-0ea1476db075" /> <img width="1920" height="333" alt="after-2-largest-spike-is-a-sword-hit" src="https://github.com/user-attachments/assets/e7e26d16-ba97-4982-bed0-670306352113" /> The analysis and the change were made with AI assistance (Claude); the testing and recordings are mine. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: hckrman101 <239247847+hckrman101@users.noreply.github.com> (cherry picked from commit e0dbf40)
addisonbair
added a commit
to addisonbair/Hookshot
that referenced
this pull request
Nov 12, 2026
…ry 8 command buffers A CPU read of GPU results (invalidate) waits for the recording that writes the range back, after catching the replay up (NotRexed#2125). With batched submits and no idle send (the Steam Deck's APU, the commit before) the work recorded during that catch-up was held until the wait's own flush, so the GPU started it cold and the reader waited for all of it. Now, while a readback waits, what is held goes out when the wait begins and then once 8 command buffers are held, and the GPU works through it during the catch-up. The waits themselves are unchanged. Steam Deck LCD, Kokiri Forest, main at eaef146 with default switches, two passes each: main 13.8 / 13.5 fps, no idle send 12.8 / 12.8, no idle send with this 14.6 / 14.3. P16 (NVIDIA, RX_VK_BATCH=1 RX_VK_BATCH_MAX=32 to imitate RADV): 40.9, 34.8, 38.6, with 15.6 / 2.8 / 5.4 flushes a frame. RX_VK_READBACK_SEND=0: off; RX_VK_READBACK_SEND_CBS: the count (default 8). tests/test_readback_send.py checks it. Assisted-by: Claude Opus 5.5 (Anthropic) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sheepbun
added a commit
to sheepbun/Bunny-Hood
that referenced
this pull request
Nov 12, 2026
… them (NotRexed#2602) Session 6 found the synced build's lost quarter of its frame rate in the game's readbacks of GPU results: upstream NotRexed#2419's wait for a submission's last part and NotRexed#2125's settle under sync_order_mu. A fix written here (kept as backup/readback-by-part-mine) took Kokiri Forest from 39 to 53.5 fps on the i7-11700 and RTX 3080; upstream merged the same fix, more complete, as NotRexed#2602, which this fork takes with its next sync instead. Area: docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NotRexed
pushed a commit
that referenced
this pull request
Nov 12, 2026
…ols the GPU writes (#2154) > **Update (2026-11-11):** a second commit widens the fix to save reloads and logs the mirrors still given back. The pause fix below is tested in game; the second commit is not yet (see "Save reloads"). ## Problem Pause the game, leave it in the background for 1–2 minutes (another window or the Start menu in front), then come back and resume: for a few seconds the picture has cyan patches. On an RX 5500M (RDNA1, 4 GB, Windows 11, AMD driver) this happened on every resume. While the game is paused, most of its memory pools are not bound. After `kIdleEpochs` (600) submissions an unbound pool's VRAM mirror is idle, and `mirrors_give_back` gives it back once it has been idle past `kLongIdle` (6000 submissions) or when a wanted mirror does not fit. In one such pause the running count in the log went from 35 to 51 (`51 mirrors given back (1136 MB in all)`). At the pool's next binding, `mirror_make` makes a new mirror from guest memory (plus the CPU's newer writes from the captures). Most of what the GPU writes into a mirror is copied back to guest memory: a written binding queues a write-back. But some of it is never copied back, by design, and is noted only as a hazard by `memory_note_gpu_write`: what a shader writes through a bare entry's guessed range, the pointer table's range or a wide binding. (Copying the whole guessed range back overwrote the CPU's own data next to it.) That data was in the old mirror alone. The new mirror starts without it, and the GPU goes on from stale data until it has rebuilt it, which is the few seconds of cyan. ## Change - `Pool::gpu_only`: set by `memory_note_gpu_write` on every imported pool its range reaches. - `mirrors_give_back` leaves an idle `gpu_only` pool's mirror alone. Its other idle mirrors go as before. - A pool that takes others in (`pool_lookup`'s merge) keeps their mark: their mirrors' contents come with them. - The give-back's log line also says how many MB of idle mirrors are kept this way. - Second commit: `Pool::gpu_written`, set by `gpu_buffer_bind` for a written binding of the mirror, carried over a merge like `gpu_only`, and the idle give-back keeps those mirrors too. - Second commit: every mirror still given back is logged (the first 200), with why (retired, retired and not inherited in time, idle, or idle for room), how many submissions since its sync, and the pool's marks. A pool that is refused a mirror for the budget meanwhile is read in system memory, which is slower but not wrong. Retired pools are unchanged: their GPU contents already go to the pool that took them in (`inherit_pending`). ## Tested RX 5500M, Windows 11, AMD driver, main `92907f7` with #1581 and #2125, same settings throughout. Each pause was 1–2 minutes with the game in the background: | | cyan after resume | mirrors given back during the run | |---|---|---| | main (`92907f7` + #1581 + #2125) | every resume (tried at Auto), for a few seconds | 16 in one pause | | `RX_VK_MIRROR_KEEP=1` (no give-back at all) | none, one pause at Auto and one at 33% Render resolution | 0 | | this PR | none, one pause at Auto and one at 33% | 48 in the session; 112 MB of idle mirrors kept at the last count | A very short black flash right on resume is still there at both resolutions. At Auto it has been there since the first builds on this card, including the `RX_VK_MIRROR_KEEP=1` run, so it looks separate. ## Save reloads With the first commit, cyan came back after reloading a save within a run (Hyrule Castle, Hyrule Field), in play and over the Start menu, with no pause and no `not imported` pools in the log. (That run also had `RX_VK_FIFO=1`, which turned out not to matter.) Against `RX_VK_MIRROR_KEEP=1`, which never gives a mirror back: | | run 1 | run 2 | |---|---|---| | first commit only | cyan after each reload (castle, field) | — | | `RX_VK_MIRROR_KEEP=1` | three reloads, no cyan over the Start menu | fewer flashes, some on the third reload | So mirrors given back explain most of the cyan after a reload, but not all of it. The first commit keeps only pools the GPU wrote past the write-back. The pools whose mirrors still went are the ones the GPU writes through a normal binding, which the second commit keeps too. **Not tested in game yet:** the second commit. I haven't played a build with it, so I don't know yet whether it matches `RX_VK_MIRROR_KEEP=1` or what it costs in VRAM. Its log line is there to answer both, and to show which mirrors are still given back before any flash that's left. ## Tests `tests/test_mirror_gpu_only.py` (new) reads the source and checks four things: `memory_note_gpu_write` marks the pools it reaches, `gpu_buffer_bind` marks a pool written through its mirror, the idle give-back leaves marked pools, and a merge keeps both marks. It fails on current main and passes with this change. `nx2/test_import_windows.py` builds `pool_lookup` against a `Pool` of its own, which now has both fields. `tests/run_all.py` on main `4d68bc83` with this branch merged: 311 passed. The 6 that fail are the same 6 that fail on plain main on this PC. Related: #1005 (cyan and UI flashes after focus loss), #2125.
sheepbun
added a commit
to sheepbun/Bunny-Hood
that referenced
this pull request
Nov 12, 2026
… them (NotRexed#2602) Session 6 found the synced build's lost quarter of its frame rate in the game's readbacks of GPU results: upstream NotRexed#2419's wait for a submission's last part and NotRexed#2125's settle under sync_order_mu. A fix written here (kept as backup/readback-by-part-mine) took Kokiri Forest from 39 to 53.5 fps on the i7-11700 and RTX 3080; upstream merged the same fix, more complete, as NotRexed#2602, which this fork takes with its next sync instead. Area: docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DFN-master
pushed a commit
to DFN-master/Hookshot
that referenced
this pull request
Nov 12, 2026
…nd no game thread waits behind a present for nothing Hyrule Field lost about 15 fps between v2.0.11-588 (ba86aee) and main (RTX 3090, 2560x1440, uncapped: 79 -> 64-71 fps, CPU 39 -> 64-73 ms a frame). One of the game's job threads was blocked 5.5 ms a frame in a CPU read of GPU results (0.1 ms before), and the game's other job threads polled with 20 us sleeps meanwhile (1.9 cores in sched_yield against 0.3). The read is of the culling results the first part of the scene's submission writes; that submission then stops at an event wait and is finished later, on the presenting thread. Three things held the read up: - since NotRexed#2419 a submission's value maps to its last part's recording, so the read waited for the GPU's whole submission. Each part writes back the pages the GPU wrote in it, at its own end: the page table now keeps the part with the value, and the read waits for the newest part of its pages. A page written in place (no mirror) keeps the wait for its whole submission, whose later parts read and write guest memory itself (issue 997). RX_VK_READBACK_PART=0: the last part, as before. - since NotRexed#2125 the read took sync_order_mu whenever a queue was stopped, behind the presenting thread's whole replay and its present. A queue can be stopped at a met wait with nobody to let it on only after a CPU signal; the read looks at the stopped queues when the CPU has signalled since the last look (2-9 of 3000 readbacks, all of them before). RX_VK_READBACK_SETTLE_ALWAYS=1: whenever anything is stopped, as before. - a wait whose recording was already done, and the host's copy bookkeeping (rx_nvn_replayed, under its own lock), took dev.present_mu to flush held submits, which exist only with batched submits. A present holds that mutex 4 ms a frame. The flushes look at an atomic first; a wait returns at once when the recordings' timeline is already there. Hyrule Field, same session, fix / base / main: 78.6 / 81.3 / 63.1 and 78.7 / 73.9 / 65.9 fps; CPU 46.7 / 38.3 / 71.0 and 45.1 / 40.0 / 69.5 ms a frame. Readbacks that waited: ~1350 of 3000 at 0.14 ms (main 172 at 5.5 ms, base ~1300 at 0.13 ms). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
DFN-master
pushed a commit
to DFN-master/Hookshot
that referenced
this pull request
Nov 12, 2026
…d submit is found only once it is held, and more in-place reads noted mooki000's two findings on the pull request, and the second review's: - A look at the stopped queues said it was done at the look's end, before the burst it ran in had finished its recording (burst_end: the write-backs noted, the recording submitted). A readback in between found the counts equal, took no look, and read the page table without that recording's pages: the hole NotRexed#2125 closed. The count is now said at the burst's end, after the recording is finished (found by mooki000). - A held submit's recording was in value_rec / part_rec before the submit was in the list of held ones. A thread that found it there and waited for it had nothing to flush, and waited for a signal still held until something else sent the batch. The recording is now put in the maps under dev.present_mu, after the submit is in the list and pend_held is up; without batched submits as before. The early raise of pend_held is gone with its window (found by mooki000; main has the same window with the mutex). - In-place reads noted for the readback's part wait: the pool handed out whole (gpu_buffer: a large texture unswizzled on the GPU, a texel buffer's view), and a binding made outside a recording's uploads now counts for the next part. The counters are read and written in order (no relaxed loads). - Not noted, with the reason at note_in_place_read: memory_rb_refresh's copies of the readback pages into their mirrors (in every part's uploads, they are how a CPU write to such a page reaches the mirror; noted, no read would wait for less than its submission), and the shaders' own pointers into a pool with no mirror. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This was referenced Nov 12, 2026
sheepbun
added a commit
to sheepbun/Bunny-Hood
that referenced
this pull request
Nov 12, 2026
… them (NotRexed#2602) Session 6 found the synced build's lost quarter of its frame rate in the game's readbacks of GPU results: upstream NotRexed#2419's wait for a submission's last part and NotRexed#2125's settle under sync_order_mu. A fix written here (kept as backup/readback-by-part-mine) took Kokiri Forest from 39 to 53.5 fps on the i7-11700 and RTX 3080; upstream merged the same fix, more complete, as NotRexed#2602, which this fork takes with its next sync instead. Area: docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it fixes
#296 made every CPU read of GPU results wait for the GPU work that writes them. Without that wait, OoT's exposure flashed for a frame (#296: "OoT exposure flashes").
With per-queue fences on (the default since #523) that guarantee has a hole. When the work that writes the range is stopped at a wait that is already met (for example a CPU event the game signalled, which is applied at the replay's next look at the queues), the read-back finds no pending write-back, waits for nothing, and reads the range as it was: an earlier frame's result.
This PR closes that hole. Before the lookup the read-back lets that stopped work on, never forcing anything, so its write-back is known and the read waits for exactly that work.
Why it's worth merging
RX_VK_PER_QUEUE_FENCES=0, the workaround in [Tracking] Flicker and flashing: brightness pulses, one-frame flashes, doubled image with frame pacing #1005, closes the same hole by waiting for every queue's work: 19–87 ms per read-back on the same card, 12.5 fps against 31.8 with this PR at the same settings.ifininvalidate, using the samesync_settle()the texture branch further down already calls. Nothing changes withRX_VK_PER_QUEUE_FENCES=0,RX_VK_ASYNC=1orRX_VK_SYNC_WAITS=0.tests/test_readback_settle.pyfails on main and passes with this change.What I haven't seen is a visible change I can pin on it alone. It's a correctness fix for a known bug class (#296), not a fix for a reported picture problem.
How the hole works
invalidatelooks up the range's write-back value, which is known only once the work that writes the range is recorded. #523 skipped the read-back's catch-up with per-queue fences on. So work stopped at a wait that was already met stayed unrecorded, its write-back was unknown, and the read went ahead with nothing to wait for.Why the read-back changes again
The read-back in
invalidatehas changed several times, each step fixing the one before:wb_pages_note, noted when that work is recorded), andinvalidatewaits for that value. Before the lookup the replay is caught up, so work handed over can note its write-back first.wait_queue_emptyin the sync replay,sync_settle).!unforced) and waits on the recordings' timeline instead. That skip was written while a catch-up still forced (step 2). It merged 53 minutes after fix(vk): let a catch-up go on only past met waits, not every stopped queue #449 had made the catch-up safe, so the skip outlived its reason.RX_VK_PER_QUEUE_FENCES=0goes back to the old path: a catch-up, then a wait on the shared timeline. That timeline is signalled only as far as no older submission of any queue is stopped, so each read-back also waits for stopped work whose waits are not met, which waits for other threads' submissions: tens of milliseconds.Change
In
invalidate's read-back, with per-queue fences on, settle before the lookup:sync_settle()applies the CPU's pending signals and lets on the stopped work whose waits are met, never forcing anything (the same call the texture branch further down already makes). That work is recorded, its write-back value is known, andwait_recordedwaits for exactly that recording, as before. Stopped work whose waits are not met is not waited for.RX_VK_PER_QUEUE_FENCES=0,RX_VK_ASYNC=1orRX_VK_SYNC_WAITS=0nothing changes: those still callwait_queue_empty()as before.RX_VK_READBACK_NO_CATCHUPstill turns the catch-up off.What it changes, measured
RX 5500M (RDNA1, 4 GB), Windows 11, AMD driver, main
92907f7with #1581. Read-back figures come from thereadbacks of GPU results: N of 3000 waitedlines.RX_VK_PER_QUEUE_FENCES=0On main the waits are tiny because they usually wait for nothing: the work that writes the range has not been recorded yet. With this PR a read-back waits for the GPU to finish the write. On the main thread that is about 0.17 ms per read, roughly 24 ms in each second of play at that run's read rate (about 2.4% of the thread). Of that run's 122 read-back lines (3000 reads each), six averaged more than 5 ms a wait; the highest, 30 ms, was during an area load.
Frame rate against
RX_VK_PER_QUEUE_FENCES=0, the workaround in #1005, at the same settings (Render resolution 50%, Hyrule Market):RX_VK_PER_QUEUE_FENCES=0I did not run main and this PR at the same settings on the same route, so this is no claim that the PR is faster than main.
Tests
tests/test_readback_settle.py(new) reads the source and checks two things: the read-back settles before the write-back lookup, and it no longer skips its catch-up with per-queue fences. It fails on current main and passes with this change.tests/test_recorded_wait_flush.pystill passes.tests/run_all.pyon mainbdfe09cwith this change: 311 passed. The 6 that fail are the same 6 that fail on plain main on this PC. Rebased ontofab8cb3since, with no conflict, and the two tests above pass there. It also merges cleanly with #1905, which changes the same wait's batch flush.Related: #1005, #523, #449, #296.