Send the held batch before waiting for a recording #1276

Merged

Odrannnn merged 1 commit from fix/radv-readback-wait-flush into main

2026-11-09 23:14:28 +00:00

Odrannnn commented

2026-11-09 20:13:59 +00:00

(Migrated from github.com)

With batched submits (RX_VK_BATCH, on by default with RADV), a recording reaches the GPU, with its signal on the recordings' timeline, only when the batch is flushed. wait_recorded did not flush first. So invalidate's readback wait could wait for a signal that was never sent. The game job that was waiting held up the main thread, and the main thread's progress polls are the other thing that flushes the batch. The game stopped presenting while it loaded a save. wait() already flushed before waiting; wait_recorded now does the same.

tests/test_recorded_wait_flush.py checks that wait_recorded flushes before it waits. It fails on current main.

How it was tested

RX 7900 XTX, RADV (Mesa 26.0.8), Linux 7.0, Ryzen 7 7800X3D. Default settings. Each run loaded a Kokiri Forest autosave from the title screen.

Build	Result
`main` (`09e4eb5`)	Loading screen after "Load"; presents stop at 3,720 frames (5 draws/frame) and never resume (300 s run)
`main` + this fix, run 1	Reaches Kokiri Forest, ~2,430 draws/frame, 33 fps; readback waits 1.0 ms
`main` + this fix, run 2	Reaches Kokiri Forest, ~2,430 draws/frame, 37–38 fps; readback waits 1.0 ms

A thread dump of the hang showed a ModuleSystemWorker job inside invalidate → wait_recorded and sead::MainThread waiting on that job, with the batch still holding the recording.

Not covered

One GPU, one scene, Linux only.
Drivers other than RADV batch only with RX_VK_BATCH=1; I did not test them.
#1081 and #1206 may be the same hang. I have not confirmed it.

tests/run_all.py --strict: the new test passes. 26 other tests fail locally only because clang++ and the Python zstandard module are missing on my machine. They fail the same way without this change.

With batched submits (`RX_VK_BATCH`, on by default with RADV), a recording reaches the GPU, with its signal on the recordings' timeline, only when the batch is flushed. `wait_recorded` did not flush first. So `invalidate`'s readback wait could wait for a signal that was never sent. The game job that was waiting held up the main thread, and the main thread's progress polls are the other thing that flushes the batch. The game stopped presenting while it loaded a save. `wait()` already flushed before waiting; `wait_recorded` now does the same. `tests/test_recorded_wait_flush.py` checks that `wait_recorded` flushes before it waits. It fails on current `main`. ## How it was tested RX 7900 XTX, RADV (Mesa 26.0.8), Linux 7.0, Ryzen 7 7800X3D. Default settings. Each run loaded a Kokiri Forest autosave from the title screen. | Build | Result | |---|---| | `main` (09e4eb5) | Loading screen after "Load"; presents stop at 3,720 frames (5 draws/frame) and never resume (300 s run) | | `main` + this fix, run 1 | Reaches Kokiri Forest, ~2,430 draws/frame, 33 fps; readback waits 1.0 ms | | `main` + this fix, run 2 | Reaches Kokiri Forest, ~2,430 draws/frame, 37–38 fps; readback waits 1.0 ms | A thread dump of the hang showed a `ModuleSystemWorker` job inside `invalidate` → `wait_recorded` and `sead::MainThread` waiting on that job, with the batch still holding the recording. ## Not covered - One GPU, one scene, Linux only. - Drivers other than RADV batch only with `RX_VK_BATCH=1`; I did not test them. - #1081 and #1206 may be the same hang. I have not confirmed it. `tests/run_all.py --strict`: the new test passes. 26 other tests fail locally only because `clang++` and the Python `zstandard` module are missing on my machine. They fail the same way without this change.

hckrman101 (Migrated from github.com) approved these changes

2026-11-09 23:07:56 +00:00

hckrman101 (Migrated from github.com) left a comment

Good catch. On main wait_recorded waits on rec_timeline without flushing first. wait() and wait_value() both flush, and queue_flush_locked is what adds pend.rec to the submit's signals, so a recording still held in the batch never signals. That matches the hang.

The flush is safe here. queue_flush_for takes present_mu briefly, returns early when nothing is held, and drops the lock before the wait. wait() already flushes before calling wait_recorded, so that call becomes a no-op, and invalidate's fallback (wait_value) already flushes from the same context. The test is picked up by run_all's directory scan.

Nits: the new comment is a bit long, and the test checks the source text, so a refactor could trip it (its message says so). No CI has run on the branch yet.

Good catch. On main `wait_recorded` waits on `rec_timeline` without flushing first. `wait()` and `wait_value()` both flush, and `queue_flush_locked` is what adds `pend.rec` to the submit's signals, so a recording still held in the batch never signals. That matches the hang. The flush is safe here. `queue_flush_for` takes `present_mu` briefly, returns early when nothing is held, and drops the lock before the wait. `wait()` already flushes before calling `wait_recorded`, so that call becomes a no-op, and `invalidate`'s fallback (`wait_value`) already flushes from the same context. The test is picked up by run_all's directory scan. Nits: the new comment is a bit long, and the test checks the source text, so a refactor could trip it (its message says so). No CI has run on the branch yet.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101