perf(vk): a draw takes its thread's pass and descriptor writer once, not at every use #198

p0lux · 2026-11-13T13:23:47Z

p0lux commented

2026-11-13 13:23:47 +00:00

Why

In heavy views on a Steam Deck the render thread's CPU time is most of the frame, and the per-draw functions of the Vulkan replay spend part of it asking for the same per-thread object again and again: pass() and dw() are function calls, and in the renderer's shared library each one reads its thread-local pointer through __tls_get_addr on Linux.

What changed

replay: begin_pass, end_pass, draw, draw_hazards and dynamic_state in gpu/vk/vk_draw.cpp take Pass& ps = pass(); once at their top, and bind_descriptors takes DescWriter& wr = dw();, then use the reference. They asked 30, 64, 15, 11 and 4 times for the pass, and three times a write for the writer. Nothing else changes: it is the same per-thread object, made once. build_descriptors already does this for the writer (#41).
checks: four read the text of those functions and follow it: tests/test_pass_hazard.py, tests/test_push_state.py, and the fixtures tests/sampler_admission_test.cpp and tests/recording_admission_test.cpp, which compile bind_descriptors whole and its tail.
docs: a row in docs/tested-configurations.md.

Flags and settings

None.

Measurements

main: 000f19fb (2026-11-13)
this PR: perf/accessors-once-longshot @ 4e359ba2, measured as ffee45f4: the same commit on main 000f19fb, the main the game folder was built from. The branch was rebased onto 0edc1377 since and got the docs row; it is not measured again there. Between the two, vk_draw.cpp changes log lines in fallback_pipelines_ready and dispatch, and #197 adds the occlusion query to draw(), which the rebase keeps whole (it does not touch the pass). 197 lowers how many draws a frame has (8 to 14% on this Deck), so on today's main both columns would be lower; what this PR saves is a draw's cost, which 197 does not change.
Hardware: Steam Deck OLED (AMD Custom APU 0932, RADV VANGOGH, Mesa 26.2.0-devel), 16 GB, SteamOS 3.9.5, kernel 7.2.7-valve2
Scene: a save standing still on a heavy view (about 2,500 draws a frame), and a save standing still in Hyrule Field (about 1,600 draws); headless, handheld profile, Minimum preset, the game's own upscaler, no frame rate cap reached
Method: host and game folder built from main 000f19fb; only longshot_vk.so differs, built alone from each side with the same flags. Runs alternated main, PR, main, PR: 4 each on the heavy view, 2 each in Hyrule Field. Each run starts the game, loads the save by script, waits 3 of the log's 120-present windows, then measures exactly 720 presents: fps over that span, the log's replay time (ms): submit and GPU time, and the render thread's user-space cycles and instructions by perf stat -t on that thread, which do not depend on the frequency the CPU ran at. Each cell is the median of its runs.
How to reproduce: longshot_env.txt with HOS_HANDHELD=1 RX_VK_LOW_SPEC=1 RX_VK_MESH_OPT=1 RX_VK_VRAM_MB=2400 RX_VK_TEX_MAX_MB=128 RX_VK_TEX_MAX_DIM=1024 RX_VK_TEX_SKIP_MB=4 RX_VK_DESC_SLOTGEN=1 HOS_RESERVE_CORE=0 RX_VK_LOAD=create RX_VK_MESH_NOCULL=2c56dc1006942f74, plus HOS_HEADLESS=1; load a save, stand still, and read the log's N presents, X fps; a frame: N draws lines while perf stat -e cycles:u,instructions:u -t <the Presentation thread> counts between two of them 720 presents apart.

	main @ `000f19fb`	this PR @ ffee45f4	Δ
heavy view: render thread, instructions a frame	77.04 M	73.57 M	-4.5%
heavy view: render thread, cycles a frame	88.30 M	86.08 M	-2.5%
heavy view: replay time a frame	31.67 ms	30.45 ms	-1.22 ms
heavy view: fps	23.83	23.35	inside the spread
heavy view: GPU time a frame	19.54 ms	19.92 ms	inside the spread
Hyrule Field: render thread, instructions a frame	47.01 M	44.70 M	-4.9%
Hyrule Field: render thread, cycles a frame	60.65 M	58.85 M	-3.0%
Hyrule Field: replay time a frame	19.95 ms	19.43 ms	-0.52 ms
Hyrule Field: fps	22.58	22.61	none

The frame rate does not move on main, and I do not claim it does. In the heavy view a frame is about 42 ms for 31 ms of replay: the rest is the render thread waiting (the read-backs), and it waits longer when the replay ends sooner. In Hyrule Field the GPU's 24.7 ms a frame is the limit. What this gives is a render thread that does about 4.5% less work a frame.

Where the replay is the limit it shows a little. With a local patch on both sides that reads the two hot 0x3980-byte ranges without their wait (not part of this PR), same host, 3 pairs on the heavy view: 25.83 to 26.02 fps (medians; 25.41 to 25.88 against 25.95 to 26.50), replay 34.68 to 34.09 ms, instructions 77.90 to 73.51 M, cycles 88.39 to 86.58 M.

main, heavy view (4 runs):  fps 23.62 24.29 24.03 23.48 | replay 32.20 31.46 31.49 31.85 ms | cycles 87.97 87.96 88.62 89.89 M | instructions 76.85 76.67 77.22 78.38 M | draws 2490 2472 2492 2542
PR, heavy view (4 runs):    fps 23.29 24.14 23.35 23.34 | replay 30.64 30.57 30.33 30.06 ms | cycles 86.05 86.10 86.17 85.64 M | instructions 73.57 73.30 73.64 73.56 M | draws 2524 2472 2520 2517
main, Hyrule Field (2 runs): fps 22.55 22.60 | replay 20.15 19.74 ms | cycles 61.07 60.23 M | instructions 47.37 46.64 M | draws 1637 1603
PR, Hyrule Field (2 runs):   fps 22.59 22.62 | replay 19.53 19.33 ms | cycles 59.01 58.68 M | instructions 45.24 44.16 M | draws 1648 1601
[lb1] 720 presents in 30.5 s: 23.62 fps; ... 2490 draws, replay 32.20 ms, GPU 19.62 ms | render thread a frame: 87.97 M cycles, 76.85 M instructions
[lp1] 720 presents in 30.9 s: 23.29 fps; ... 2524 draws, replay 30.64 ms, GPU 20.00 ms | render thread a frame: 86.05 M cycles, 73.57 M instructions

Testing

Tested: on the Steam Deck above, Longshot built from main 000f19fb: 22 headless runs across four renderers (main, this PR, and both with the local read-back patch), each loading a save and playing still for about a minute, with no FATAL, device lost or crash line in any log; draws, passes and GPU time a frame match between the sides within the runs' spread. Headless: no picture was looked at on Longshot. The same edit on Hookshot ran with the picture on the Deck for a play session, with nothing wrong seen. python -I -B tests/run_all.py on macOS with the Vulkan headers given (LONGSHOT_VULKAN_HEADERS), run again after the rebase onto 0edc1377: 471 passed, 19 failed, 36 skipped. 18 of the 19 fail the same way on main on that machine, for reasons of the machine (no /proc, no posix_fallocate, the Linux launch scripts): test_appimage_source_headers, test_bake_portability, test_collect_wide, test_control, test_display_pointer, test_gpu_hang_watch, test_longshot_launch, test_low_memory_notice, test_mem_gate, test_mem_gate_children, test_mirror_trim, test_pcache_header_read, test_pipeline_cache_file, test_pipeline_cache_save, test_pipeline_play_cache, test_prof_symbols, test_program_cache_cleanup, test_program_cache_one_variant. The 19th is test_texture_staging_publication, which fails on main since the log levels came in (#195, #202 and #215 mend it). The checks that read the changed functions pass, test_vk_occlusion_queries.py among them.

Not tested: the picture on Longshot (the runs were headless); the suite on Linux; a Windows build and run; other GPUs.

Needs other testers:

Windows, any GPU: the PR builds with llvm-mingw and the game plays as main does, expecting the same picture and no new log line (request to open)

Screenshots

Nothing on screen.

Risk and rollback

No behaviour change is intended: each function reads the same per-thread object, through a reference taken at its top instead of a call at every use. A mistake would show as wrong pass state (draws missing, a pass left open) on every platform at once. Reverting the commit is enough.

Reviewer notes

The reference is safe because TLS_OBJECT makes a thread's object once and frees it when the thread ends: draw_begin_recording's pass() = Pass{} and build_descriptors' out.reset() both reset in place. The comment at each of the six sites now says so, so a later change that makes pass() per recording trips over it there.

draw() still calls dw() three times (same_desc, the fallback test, FC_WRITES), on purpose: they are on the fallback and profiling paths, and test_mesh_fallback.py matches dw().prog == &p.

texture_view_of's handle_hits() calls are left as they are, so this does not collide with the flat per-draw caches being ported. This is meant to add to a static TLS model for the renderer, not to replace it: static TLS makes each thread-local read cheap, this removes most of the calls.

Issues

None.

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A row in docs/tested-configurations.md for each measured run.
A check under tests/ that reads changed source updated in this PR, not weakened.
Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. Linux: built on the Deck (clang 23, the AppImage's tools). Windows: not built on Longshot yet; the same edit built with llvm-mingw in Hookshot's checks on 2026-11-13.

## Why In heavy views on a Steam Deck the render thread's CPU time is most of the frame, and the per-draw functions of the Vulkan replay spend part of it asking for the same per-thread object again and again: `pass()` and `dw()` are function calls, and in the renderer's shared library each one reads its thread-local pointer through `__tls_get_addr` on Linux. ## What changed - replay: `begin_pass`, `end_pass`, `draw`, `draw_hazards` and `dynamic_state` in `gpu/vk/vk_draw.cpp` take `Pass& ps = pass();` once at their top, and `bind_descriptors` takes `DescWriter& wr = dw();`, then use the reference. They asked 30, 64, 15, 11 and 4 times for the pass, and three times a write for the writer. Nothing else changes: it is the same per-thread object, made once. `build_descriptors` already does this for the writer (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/41). - checks: four read the text of those functions and follow it: `tests/test_pass_hazard.py`, `tests/test_push_state.py`, and the fixtures `tests/sampler_admission_test.cpp` and `tests/recording_admission_test.cpp`, which compile `bind_descriptors` whole and its tail. - docs: a row in `docs/tested-configurations.md`. ## Flags and settings None. ## Measurements main: 000f19fb (2026-11-13) this PR: perf/accessors-once-longshot @ 4e359ba2, measured as ffee45f4: the same commit on main 000f19fb, the main the game folder was built from. The branch was rebased onto 0edc1377 since and got the docs row; it is not measured again there. Between the two, `vk_draw.cpp` changes log lines in `fallback_pipelines_ready` and `dispatch`, and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197 adds the occlusion query to `draw()`, which the rebase keeps whole (it does not touch the pass). 197 lowers how many draws a frame has (8 to 14% on this Deck), so on today's main both columns would be lower; what this PR saves is a draw's cost, which 197 does not change. Hardware: Steam Deck OLED (AMD Custom APU 0932, RADV VANGOGH, Mesa 26.2.0-devel), 16 GB, SteamOS 3.9.5, kernel 7.2.7-valve2 Scene: a save standing still on a heavy view (about 2,500 draws a frame), and a save standing still in Hyrule Field (about 1,600 draws); headless, handheld profile, Minimum preset, the game's own upscaler, no frame rate cap reached Method: host and game folder built from main 000f19fb; only `longshot_vk.so` differs, built alone from each side with the same flags. Runs alternated main, PR, main, PR: 4 each on the heavy view, 2 each in Hyrule Field. Each run starts the game, loads the save by script, waits 3 of the log's 120-present windows, then measures exactly 720 presents: fps over that span, the log's `replay time (ms): submit` and `GPU time`, and the render thread's user-space cycles and instructions by `perf stat -t` on that thread, which do not depend on the frequency the CPU ran at. Each cell is the median of its runs. How to reproduce: `longshot_env.txt` with `HOS_HANDHELD=1 RX_VK_LOW_SPEC=1 RX_VK_MESH_OPT=1 RX_VK_VRAM_MB=2400 RX_VK_TEX_MAX_MB=128 RX_VK_TEX_MAX_DIM=1024 RX_VK_TEX_SKIP_MB=4 RX_VK_DESC_SLOTGEN=1 HOS_RESERVE_CORE=0 RX_VK_LOAD=create RX_VK_MESH_NOCULL=2c56dc1006942f74`, plus `HOS_HEADLESS=1`; load a save, stand still, and read the log's `N presents, X fps; a frame: N draws` lines while `perf stat -e cycles:u,instructions:u -t <the Presentation thread>` counts between two of them 720 presents apart. | | main @ 000f19fb | this PR @ ffee45f4 | Δ | |---|---|---|---| | heavy view: render thread, instructions a frame | 77.04 M | 73.57 M | -4.5% | | heavy view: render thread, cycles a frame | 88.30 M | 86.08 M | -2.5% | | heavy view: replay time a frame | 31.67 ms | 30.45 ms | -1.22 ms | | heavy view: fps | 23.83 | 23.35 | inside the spread | | heavy view: GPU time a frame | 19.54 ms | 19.92 ms | inside the spread | | Hyrule Field: render thread, instructions a frame | 47.01 M | 44.70 M | -4.9% | | Hyrule Field: render thread, cycles a frame | 60.65 M | 58.85 M | -3.0% | | Hyrule Field: replay time a frame | 19.95 ms | 19.43 ms | -0.52 ms | | Hyrule Field: fps | 22.58 | 22.61 | none | The frame rate does not move on main, and I do not claim it does. In the heavy view a frame is about 42 ms for 31 ms of replay: the rest is the render thread waiting (the read-backs), and it waits longer when the replay ends sooner. In Hyrule Field the GPU's 24.7 ms a frame is the limit. What this gives is a render thread that does about 4.5% less work a frame. Where the replay is the limit it shows a little. With a local patch on both sides that reads the two hot 0x3980-byte ranges without their wait (not part of this PR), same host, 3 pairs on the heavy view: 25.83 to 26.02 fps (medians; 25.41 to 25.88 against 25.95 to 26.50), replay 34.68 to 34.09 ms, instructions 77.90 to 73.51 M, cycles 88.39 to 86.58 M. ``` main, heavy view (4 runs): fps 23.62 24.29 24.03 23.48 | replay 32.20 31.46 31.49 31.85 ms | cycles 87.97 87.96 88.62 89.89 M | instructions 76.85 76.67 77.22 78.38 M | draws 2490 2472 2492 2542 PR, heavy view (4 runs): fps 23.29 24.14 23.35 23.34 | replay 30.64 30.57 30.33 30.06 ms | cycles 86.05 86.10 86.17 85.64 M | instructions 73.57 73.30 73.64 73.56 M | draws 2524 2472 2520 2517 main, Hyrule Field (2 runs): fps 22.55 22.60 | replay 20.15 19.74 ms | cycles 61.07 60.23 M | instructions 47.37 46.64 M | draws 1637 1603 PR, Hyrule Field (2 runs): fps 22.59 22.62 | replay 19.53 19.33 ms | cycles 59.01 58.68 M | instructions 45.24 44.16 M | draws 1648 1601 [lb1] 720 presents in 30.5 s: 23.62 fps; ... 2490 draws, replay 32.20 ms, GPU 19.62 ms | render thread a frame: 87.97 M cycles, 76.85 M instructions [lp1] 720 presents in 30.9 s: 23.29 fps; ... 2524 draws, replay 30.64 ms, GPU 20.00 ms | render thread a frame: 86.05 M cycles, 73.57 M instructions ``` ## Testing **Tested:** on the Steam Deck above, Longshot built from main 000f19fb: 22 headless runs across four renderers (main, this PR, and both with the local read-back patch), each loading a save and playing still for about a minute, with no `FATAL`, `device lost` or crash line in any log; draws, passes and GPU time a frame match between the sides within the runs' spread. Headless: no picture was looked at on Longshot. The same edit on Hookshot ran with the picture on the Deck for a play session, with nothing wrong seen. `python -I -B tests/run_all.py` on macOS with the Vulkan headers given (`LONGSHOT_VULKAN_HEADERS`), run again after the rebase onto 0edc1377: 471 passed, 19 failed, 36 skipped. 18 of the 19 fail the same way on main on that machine, for reasons of the machine (no `/proc`, no `posix_fallocate`, the Linux launch scripts): `test_appimage_source_headers`, `test_bake_portability`, `test_collect_wide`, `test_control`, `test_display_pointer`, `test_gpu_hang_watch`, `test_longshot_launch`, `test_low_memory_notice`, `test_mem_gate`, `test_mem_gate_children`, `test_mirror_trim`, `test_pcache_header_read`, `test_pipeline_cache_file`, `test_pipeline_cache_save`, `test_pipeline_play_cache`, `test_prof_symbols`, `test_program_cache_cleanup`, `test_program_cache_one_variant`. The 19th is `test_texture_staging_publication`, which fails on main since the log levels came in (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/195, https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/202 and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/215 mend it). The checks that read the changed functions pass, `test_vk_occlusion_queries.py` among them. **Not tested:** the picture on Longshot (the runs were headless); the suite on Linux; a Windows build and run; other GPUs. **Needs other testers:** - Windows, any GPU: the PR builds with llvm-mingw and the game plays as main does, expecting the same picture and no new log line (request to open) ## Screenshots Nothing on screen. ## Risk and rollback No behaviour change is intended: each function reads the same per-thread object, through a reference taken at its top instead of a call at every use. A mistake would show as wrong pass state (draws missing, a pass left open) on every platform at once. Reverting the commit is enough. ## Reviewer notes The reference is safe because `TLS_OBJECT` makes a thread's object once and frees it when the thread ends: `draw_begin_recording`'s `pass() = Pass{}` and `build_descriptors`' `out.reset()` both reset in place. The comment at each of the six sites now says so, so a later change that makes `pass()` per recording trips over it there. `draw()` still calls `dw()` three times (`same_desc`, the fallback test, `FC_WRITES`), on purpose: they are on the fallback and profiling paths, and `test_mesh_fallback.py` matches `dw().prog == &p`. `texture_view_of`'s `handle_hits()` calls are left as they are, so this does not collide with the flat per-draw caches being ported. This is meant to add to a static TLS model for the renderer, not to replace it: static TLS makes each thread-local read cheap, this removes most of the calls. ## Issues None. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A row in `docs/tested-configurations.md` for each measured run. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. Linux: built on the Deck (clang 23, the AppImage's tools). Windows: not built on Longshot yet; the same edit built with llvm-mingw in Hookshot's checks on 2026-11-13.

p0lux added 1 commit

2026-11-13 13:23:48 +00:00

perf(vk): a draw takes its thread's pass and descriptor writer once, not at every use

PR description / Description follows the template (pull_request_target) Successful in 16s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

838d7877d7

pass() and dw() are calls, and in a shared object each one reads its thread-local pointer through __tls_get_addr.
begin_pass asked for the pass 30 times, end_pass 64, draw 15, draw_hazards 11, dynamic_state 4, and bind_descriptors
asked for the writer three times a write. Each of these functions now takes the reference once at its top, as
build_descriptors already does for the writer. Nothing else changes.

Assisted-by: Claude Fable 5.2
Co-Authored-By: Claude Fable 5.2 <[email protected]>

p0lux added 1 commit

2026-11-13 14:47:33 +00:00

docs: a Steam Deck run for the accessors taken once

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

1d99299687

Assisted-by: Claude Fable 5.2
Co-Authored-By: Claude Fable 5.2 <[email protected]>

p0lux force-pushed perf/accessors-once-longshot from 1d99299687

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

to e627d96bff

PR description / Description follows the template (pull_request_target) Successful in 6s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

2026-11-13 14:49:28 +00:00

Compare

p0lux commented

2026-11-13 14:49:34 +00:00

Measured on Longshot on the Steam Deck, and rebased onto main dd02c6cc. Host and game folder built from main 000f19fb; the renderer built alone from main and from this change on it, runs alternated. Heavy view (4 runs each, medians): the render thread does 77.04 to 73.57 M instructions and 88.30 to 86.08 M cycles a frame, replay 31.67 to 30.45 ms; fps 23.83 to 23.35, inside the spread, since on main that frame waits on read-backs. Hyrule Field (2 runs each): 47.01 to 44.70 M instructions, fps unchanged (the GPU is the limit). A row for the run is in docs/tested-configurations.md. Still open before ready: a Windows build and run.

Measured on Longshot on the Steam Deck, and rebased onto main dd02c6cc. Host and game folder built from main 000f19fb; the renderer built alone from main and from this change on it, runs alternated. Heavy view (4 runs each, medians): the render thread does 77.04 to 73.57 M instructions and 88.30 to 86.08 M cycles a frame, replay 31.67 to 30.45 ms; fps 23.83 to 23.35, inside the spread, since on main that frame waits on read-backs. Hyrule Field (2 runs each): 47.01 to 44.70 M instructions, fps unchanged (the GPU is the limit). A row for the run is in docs/tested-configurations.md. Still open before ready: a Windows build and run.

p0lux referenced this pull request

2026-11-13 16:12:42 +00:00

fix(vk): give the game's occlusion reports the GPU's counts, so it culls what is hidden #197

HumanBean requested changes

2026-11-13 16:19:10 +00:00

HumanBean left a comment

The change itself is sound. I read it as a review of the code only: I didn't build it or run the checks.

What's good

It is purely mechanical. Every pass() and dw() in the six functions maps one to one onto ps or wr. No local ps clashes: bind_descriptors' Pushed& ps is untouched and the writer there is wr. No callee can leave the reference stale or dangling. TLS_OBJECT makes each thread's object once, on first use, through tls_make, and tls_owned()'s ~List frees it only when the thread ends. Nothing else writes pass_tls or dw_tls. The only reset is draw_begin_recording's pass() = Pass{}, which assigns into the same object, so the reference sees it exactly as a fresh pass() call would. The things that do change mid-draw are cur (the recording slot) and the descriptor pools in bind_descriptors (cur->pool, cur->extra), and this PR doesn't cache those. The same holds for pass splits (end_pass/begin_pass inside draw_hazards/draw), a nested end_pass from the dump and probe paths (open is already false), and the recursive bind_descriptors after same < 0.
The checks follow the text without being weakened. test_pass_hazard.py's negative check now refuses both spellings. The two fixtures name the types the cut code now uses (DescWriter, and Writes& wr above the DESCRIPTORS slice).
The evidence is good. The runs were interleaved on the Deck with perf stat on the render thread. On the heavy view the per-run ranges don't overlap: instructions 76.67–78.38 M against 73.30–73.64 M, cycles 87.96–89.89 M against 85.64–86.17 M, replay 31.46–32.20 ms against 30.06–30.64 ms. The description says plainly that fps doesn't move and why. Measuring on 000f19fb holds: between it and dd02c6cc, vk_draw.cpp only changes log lines in fallback_pipelines_ready and dispatch.
On conflicts with main: the build_descriptors rewrite to DescWriter& out = dw(); is already in this PR's base (it came with #41, as the description says). #147 is the dev-packs script and doesn't touch vk_draw.cpp, so nothing conflicts there.
No overlap with #231. Its vk_draw.cpp hunks are handle_hits, graphics_pipeline's mine and RecordingRes::c1_at, all far from these functions, and the two merge without conflict. It edits test_recording_admission.py, while this PR edits the fixture recording_admission_test.cpp. The two are complementary. 231's Deck numbers come from a combined build "among others", so if this change was in that build, don't count its gain twice.

Must change

Not rebased on current main, and it conflicts. #197 (cd906265) landed after this PR's base dd02c6cc. It puts the OccGuard occlusion query in draw() between begin_pass(cmd, at); } and probe_draw(s, p, pass().draws);, the line this PR changes to ps.draws. A three-way merge conflicts there; Forgejo reports mergeable: false. The fix is trivial: keep 197's block and use ps.draws. OccGuard doesn't touch the pass, so ps taken at the top stays right. Until then, "Rebased on current main" is ticked but false. Untick it, or rebase and run the checks that read draw() again.

Should change

Say in the comment why the reference is safe, not only why it's faster: one object for the thread's life, which draw_begin_recording resets in place. That way nobody later replaces pass() with something per recording or per slot without seeing this.
Small: draw() still calls dw() three times (same_desc, the fallback test, FC_WRITES). That's fine, since test_mesh_fallback.py matches dw().prog == &p, but the description could say they are left on purpose.
The Windows build (llvm-mingw) is still open, as the description already says. Nothing in the diff is platform-specific, so I expect no surprise there.

The change itself is sound. I read it as a review of the code only: I didn't build it or run the checks. **What's good** - It is purely mechanical. Every `pass()` and `dw()` in the six functions maps one to one onto `ps` or `wr`. No local `ps` clashes: `bind_descriptors`' `Pushed& ps` is untouched and the writer there is `wr`. No callee can leave the reference stale or dangling. `TLS_OBJECT` makes each thread's object once, on first use, through `tls_make`, and `tls_owned()`'s `~List` frees it only when the thread ends. Nothing else writes `pass_tls` or `dw_tls`. The only reset is `draw_begin_recording`'s `pass() = Pass{}`, which assigns into the same object, so the reference sees it exactly as a fresh `pass()` call would. The things that do change mid-draw are `cur` (the recording slot) and the descriptor pools in `bind_descriptors` (`cur->pool`, `cur->extra`), and this PR doesn't cache those. The same holds for pass splits (`end_pass`/`begin_pass` inside `draw_hazards`/`draw`), a nested `end_pass` from the dump and probe paths (`open` is already false), and the recursive `bind_descriptors` after `same < 0`. - The checks follow the text without being weakened. `test_pass_hazard.py`'s negative check now refuses both spellings. The two fixtures name the types the cut code now uses (`DescWriter`, and `Writes& wr` above the `DESCRIPTORS` slice). - The evidence is good. The runs were interleaved on the Deck with `perf stat` on the render thread. On the heavy view the per-run ranges don't overlap: instructions 76.67–78.38 M against 73.30–73.64 M, cycles 87.96–89.89 M against 85.64–86.17 M, replay 31.46–32.20 ms against 30.06–30.64 ms. The description says plainly that fps doesn't move and why. Measuring on `000f19fb` holds: between it and `dd02c6cc`, `vk_draw.cpp` only changes log lines in `fallback_pipelines_ready` and `dispatch`. - On conflicts with main: the `build_descriptors` rewrite to `DescWriter& out = dw();` is already in this PR's base (it came with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/41, as the description says). https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/147 is the dev-packs script and doesn't touch `vk_draw.cpp`, so nothing conflicts there. - No overlap with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/231. Its `vk_draw.cpp` hunks are `handle_hits`, `graphics_pipeline`'s `mine` and `RecordingRes::c1_at`, all far from these functions, and the two merge without conflict. It edits `test_recording_admission.py`, while this PR edits the fixture `recording_admission_test.cpp`. The two are complementary. 231's Deck numbers come from a combined build "among others", so if this change was in that build, don't count its gain twice. **Must change** - **Not rebased on current main, and it conflicts.** https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197 (`cd906265`) landed after this PR's base `dd02c6cc`. It puts the `OccGuard` occlusion query in `draw()` between `begin_pass(cmd, at); }` and `probe_draw(s, p, pass().draws);`, the line this PR changes to `ps.draws`. A three-way merge conflicts there; Forgejo reports `mergeable: false`. The fix is trivial: keep 197's block and use `ps.draws`. `OccGuard` doesn't touch the pass, so `ps` taken at the top stays right. Until then, "Rebased on current main" is ticked but false. Untick it, or rebase and run the checks that read `draw()` again. **Should change** - Say in the comment why the reference is safe, not only why it's faster: one object for the thread's life, which `draw_begin_recording` resets in place. That way nobody later replaces `pass()` with something per recording or per slot without seeing this. - Small: `draw()` still calls `dw()` three times (`same_desc`, the fallback test, `FC_WRITES`). That's fine, since `test_mesh_fallback.py` matches `dw().prog == &p`, but the description could say they are left on purpose. - The Windows build (llvm-mingw) is still open, as the description already says. Nothing in the diff is platform-specific, so I expect no surprise there.

gpu/vk/vk_draw.cpp Outdated

					
				@ -6717,6 +6722,7 @@ static bool fallback_list_held(uint64_t list) {

				}

				void draw(VkCommandBuffer cmd, NvnState& s, const DrawArgs& d) {

				    Pass& ps = pass();   // (once: the accessor is a call and a thread-local read at each use)

I checked this is safe. TLS_OBJECT makes the thread's Pass once and frees it only when the thread ends, and draw_begin_recording's pass() = Pass{} assigns in place. So no split, slot change or recording restart inside draw() can leave ps stale. Could the comment say that too, e.g. (the same object for the thread's life: draw_begin_recording resets it in place)? Then a later change that makes pass() per recording would trip over it here.

I checked this is safe. `TLS_OBJECT` makes the thread's `Pass` once and frees it only when the thread ends, and `draw_begin_recording`'s `pass() = Pass{}` assigns in place. So no split, slot change or recording restart inside `draw()` can leave `ps` stale. Could the comment say that too, e.g. `(the same object for the thread's life: draw_begin_recording resets it in place)`? Then a later change that makes `pass()` per recording would trip over it here.

p0lux marked this conversation as resolved

gpu/vk/vk_draw.cpp Outdated

					
				@ -7108,3 +7114,3 @@

				        begin_pass(cmd, at);

				    }

				    probe_draw(s, p, pass().draws);   // (longshot: RX_VK_PROBE)

				    probe_draw(s, p, ps.draws);   // (longshot: RX_VK_PROBE)

This conflicts with main since #197 (cd906265). It inserts the OccGuard block, with vkCmdBeginQuery and the guard ending it, between begin_pass(cmd, at); } and this line, so a three-way merge conflicts here. To resolve it, keep 197's block and then probe_draw(s, p, ps.draws);. OccGuard doesn't touch the pass, so ps from the top of draw() is still the right object.

This conflicts with main since https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197 (`cd906265`). It inserts the `OccGuard` block, with `vkCmdBeginQuery` and the guard ending it, between `begin_pass(cmd, at); }` and this line, so a three-way merge conflicts here. To resolve it, keep 197's block and then `probe_draw(s, p, ps.draws);`. `OccGuard` doesn't touch the pass, so `ps` from the top of `draw()` is still the right object.

p0lux marked this conversation as resolved

p0lux force-pushed perf/accessors-once-longshot from e627d96bff

PR description / Description follows the template (pull_request_target) Successful in 6s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

to 4e359ba2f1

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 19s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

2026-11-13 16:25:13 +00:00

Compare

p0lux commented

2026-11-13 16:25:43 +00:00

After the review: rebased onto main 0edc1377. The conflict in draw() with #197 is resolved as proposed: 197's OccGuard block kept whole, then probe_draw(s, p, ps.draws). The comment at each of the six sites now says why the reference is safe (the same object for the thread's life; draw_begin_recording, and build_descriptors for the writer, reset it in place). The description says the three dw() calls left in draw() are left on purpose, and why the numbers measured on 000f19fb still describe the change. The suite ran again on the rebased branch: 471 passed, 19 failed, 36 skipped, the 19 being main's on this machine; the checks that read draw() pass, test_vk_occlusion_queries.py among them. Not built or run in the game since the rebase. Still open: the Windows build. On 231: the Deck numbers here are main against this PR alone, with nothing of 231 in either build.

After the review: rebased onto main 0edc1377. The conflict in draw() with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197 is resolved as proposed: 197's OccGuard block kept whole, then probe_draw(s, p, ps.draws). The comment at each of the six sites now says why the reference is safe (the same object for the thread's life; draw_begin_recording, and build_descriptors for the writer, reset it in place). The description says the three dw() calls left in draw() are left on purpose, and why the numbers measured on 000f19fb still describe the change. The suite ran again on the rebased branch: 471 passed, 19 failed, 36 skipped, the 19 being main's on this machine; the checks that read draw() pass, test_vk_occlusion_queries.py among them. Not built or run in the game since the rebase. Still open: the Windows build. On 231: the Deck numbers here are main against this PR alone, with nothing of 231 in either build.

p0lux changed title from ~~WIP: perf(vk): a draw takes its thread's pass and descriptor writer once, not at every use~~ to perf(vk): a draw takes its thread's pass and descriptor writer once, not at every use

2026-11-13 17:04:19 +00:00

HumanBean added the

platform/steam-deck

label

2026-11-13 17:11:04 +00:00

HumanBean merged commit fbd420241a into main

2026-11-13 17:12:19 +00:00

HumanBean referenced this pull request from a commit

2026-11-13 17:12:20 +00:00

perf(vk): a draw takes its thread's pass and descriptor writer once, not at every use (#198)

HumanBean deleted branch perf/accessors-once-longshot

2026-11-13 17:12:21 +00:00