WIP: perf(vk): flat per-draw caches, inline op payload copies and a flat c1 map in the replay #231

Draft
dukenheimer wants to merge 8 commits from dukenheimer/Longshot:opus/replay-flat-caches into main
First-time contributor

Why

Each draw the Vulkan replay records on the game's render thread spent a good part of its time in hash-map lookups, a C library memmove per replayed op, and a heap allocation per kept c1 upload. This makes those per-draw costs cheaper, with the same results. Measured: +3.4 fps on an RTX 5090 at 5120x2160 (MP165), none at 2048x1152 (M1152), and none on the Steam Deck, whose frame is set by readback and GPU waits, not by the replay (below).

What changed

  • vk: gpu/vk/flat_map.h (new): FlatMap<K, V>, an integer-keyed table in one array (open addressing, linear probing, at most 3/4 full, slot from a multiplicative hash; find/end, operator[], size, clear; a clear gives back an array over 4096 slots). Ported from Corentin Bonjour's https://github.com/NotRexed/Hookshot/pull/2774 (its two commits, with authorship kept).
  • vk: three per-draw thread caches use it: handle_hits (texture_view_of, gpu/vk/vk_draw.cpp), graphics_pipeline's mine (vk_draw.cpp) and picks (program_for, gpu/vk/vk_shader.cpp). HandleHit is reordered and its tex_epoch is 32 bits, as texture_epoch() returns. The clear thresholds are the same, so entries are kept and dropped as before.
  • vk: payload<T>() (gpu/vk/vk_replay.cpp) copies sizeof(T) bytes, inline, when the op is at least that long (nearly always), instead of a variable-length memcpy, which was a call of libc's memmove for every op replayed. A shorter op is copied for its length as before.
  • vk: RecordingRes::c1_at (vk_draw.cpp) is a FlatMap keyed by the stage's address: no node allocation for each c1 upload a recording keeps, and no frees at the recording's clear. It is emptied at each recording's start with FlatMap::reset, which keeps its array (a clear would give an array over 4096 slots back and grow it again from 16 every recording); the recording streams' log line (every 3000 recordings) now says the most c1 stages a recording uploaded.
  • vk: program_for (vk_shader.cpp) holds Pick& pk across program_pick, the one place a reference into a FlatMap is held across a possible insert; a comment there says why that is safe (nothing in between inserts into the thread's picks: program_pick takes the non-recursive shader_mu and can't reach program_for again).
  • tests: tests/test_flat_map.py (from #2774: FlatMap against std::unordered_map, and texture_view_of over either; here also reset against clear, and c1_at emptied with reset); tests/test_replay_copies.py (payload<T> cut out and run for every length, and the flat c1_at); test_fsr_uniform_ownership.py, test_recording_admission.py and test_draw_stream_visibility.py include flat_map.h, since they cut RecordingRes out of the source.

vk_shader.cpp changes (the picks' map), so the translator's fingerprint changes: the first start after updating prepares the shaders again (the program cache of the old build is not read).

Flags and settings

None. The change is internal and gives the same results; reverting the commits removes it.

Measurements

No fps gain on the Steam Deck; on the PC +3.4 fps at MP165 (two pairs), none at M1152.

PC (PC runner, one prepared game folder per arm under D:\Githubproject\Longshot-bench, warm-up per folder; experiments.md, "flat caches redone with separate bench folders")

main: e28236b4 (2026-11-12)
this PR: opus/replay-flat-caches-measured @ 53a83cca (pushed to the fork so it can be seen: the same five changes on that main). The head has been rebased since (onto fbd4202, after #198 and #107, without conflict in these files) and has the review's c1_at reset and comment on top, not measured.
Hardware: RTX 5090, Windows 11 (the runner's PC)
Scene: Kokiri Forest, the runner's moving route; M1152 = 2048x1152 window; MP165 = the 165 Hz present config
Method: 2 interleaved rounds each (MP165 round 1 excluded and rerun as round 2: main's run had outside CPU load, 22 fps for ~20 s); fps and p99 from the runner's frame log, replay times from vk: a frame's replay time (ms) lines
How to reproduce: the runner's pcrun.ps1 labels lsflat (two folders: main and this PR, each built with make_game and shaders prepared)

main @ e28236b4 this PR @ 53a83cca
MP165 fps (2 pairs) 67.5, 70.0 72.2, 72.0 (+4.7, +2.0; mean +3.4)
MP165 p99 22.1 ms 20.4-21.3 ms
M1152 fps 75.0 75.4 (+0.4, sd 1.2: noise)
M1152 p99 19.7 ms 19.3 ms
replay "descriptors", ms a frame 1.61-1.81 1.54-1.57
replay "submit", ms a frame −0.3 to −1.1 against main

An earlier pair that swapped only the renderer DLL into one folder is withdrawn: this branch changes the translator's fingerprint, so the swapped DLL dropped the shader cache and prepared in play.

Steam Deck OLED, this PR alone (Deck agent, 2026-11-13): no fps gain.

main: d964886 + #225 (the Steam Deck profile, which the Deck needs: plain main runs out of memory there), e298bd4
this PR: opus/replay-flat-caches @ 559f480, merged on that base (a full Linux build)
Hardware: Steam Deck OLED (AMD Custom GPU 0932, RADV VANGOGH, Mesa 26.2.0-devel), SteamOS 3.9.5, 16 GB (swapping in every run)
Scene: Kokiri Forest, the moving route (125 s, measured 30-110 s), the user's options (Deck profile: 2048x1152 output, FSR 3.1 ~50%, frame generation 2x), stock clocks, memory guard on
Method: one warm-up start each (this PR's translated the shaders again: the stamp changes), then interleaved base, this PR, ..., this PR, base; fps from the Deck agent's frame log ("real": the game's frames); replay times the average of the last 6 a frame's replay time lines
How to reproduce: the Deck agent's harness, data in deck-s3 (X231-, LB-x)

main @ d964886 + #225 this PR @ 559f480
real fps (runs) 15.8, 14.7 14.1, 14.7
shown fps 30.2, 27.6 26.0, 26.5
p95 frame time 79, 84 ms 101, 108 ms
frames over 100 ms 74, 72 109, 123
replay submit / draw / descriptors, ms a frame 20.1-23.8 / 11.9-13.8 / 6.43-6.46 19.2-22.0 / 10.8-12.8 / 6.24-7.01
pages swapped out 673k, 390k 995k, 870k

Real fps is within the run-to-run spread (about ±1), and the replay's times are within the base's range: on the Deck the frame is set by the readback and GPU waits (~28 ms a frame waiting on the GPU), not by the replay. This PR's two runs had the worse p95 and more slow frames; they also swapped more, and the cause is not isolated (FlatMap's arrays are a few MB more per recording thread, Risk below).

Earlier, inside a combined Hookshot-based build, the render thread's profile showed what these changes target: texture_view_of 3.1%, program_for 2.5% and graphics_pipeline 1.5% of the thread were these three maps' lookups; libc memmove 6.7%; malloc/free about 3.5%.

Testing

Tested:

  • python -I -B tests/run_all.py, Linux (Ubuntu 22.04 container, clang 23, the CPM Vulkan headers) and Windows (llvm-mingw): this head @ 9951d91, rebased on main 5a8ae7e, fails exactly what main @ 5a8ae7e fails: Linux 5 (test_cpp_preflight, test_dev_packs, test_pad_layout, test_prof_clock, the byte-compile without .git), Windows 4 (test_bake_portability, test_fpx_sticky, test_pad_layout, test_spawn_emit); 512 / 469 passed.
  • tests/test_flat_map.py and tests/test_replay_copies.py pass on both.
  • In the game: the PC runner's runs above (Windows, RTX 5090, at 53a83cca); the Steam Deck OLED alone at 559f480 (above, 2 interleaved pairs). The head's c1_at reset and comment ran in the Deck's runs.

Not tested:

  • AMD and Intel GPUs on Windows.

Needs other testers:

  • A CPU-bound PC (4-6 cores, a laptop): Kokiri Forest's replay "descriptors" and "submit" ms a frame against main, expecting them lower and fps not lower (request to open)

Screenshots

Nothing on screen.

Risk and rollback

A lookup that answered differently would show as a wrong texture or pipeline; the tests compare FlatMap with std::unordered_map over random keys and texture_view_of over both. Reverting the commits is enough. The first start after the change prepares the shaders again (the translator's fingerprint changes with vk_shader.cpp).

Memory: FlatMap holds its entries in one array. At handle_hits' clear threshold (65536 entries) that is 131072 × 56 B ≈ 7.3 MB per recording thread, against about 5 MB of nodes before, with a moment at about 11 MB while it grows (the old array and the new). With several recording threads (RX_VK_RECORDER=thread, the parallel workers) that adds up to a few MB more on the 8 GB / Steam Deck tier. c1_at keeps its array between recordings (its largest size so far, logged).

Reviewer notes

Read gpu/vk/flat_map.h first, then the three map declarations in vk_draw.cpp and vk_shader.cpp, then payload<T> in vk_replay.cpp.

Issues

See https://github.com/NotRexed/Hookshot/pull/2774 (the flat caches, by Corentin Bonjour).

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). (The test request is still to open.)
  • 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.
## Why Each draw the Vulkan replay records on the game's render thread spent a good part of its time in hash-map lookups, a C library `memmove` per replayed op, and a heap allocation per kept c1 upload. This makes those per-draw costs cheaper, with the same results. **Measured: +3.4 fps on an RTX 5090 at 5120x2160 (MP165), none at 2048x1152 (M1152), and none on the Steam Deck**, whose frame is set by readback and GPU waits, not by the replay (below). ## What changed - vk: `gpu/vk/flat_map.h` (new): `FlatMap<K, V>`, an integer-keyed table in one array (open addressing, linear probing, at most 3/4 full, slot from a multiplicative hash; `find`/`end`, `operator[]`, `size`, `clear`; a clear gives back an array over 4096 slots). Ported from Corentin Bonjour's https://github.com/NotRexed/Hookshot/pull/2774 (its two commits, with authorship kept). - vk: three per-draw thread caches use it: `handle_hits` (`texture_view_of`, `gpu/vk/vk_draw.cpp`), `graphics_pipeline`'s `mine` (`vk_draw.cpp`) and `picks` (`program_for`, `gpu/vk/vk_shader.cpp`). `HandleHit` is reordered and its `tex_epoch` is 32 bits, as `texture_epoch()` returns. The clear thresholds are the same, so entries are kept and dropped as before. - vk: `payload<T>()` (`gpu/vk/vk_replay.cpp`) copies `sizeof(T)` bytes, inline, when the op is at least that long (nearly always), instead of a variable-length `memcpy`, which was a call of libc's `memmove` for every op replayed. A shorter op is copied for its length as before. - vk: `RecordingRes::c1_at` (`vk_draw.cpp`) is a `FlatMap` keyed by the stage's address: no node allocation for each c1 upload a recording keeps, and no frees at the recording's clear. It is emptied at each recording's start with `FlatMap::reset`, which keeps its array (a `clear` would give an array over 4096 slots back and grow it again from 16 every recording); the recording streams' log line (every 3000 recordings) now says the most c1 stages a recording uploaded. - vk: `program_for` (`vk_shader.cpp`) holds `Pick& pk` across `program_pick`, the one place a reference into a `FlatMap` is held across a possible insert; a comment there says why that is safe (nothing in between inserts into the thread's `picks`: `program_pick` takes the non-recursive `shader_mu` and can't reach `program_for` again). - tests: `tests/test_flat_map.py` (from #2774: `FlatMap` against `std::unordered_map`, and `texture_view_of` over either; here also `reset` against `clear`, and `c1_at` emptied with `reset`); `tests/test_replay_copies.py` (`payload<T>` cut out and run for every length, and the flat `c1_at`); `test_fsr_uniform_ownership.py`, `test_recording_admission.py` and `test_draw_stream_visibility.py` include `flat_map.h`, since they cut `RecordingRes` out of the source. `vk_shader.cpp` changes (the picks' map), so the translator's fingerprint changes: the first start after updating prepares the shaders again (the program cache of the old build is not read). ## Flags and settings None. The change is internal and gives the same results; reverting the commits removes it. ## Measurements **No fps gain on the Steam Deck; on the PC +3.4 fps at MP165 (two pairs), none at M1152.** **PC (PC runner, one prepared game folder per arm under `D:\Githubproject\Longshot-bench`, warm-up per folder; experiments.md, "flat caches redone with separate bench folders")** main: e28236b4 (2026-11-12) this PR: opus/replay-flat-caches-measured @ 53a83cca (pushed to the fork so it can be seen: the same five changes on that main). The head has been rebased since (onto fbd4202, after #198 and #107, without conflict in these files) and has the review's `c1_at` reset and comment on top, not measured. Hardware: RTX 5090, Windows 11 (the runner's PC) Scene: Kokiri Forest, the runner's moving route; M1152 = 2048x1152 window; MP165 = the 165 Hz present config Method: 2 interleaved rounds each (MP165 round 1 excluded and rerun as round 2: main's run had outside CPU load, 22 fps for ~20 s); fps and p99 from the runner's frame log, replay times from `vk: a frame's replay time (ms)` lines How to reproduce: the runner's `pcrun.ps1` labels `lsflat` (two folders: main and this PR, each built with `make_game` and shaders prepared) | | main @ e28236b4 | this PR @ 53a83cca | |---|---|---| | MP165 fps (2 pairs) | 67.5, 70.0 | 72.2, 72.0 (**+4.7, +2.0; mean +3.4**) | | MP165 p99 | 22.1 ms | 20.4-21.3 ms | | M1152 fps | 75.0 | 75.4 (+0.4, sd 1.2: noise) | | M1152 p99 | 19.7 ms | 19.3 ms | | replay "descriptors", ms a frame | 1.61-1.81 | 1.54-1.57 | | replay "submit", ms a frame | | −0.3 to −1.1 against main | An earlier pair that swapped only the renderer DLL into one folder is withdrawn: this branch changes the translator's fingerprint, so the swapped DLL dropped the shader cache and prepared in play. **Steam Deck OLED, this PR alone (Deck agent, 2026-11-13): no fps gain.** main: d964886 + #225 (the Steam Deck profile, which the Deck needs: plain main runs out of memory there), e298bd4 this PR: opus/replay-flat-caches @ 559f480, merged on that base (a full Linux build) Hardware: Steam Deck OLED (AMD Custom GPU 0932, RADV VANGOGH, Mesa 26.2.0-devel), SteamOS 3.9.5, 16 GB (swapping in every run) Scene: Kokiri Forest, the moving route (125 s, measured 30-110 s), the user's options (Deck profile: 2048x1152 output, FSR 3.1 ~50%, frame generation 2x), stock clocks, memory guard on Method: one warm-up start each (this PR's translated the shaders again: the stamp changes), then interleaved base, this PR, ..., this PR, base; fps from the Deck agent's frame log ("real": the game's frames); replay times the average of the last 6 `a frame's replay time` lines How to reproduce: the Deck agent's harness, data in `deck-s3` (X231-*, LB-x*) | | main @ d964886 + #225 | this PR @ 559f480 | |---|---|---| | real fps (runs) | 15.8, 14.7 | 14.1, 14.7 | | shown fps | 30.2, 27.6 | 26.0, 26.5 | | p95 frame time | 79, 84 ms | 101, 108 ms | | frames over 100 ms | 74, 72 | 109, 123 | | replay submit / draw / descriptors, ms a frame | 20.1-23.8 / 11.9-13.8 / 6.43-6.46 | 19.2-22.0 / 10.8-12.8 / 6.24-7.01 | | pages swapped out | 673k, 390k | 995k, 870k | Real fps is within the run-to-run spread (about ±1), and the replay's times are within the base's range: on the Deck the frame is set by the readback and GPU waits (~28 ms a frame waiting on the GPU), not by the replay. This PR's two runs had the worse p95 and more slow frames; they also swapped more, and the cause is not isolated (FlatMap's arrays are a few MB more per recording thread, Risk below). Earlier, inside a combined Hookshot-based build, the render thread's profile showed what these changes target: `texture_view_of` 3.1%, `program_for` 2.5% and `graphics_pipeline` 1.5% of the thread were these three maps' lookups; libc `memmove` 6.7%; malloc/free about 3.5%. ## Testing **Tested:** - `python -I -B tests/run_all.py`, Linux (Ubuntu 22.04 container, clang 23, the CPM Vulkan headers) and Windows (llvm-mingw): this head @ 9951d91, rebased on main 5a8ae7e, fails exactly what main @ 5a8ae7e fails: Linux 5 (`test_cpp_preflight`, `test_dev_packs`, `test_pad_layout`, `test_prof_clock`, the byte-compile without `.git`), Windows 4 (`test_bake_portability`, `test_fpx_sticky`, `test_pad_layout`, `test_spawn_emit`); 512 / 469 passed. - `tests/test_flat_map.py` and `tests/test_replay_copies.py` pass on both. - In the game: the PC runner's runs above (Windows, RTX 5090, at 53a83cca); the Steam Deck OLED alone at 559f480 (above, 2 interleaved pairs). The head's `c1_at` reset and comment ran in the Deck's runs. **Not tested:** - AMD and Intel GPUs on Windows. **Needs other testers:** - A CPU-bound PC (4-6 cores, a laptop): Kokiri Forest's replay "descriptors" and "submit" ms a frame against main, expecting them lower and fps not lower (request to open) ## Screenshots Nothing on screen. ## Risk and rollback A lookup that answered differently would show as a wrong texture or pipeline; the tests compare `FlatMap` with `std::unordered_map` over random keys and `texture_view_of` over both. Reverting the commits is enough. The first start after the change prepares the shaders again (the translator's fingerprint changes with `vk_shader.cpp`). Memory: `FlatMap` holds its entries in one array. At `handle_hits`' clear threshold (65536 entries) that is 131072 × 56 B ≈ 7.3 MB per recording thread, against about 5 MB of nodes before, with a moment at about 11 MB while it grows (the old array and the new). With several recording threads (`RX_VK_RECORDER=thread`, the parallel workers) that adds up to a few MB more on the 8 GB / Steam Deck tier. `c1_at` keeps its array between recordings (its largest size so far, logged). ## Reviewer notes Read `gpu/vk/flat_map.h` first, then the three map declarations in `vk_draw.cpp` and `vk_shader.cpp`, then `payload<T>` in `vk_replay.cpp`. ## Issues See https://github.com/NotRexed/Hookshot/pull/2774 (the flat caches, by Corentin Bonjour). ## 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. - [ ] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). (The test request is still to open.) - [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. - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.
handle_hits, picks and graphics_pipeline's mine are searched on every draw
of the replay's render thread, the frame's bottleneck. std::unordered_map
paid a modulo by a prime and a node pointer chase per lookup; FlatMap
(open addressing, linear probing, at most half full) keeps key and value
in one array. HandleHit's tex_epoch takes texture_epoch()'s 32 bits, so a
slot is 56 bytes. tests/test_flat_map.py checks it against
std::unordered_map and texture_view_of over both.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 0b8ad723bc42abbdf2c5be959089c1b412019222)
At handle_hits' 65536-entry clear threshold the half-full table took
14 MB per recording thread against about 5 MB for the node map, and
clear kept it. Up to 3/4 full it is 7 MB there, and a clear gives back
an array over 4096 slots, so a cache cleared at its limit does not keep
its peak.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 46f612e2da37d58a3def0f40e4a6d17db2dac1ea)
Co-authored-by: Copilot App <[email protected]>
(cherry picked from commit 788dc480c412e11b679aa5a7e58993c4018079e7)
(cherry picked from commit fc9a88e8 in Hookshot's Deck branch; Longshot has no uniform snapshot map, so only c1_at)

Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
fix(tests): the checks that cut RecordingRes include flat_map.h
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 16s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
8ca9aa0423
Assisted-by: Claude Opus 5.5

Co-Authored-By: Claude Opus 5.5 <[email protected]>
HumanBean left a comment

Read-only review of 8ca9aa04 against main@origin (1ef93e79). Nothing built or run.

What's good

  • FlatMap is correct as far as I can trace it. The load factor stays at or below 3/4, so a probe always reaches an empty slot. shift_ is right for every power-of-two size. find checks count_ before it hashes, so the shift of 64 on an empty table is never evaluated. A failed allocation in grow() throws before the swap and leaves the map as it was. The hash is multiplicative and takes the top bits, so the zero low bits of c1_at's pointer keys and the slot/want/use/format fields of handle_hits' keys spread well.
  • The behaviour matches main. Nothing iterates these maps (there is no begin(), so it wouldn't compile), and the clear thresholds haven't moved. tex_epoch going to 32 bits matches uint32_t texture_epoch(), and the reordered HandleHit initializer at its only use matches the new field order.
    • Stale entries: pipelines is never erased, so mine's PipeEntry* can't dangle. handle_hits still checks version and destroy_gen before it trusts tex.
    • Threads: all four maps belong to one thread (thread_local, TLS_OBJECT, or reached through the thread-local cur), as on main, so RX_VK_RECORDER=thread and the parallel workers don't share them.
  • References across an insert: FlatMap moves its entries when it grows, where unordered_map didn't. Only one place holds a reference across a possible insert: Pick& pk = picks[key] is held across program_pick. That is safe, because program_pick takes the non-recursive shader_mu and can't reach program_for again. In texture_view_of and uniform_range, the found slot isn't used after the operator[] write.
  • payload<T> does the same thing as before: it never reads past bytes, and memcpy needs no alignment.
  • Conflicts: none with #198 (its vk_draw.cpp hunks are all at lines 3814 and later, away from these; its one-line change to recording_admission_test.cpp builds with the -include flat_map.h added here). None with #216 (it changes only vk_memory.cpp).
  • The checks that cut RecordingRes get the -include. test_handle_lookup, test_same_draw, test_pick_tex_gen and test_mem_trim still find their markers. test_flat_map.py is a good differential check.

Must change (before it leaves WIP)

  • "Rebased on current main" is ticked but isn't true. The fork point is fdc2840b, and main has 08771847 and 1ef93e79 on top. Neither touches these files, so a rebase is trivial. Rebase, or untick the box.

  • The measurement is of a commit nobody else can see. The PC numbers are for 53a83cca, which isn't on the forge, and the head has been rebased since. The evidence is also thin:

    • MP165 rests on two pairs, after round 1 was dropped and rerun.
    • M1152 is one pair, inside the noise (+0.4 fps, sd 1.2).
    • The PC is an RTX 5090, not a CPU-bound machine.
    • On the Deck, the change was only measured as part of a combined build.

    The Measured box is rightly unticked. Before pr ready, measure the head (or a pushed commit) against main with several interleaved runs, ideally on a CPU-bound machine, and report the replay "descriptors"/"submit" ms as well as fps. Then open the test request and add the docs/tested-configurations.md row (both boxes are still open).

Should change

  • Say in the code why pk in program_for is safe to hold across program_pick. The header says no reference may be held across an insert, and this is the only place that does it. A future callback from program_pick into the draw path would turn it into a use-after-free that the old map would have survived.
  • c1_at is cleared at every recording start, and clear() gives back any array over 4096 slots. A recording with more than 3072 distinct c1 stages would therefore free its table and grow it again from 16 every recording: about 9 reallocations and rehashes, a cost the old map didn't have in that form. Below that size, a clear walks up to 4096 slots. Both are probably small for OoT. Logging c1_at.size()'s peak once would settle it, or c1_at could take a clear that keeps its capacity.
  • Memory: at the 65536-entry clear threshold, handle_hits holds 131072 × 56 B ≈ 7.3 MB per recording thread, against about 5 MB before (as the commit message says), with a moment at about 11 MB while it grows. That's fine, but worth a line under Risk for the 8 GB/Deck tier when several recording threads are running.
Read-only review of 8ca9aa04 against main@origin (1ef93e79). Nothing built or run. **What's good** - `FlatMap` is correct as far as I can trace it. The load factor stays at or below 3/4, so a probe always reaches an empty slot. `shift_` is right for every power-of-two size. `find` checks `count_` before it hashes, so the shift of 64 on an empty table is never evaluated. A failed allocation in `grow()` throws before the swap and leaves the map as it was. The hash is multiplicative and takes the top bits, so the zero low bits of `c1_at`'s pointer keys and the slot/want/use/format fields of `handle_hits`' keys spread well. - The behaviour matches main. Nothing iterates these maps (there is no `begin()`, so it wouldn't compile), and the clear thresholds haven't moved. `tex_epoch` going to 32 bits matches `uint32_t texture_epoch()`, and the reordered `HandleHit` initializer at its only use matches the new field order. - **Stale entries:** `pipelines` is never erased, so `mine`'s `PipeEntry*` can't dangle. `handle_hits` still checks `version` and `destroy_gen` before it trusts `tex`. - **Threads:** all four maps belong to one thread (`thread_local`, `TLS_OBJECT`, or reached through the thread-local `cur`), as on main, so `RX_VK_RECORDER=thread` and the parallel workers don't share them. - **References across an insert:** `FlatMap` moves its entries when it grows, where `unordered_map` didn't. Only one place holds a reference across a possible insert: `Pick& pk = picks[key]` is held across `program_pick`. That is safe, because `program_pick` takes the non-recursive `shader_mu` and can't reach `program_for` again. In `texture_view_of` and `uniform_range`, the found slot isn't used after the `operator[]` write. - `payload<T>` does the same thing as before: it never reads past `bytes`, and `memcpy` needs no alignment. - **Conflicts:** none with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/198 (its `vk_draw.cpp` hunks are all at lines 3814 and later, away from these; its one-line change to `recording_admission_test.cpp` builds with the `-include flat_map.h` added here). None with https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/216 (it changes only `vk_memory.cpp`). - The checks that cut `RecordingRes` get the `-include`. `test_handle_lookup`, `test_same_draw`, `test_pick_tex_gen` and `test_mem_trim` still find their markers. `test_flat_map.py` is a good differential check. **Must change (before it leaves WIP)** - **"Rebased on current `main`" is ticked but isn't true.** The fork point is fdc2840b, and main has 08771847 and 1ef93e79 on top. Neither touches these files, so a rebase is trivial. Rebase, or untick the box. - **The measurement is of a commit nobody else can see.** The PC numbers are for `53a83cca`, which isn't on the forge, and the head has been rebased since. The evidence is also thin: - MP165 rests on two pairs, after round 1 was dropped and rerun. - M1152 is one pair, inside the noise (+0.4 fps, sd 1.2). - The PC is an RTX 5090, not a CPU-bound machine. - On the Deck, the change was only measured as part of a combined build. The *Measured* box is rightly unticked. Before `pr ready`, measure the head (or a pushed commit) against main with several interleaved runs, ideally on a CPU-bound machine, and report the replay "descriptors"/"submit" ms as well as fps. Then open the test request and add the `docs/tested-configurations.md` row (both boxes are still open). **Should change** - Say in the code why `pk` in `program_for` is safe to hold across `program_pick`. The header says no reference may be held across an insert, and this is the only place that does it. A future callback from `program_pick` into the draw path would turn it into a use-after-free that the old map would have survived. - **`c1_at` is cleared at every recording start, and `clear()` gives back any array over 4096 slots.** A recording with more than 3072 distinct c1 stages would therefore free its table and grow it again from 16 every recording: about 9 reallocations and rehashes, a cost the old map didn't have in that form. Below that size, a clear walks up to 4096 slots. Both are probably small for OoT. Logging `c1_at.size()`'s peak once would settle it, or `c1_at` could take a clear that keeps its capacity. - **Memory:** at the 65536-entry clear threshold, `handle_hits` holds 131072 × 56 B ≈ 7.3 MB per recording thread, against about 5 MB before (as the commit message says), with a moment at about 11 MB while it grows. That's fine, but worth a line under Risk for the 8 GB/Deck tier when several recording threads are running.
@ -0,0 +38,4 @@
void clear() {
if (!count_) return;
count_ = 0;
if (slots_.size() > kKeptSlots) { // (a cache cleared at its size limit: its memory goes back too)
Owner

This suits the caches cleared at their size limit. RecordingRes::c1_at, though, is cleared at every recording start: a recording with more than 3072 c1 stages would free its table and grow it again from 16 (about 9 reallocations and rehashes) every recording. Below that size the clear walks up to 4096 slots. Probably small for OoT. Do you have c1_at's peak size? If it can get near this, c1_at could take a clear that keeps its capacity.

This suits the caches cleared at their size limit. `RecordingRes::c1_at`, though, is cleared at every recording start: a recording with more than 3072 c1 stages would free its table and grow it again from 16 (about 9 reallocations and rehashes) every recording. Below that size the clear walks up to 4096 slots. Probably small for OoT. Do you have `c1_at`'s peak size? If it can get near this, `c1_at` could take a clear that keeps its capacity.
@ -1731,3 +1732,3 @@
std::vector<uint32_t> handles; // (at its answers' descriptors: their slots and the slots' versions)
};
thread_local std::unordered_map<uint64_t, Pick> picks; // (each recording thread's: program_pick has its own lock)
thread_local FlatMap<uint64_t, Pick> picks; // (each recording thread's: program_pick has its own lock)
Owner

program_for holds Pick& pk = picks[key] across program_pick(...) and writes through it afterwards. With FlatMap, an insert into picks during that call would move the entry and leave pk dangling, where unordered_map kept references stable. It's safe today, because program_pick takes the non-recursive shader_mu and never gets back to program_for. Please add a line here or at pk saying so, since flat_map.h's own rule is that no reference is held across an insert.

`program_for` holds `Pick& pk = picks[key]` across `program_pick(...)` and writes through it afterwards. With `FlatMap`, an insert into `picks` during that call would move the entry and leave `pk` dangling, where `unordered_map` kept references stable. It's safe today, because `program_pick` takes the non-recursive `shader_mu` and never gets back to `program_for`. Please add a line here or at `pk` saying so, since flat_map.h's own rule is that no reference is held across an insert.
dukenheimer force-pushed opus/replay-flat-caches from 8ca9aa0423
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 16s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 4727116511
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 52s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 17:17:31 +00:00
Compare
dukenheimer force-pushed opus/replay-flat-caches from 4727116511
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 52s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 559f48074b
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 17:58:28 +00:00
Compare
Author
First-time contributor

Thanks for the review. Point by point, at 559f480 (rebased on main fbd4202):

Must change

  • Rebased: the branch is now on current main fbd4202 (after #198, #107 and the rest), with no conflicts, so the box is true. Suites below.
  • Measurement of a commit nobody can see: 53a83cca is pushed to the fork as opus/replay-flat-caches-measured, and the description names it. The description now says plainly how thin the evidence is: two MP165 pairs after a rerun, one M1152 pair inside the noise, an RTX 5090, and the Deck only inside a combined build. Measured stays unticked. Still owed before pr ready: this head against main with several interleaved runs on a CPU-bound machine (the Steam Deck), with the replay's "descriptors" and "submit" ms as well as fps, then the test request and the docs/tested-configurations.md row. I don't run the game myself, so I've asked for those runs.

Should change

  • pk across program_pick: there is now a comment at Pick& pk = picks[key] (gpu/vk/vk_shader.cpp, in 559f480). It says nothing between the lookup and the write inserts into this thread's picks: program_pick takes the non-recursive shader_mu and can't reach program_for again. A future callback from it into the draw path would have to look pk up again.
  • c1_at cleared at every recording: FlatMap::reset() empties the map and keeps its array whatever its size, and c1_at uses it at each recording's start (clear() still gives a big array back for the caches cleared at their size limit). I don't have a peak size yet, so the recording streams' log line (every 3000 recordings) now ends with c1 stages: at most N a recording, and the next run will show it. tests/test_flat_map.py checks reset against clear (capacity kept or given back, then refilled) and alternates the two in the differential against std::unordered_map; its source check requires r.c1_at.reset().
  • Memory: there is a paragraph under Risk: ~7.3 MB per recording thread at handle_hits' threshold against ~5 MB before, ~11 MB for a moment while it grows, a few MB more on the 8 GB / Deck tier with several recording threads.

Tests (python -I -B tests/run_all.py): on Linux, 559f480 and main @ fbd4202 fail the same 4 (test_cpp_preflight, test_dev_packs, test_prof_clock, the byte-compile without .git); on Windows, 454 passed and the same 5 failed as on main (test_fpx_sticky, test_bake_portability, test_check_third_party, test_spawn_emit, test_release).

Thanks for the review. Point by point, at 559f480 (rebased on main fbd4202): **Must change** - **Rebased:** the branch is now on current main fbd4202 (after #198, #107 and the rest), with no conflicts, so the box is true. Suites below. - **Measurement of a commit nobody can see:** 53a83cca is pushed to the fork as `opus/replay-flat-caches-measured`, and the description names it. The description now says plainly how thin the evidence is: two MP165 pairs after a rerun, one M1152 pair inside the noise, an RTX 5090, and the Deck only inside a combined build. *Measured* stays unticked. Still owed before `pr ready`: this head against main with several interleaved runs on a CPU-bound machine (the Steam Deck), with the replay's "descriptors" and "submit" ms as well as fps, then the test request and the `docs/tested-configurations.md` row. I don't run the game myself, so I've asked for those runs. **Should change** - **`pk` across `program_pick`:** there is now a comment at `Pick& pk = picks[key]` (`gpu/vk/vk_shader.cpp`, in 559f480). It says nothing between the lookup and the write inserts into this thread's `picks`: `program_pick` takes the non-recursive `shader_mu` and can't reach `program_for` again. A future callback from it into the draw path would have to look `pk` up again. - **`c1_at` cleared at every recording:** `FlatMap::reset()` empties the map and keeps its array whatever its size, and `c1_at` uses it at each recording's start (`clear()` still gives a big array back for the caches cleared at their size limit). I don't have a peak size yet, so the recording streams' log line (every 3000 recordings) now ends with `c1 stages: at most N a recording`, and the next run will show it. `tests/test_flat_map.py` checks `reset` against `clear` (capacity kept or given back, then refilled) and alternates the two in the differential against `std::unordered_map`; its source check requires `r.c1_at.reset()`. - **Memory:** there is a paragraph under Risk: ~7.3 MB per recording thread at `handle_hits`' threshold against ~5 MB before, ~11 MB for a moment while it grows, a few MB more on the 8 GB / Deck tier with several recording threads. **Tests** (`python -I -B tests/run_all.py`): on Linux, 559f480 and main @ fbd4202 fail the same 4 (`test_cpp_preflight`, `test_dev_packs`, `test_prof_clock`, the byte-compile without `.git`); on Windows, 454 passed and the same 5 failed as on main (`test_fpx_sticky`, `test_bake_portability`, `test_check_third_party`, `test_spawn_emit`, `test_release`).
Author
First-time contributor

Steam Deck: no measurable change.

Setup:

  • Hardware and OS: Steam Deck OLED, SteamOS 3.9.5 (kernel 7.2.7-valve2), RADV Mesa 26.2.0-devel.
  • Base: main d964886 + the Steam Deck profile (#225), because plain main runs out of memory on the Deck. This PR is merged on that base.
  • Settings: the user's options, stock clocks, the moving Kokiri Forest route, warm caches, memory guard on.
  • Order: one warm-up start each, then base, #231, #232, #156, then #156, #232, #231, base, interleaved.
  • On this Deck and route one run spreads about ±1 real fps.
  • The PR changes the translator stamp, so its warm-up start translated again (0 program keys kept, 231 s). Budget one cold start when it lands.
real fps shown fps p95 hitches replay submit / draw / descriptors (avg of last 6 lines)
base, run 1 15.8 30.2 79 ms 74 20.1 / 11.9 / 6.46 ms
base, run 2 14.7 27.6 84 ms 72 23.8 / 13.8 / 6.43 ms
#231, run 1 14.1 26.0 101 ms 109 22.0 / 12.8 / 7.01 ms
#231, run 2 14.7 26.5 108 ms 123 19.2 / 10.8 / 6.24 ms

The replay's descriptor and draw times are inside the base's range. On the Deck the frame is set by the readback and GPU waits (~28 ms a frame waiting on the GPU), not by the replay. Memory is the same (RSS 7.5-8.3 GB).

**Steam Deck: no measurable change.** **Setup:** - Hardware and OS: Steam Deck OLED, SteamOS 3.9.5 (kernel 7.2.7-valve2), RADV Mesa 26.2.0-devel. - Base: main d964886 + the Steam Deck profile (#225), because plain main runs out of memory on the Deck. This PR is merged on that base. - Settings: the user's options, stock clocks, the moving Kokiri Forest route, warm caches, memory guard on. - Order: one warm-up start each, then base, #231, #232, #156, then #156, #232, #231, base, interleaved. - On this Deck and route one run spreads about ±1 real fps. - The PR changes the translator stamp, so its warm-up start translated again (0 program keys kept, 231 s). Budget one cold start when it lands. | | real fps | shown fps | p95 | hitches | replay submit / draw / descriptors (avg of last 6 lines) | |---|---|---|---|---|---| | base, run 1 | 15.8 | 30.2 | 79 ms | 74 | 20.1 / 11.9 / 6.46 ms | | base, run 2 | 14.7 | 27.6 | 84 ms | 72 | 23.8 / 13.8 / 6.43 ms | | **#231**, run 1 | 14.1 | 26.0 | 101 ms | 109 | 22.0 / 12.8 / 7.01 ms | | **#231**, run 2 | 14.7 | 26.5 | 108 ms | 123 | 19.2 / 10.8 / 6.24 ms | The replay's descriptor and draw times are inside the base's range. On the Deck the frame is set by the readback and GPU waits (~28 ms a frame waiting on the GPU), not by the replay. Memory is the same (RSS 7.5-8.3 GB).
dukenheimer force-pushed opus/replay-flat-caches from 559f48074b
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 9951d916d8
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 8s
Website / Website on Cloudflare (pull_request_target) Has been skipped
2026-11-14 02:00:17 +00:00
Compare
Author
First-time contributor

Steam Deck, this PR alone: no fps gain. These are the Deck agent's runs (comment above) on a Steam Deck OLED, main d964886 + #225, 2 interleaved pairs:

  • Real fps: 14.1 / 14.7 against the base's 15.8 / 14.7, within the run-to-run spread.
  • Replay times: descriptors 6.24–7.01 against 6.43–6.46 ms; within the base's range. The Deck's frame is set by readback and GPU waits, not by the replay.
  • Slow frames: this PR's two runs had the worse p95 (101 / 108 against 79 / 84 ms) and more frames over 100 ms (109 / 123 against 74 / 72). They also swapped more (870k–995k pages out against 390k–673k); the cause isn't isolated.

So the only measured gain is the PC's +3.4 fps at MP165 (RTX 5090, two pairs). There is none at M1152 and none on the Deck. I've corrected the description's Why and Measurements to say so plainly; the Deck no longer appears as a reason for this PR.

Also in this update:

  • Rebased on main 5a8ae7e (head 9951d91), no conflicts. Suites fail exactly main's set (Linux 5, Windows 4).
  • docs/tested-configurations.md has the Deck runs.
**Steam Deck, this PR alone: no fps gain.** These are the Deck agent's runs (comment above) on a Steam Deck OLED, main d964886 + #225, 2 interleaved pairs: - **Real fps:** 14.1 / 14.7 against the base's 15.8 / 14.7, within the run-to-run spread. - **Replay times:** descriptors 6.24–7.01 against 6.43–6.46 ms; within the base's range. The Deck's frame is set by readback and GPU waits, not by the replay. - **Slow frames:** this PR's two runs had the worse p95 (101 / 108 against 79 / 84 ms) and more frames over 100 ms (109 / 123 against 74 / 72). They also swapped more (870k–995k pages out against 390k–673k); the cause isn't isolated. **So the only measured gain is the PC's +3.4 fps at MP165** (RTX 5090, two pairs). There is none at M1152 and none on the Deck. I've corrected the description's Why and Measurements to say so plainly; the Deck no longer appears as a reason for this PR. Also in this update: - Rebased on main 5a8ae7e (head 9951d91), no conflicts. Suites fail exactly main's set (Linux 5, Windows 4). - `docs/tested-configurations.md` has the Deck runs.
Contributor

Measured on a 6-core desktop, Linux: no change in the Market

No gain and no regression I can tell from the noise on this PC: fps, the 1% low, the frame times and the replay's cost per draw are within the spread of the base's own two runs. The frame there is not the replay alone (23.9 ms against 18.7 ms of replay and 12.8 ms of GPU).

main: 5a8ae7e3 (this PR's merge base), built as longshot abc-base
this PR: 9951d916, built as longshot abc-231
Baseline: main @ 5a8ae7e3
Hardware: RX 9070 XT 16 GB, Mesa RADV 26.2.6 (GFX1201); Ryzen 5 5600X (6 cores, 12 threads), 31 GB; Fedora 45, kernel 7.3.3, KDE Plasma (Wayland)
Settings: output 2560x1440 in a 1280x720 window (mailbox), AMD FSR 4 Native AA (fsr4-vk, 4.1.1 FP8), HDR Expanded, sharpness 50%, AF 16x, VRAM budget 11 GB, frame generation off; HOS_FPS_MAX=240 (uncapped), HOS_PRESENT_TRACE on for every run
Scene: the Market (child) from one save, the same time of day each run, standing still: no input at all
Method: one full build per side from the Linux container (scripts/linux/docker.sh, then the AppImage's --cli --fsr), each in its own folder; a warm-up start of each; then six runs through the control bridge (scripts/longshot_ctl.py launch --save --slot, A until in play, the Market loaded and no event, 10 s, then 120 s measured, quit), interleaved base, this PR, #232, #232, this PR, base. Medians of the two runs a side.
How to reproduce: a child Market save; longshot_ctl.py launch <game> --save <save folder> --slot <NN> --env HOS_FPS_MAX=240 --env HOS_PRESENT_TRACE=<csv>; frame times from the trace between two status frames 120 s apart

main @ 5a8ae7e3 (run 1 / run 6) this PR @ 9951d916 (run 2 / run 5) Δ (medians)
fps, average 41.6 / 41.7 41.6 / 41.8 +0.1%
fps, 1% low 32.1 / 32.2 32.7 / 31.8 +0.3%
frame time, median 23.92 / 23.84 ms 23.92 / 23.84 ms 0.00 ms
frame time, p99 29.48 / 29.54 ms 29.27 / 29.36 ms -0.19 ms
replay submit a frame (Presentation thread) 18.68 / 18.72 ms 18.67 / 18.73 ms 0.00 ms
replay submit per 1000 draws 7.10 / 7.10 ms 7.08 / 7.06 ms -0.03 ms (-0.4%)
replay draw / descriptors (run 1 against run 2) 10.43 / 2.76 ms 10.26 / 2.56 ms
draws a frame 2626 / 2637 2636 / 2636 same scene
GPU time a frame 12.8 / 12.8 ms 12.8 / 12.8 ms none

A status line from each side, in the measured window:

main @ 5a8ae7e3 (run 1)
[hos Presentation Thread] vk: 4920 presents, 41.6 fps; a frame: 2642 draws (0 skipped), 121 dispatches, 175 passes, 161 barriers, 6 submits, 0.0 uploads (0.0 MB), 82 ub updates (57 overlays), GPU-bound wait 0.1 ms
[hos Presentation Thread] vk: a frame's replay time (ms): submit 18.42, present 0.07, draw 10.43, descriptors 2.76, texture lookups 0.11, uploads 0.00, pipelines 0.46, programs 1.35, commands 4.90, dispatch 1.13, memory sync 2.37, queue submit 1.55
[hos Presentation Thread] vk: GPU time 12.9 ms a frame (the longest submission 3.3 ms); 0.6 textures made a frame

this PR @ 9951d916 (run 2)
[hos Presentation Thread] vk: 4800 presents, 41.7 fps; a frame: 2646 draws (0 skipped), 121 dispatches, 175 passes, 161 barriers, 6 submits, 0.0 uploads (0.0 MB), 82 ub updates (57 overlays), GPU-bound wait 0.1 ms
[hos Presentation Thread] vk: a frame's replay time (ms): submit 18.74, present 0.06, draw 10.26, descriptors 2.56, texture lookups 0.09, uploads 0.00, pipelines 0.38, programs 1.32, commands 5.05, dispatch 1.11, memory sync 2.73, queue submit 1.59
[hos Presentation Thread] vk: GPU time 12.9 ms a frame (the longest submission 3.3 ms); 0.0 textures made a frame

Also checked: python -I -B tests/run_all.py on this PR: 470 passed, 2 failed, 74 skipped. The two (tests/test_menu_keys.py, tests/test_pad_layout.py) fail on main @ 5a8ae7e3 on this PC too, with tests/test_display_pointer.py once more there. No device lost in any run.

## Measured on a 6-core desktop, Linux: no change in the Market No gain and no regression I can tell from the noise on this PC: fps, the 1% low, the frame times and the replay's cost per draw are within the spread of the base's own two runs. The frame there is not the replay alone (23.9 ms against 18.7 ms of replay and 12.8 ms of GPU). main: 5a8ae7e3 (this PR's merge base), built as `longshot abc-base` this PR: 9951d916, built as `longshot abc-231` Baseline: main @ 5a8ae7e3 Hardware: RX 9070 XT 16 GB, Mesa RADV 26.2.6 (GFX1201); Ryzen 5 5600X (6 cores, 12 threads), 31 GB; Fedora 45, kernel 7.3.3, KDE Plasma (Wayland) Settings: output 2560x1440 in a 1280x720 window (mailbox), AMD FSR 4 Native AA (fsr4-vk, 4.1.1 FP8), HDR Expanded, sharpness 50%, AF 16x, VRAM budget 11 GB, frame generation off; `HOS_FPS_MAX=240` (uncapped), `HOS_PRESENT_TRACE` on for every run Scene: the Market (child) from one save, the same time of day each run, standing still: no input at all Method: one full build per side from the Linux container (`scripts/linux/docker.sh`, then the AppImage's `--cli --fsr`), each in its own folder; a warm-up start of each; then six runs through the control bridge (`scripts/longshot_ctl.py launch --save --slot`, A until in play, the Market loaded and no event, 10 s, then 120 s measured, `quit`), interleaved base, this PR, #232, #232, this PR, base. Medians of the two runs a side. How to reproduce: a child Market save; `longshot_ctl.py launch <game> --save <save folder> --slot <NN> --env HOS_FPS_MAX=240 --env HOS_PRESENT_TRACE=<csv>`; frame times from the trace between two `status` frames 120 s apart | | main @ 5a8ae7e3 (run 1 / run 6) | this PR @ 9951d916 (run 2 / run 5) | Δ (medians) | |---|---|---|---| | fps, average | 41.6 / 41.7 | 41.6 / 41.8 | +0.1% | | fps, 1% low | 32.1 / 32.2 | 32.7 / 31.8 | +0.3% | | frame time, median | 23.92 / 23.84 ms | 23.92 / 23.84 ms | 0.00 ms | | frame time, p99 | 29.48 / 29.54 ms | 29.27 / 29.36 ms | -0.19 ms | | replay submit a frame (Presentation thread) | 18.68 / 18.72 ms | 18.67 / 18.73 ms | 0.00 ms | | replay submit per 1000 draws | 7.10 / 7.10 ms | 7.08 / 7.06 ms | -0.03 ms (-0.4%) | | replay draw / descriptors (run 1 against run 2) | 10.43 / 2.76 ms | 10.26 / 2.56 ms | | | draws a frame | 2626 / 2637 | 2636 / 2636 | same scene | | GPU time a frame | 12.8 / 12.8 ms | 12.8 / 12.8 ms | none | A status line from each side, in the measured window: ``` main @ 5a8ae7e3 (run 1) [hos Presentation Thread] vk: 4920 presents, 41.6 fps; a frame: 2642 draws (0 skipped), 121 dispatches, 175 passes, 161 barriers, 6 submits, 0.0 uploads (0.0 MB), 82 ub updates (57 overlays), GPU-bound wait 0.1 ms [hos Presentation Thread] vk: a frame's replay time (ms): submit 18.42, present 0.07, draw 10.43, descriptors 2.76, texture lookups 0.11, uploads 0.00, pipelines 0.46, programs 1.35, commands 4.90, dispatch 1.13, memory sync 2.37, queue submit 1.55 [hos Presentation Thread] vk: GPU time 12.9 ms a frame (the longest submission 3.3 ms); 0.6 textures made a frame this PR @ 9951d916 (run 2) [hos Presentation Thread] vk: 4800 presents, 41.7 fps; a frame: 2646 draws (0 skipped), 121 dispatches, 175 passes, 161 barriers, 6 submits, 0.0 uploads (0.0 MB), 82 ub updates (57 overlays), GPU-bound wait 0.1 ms [hos Presentation Thread] vk: a frame's replay time (ms): submit 18.74, present 0.06, draw 10.26, descriptors 2.56, texture lookups 0.09, uploads 0.00, pipelines 0.38, programs 1.32, commands 5.05, dispatch 1.11, memory sync 2.73, queue submit 1.59 [hos Presentation Thread] vk: GPU time 12.9 ms a frame (the longest submission 3.3 ms); 0.0 textures made a frame ``` Also checked: `python -I -B tests/run_all.py` on this PR: 470 passed, 2 failed, 74 skipped. The two (`tests/test_menu_keys.py`, `tests/test_pad_layout.py`) fail on main @ 5a8ae7e3 on this PC too, with `tests/test_display_pointer.py` once more there. No device lost in any run.
mooki left a comment

Read head 9951d916 against main cf605c85: the complete diff, cache users and replay dispatch, all comments, and the earlier review with its two inline comments; the five added or changed test drivers and five related drivers pass here, with GCC and Clang for every compiled fixture. No game or renderer build was run.

What's good

  • Main still uses the four node-based maps and a variable-size payload copy, so the costs being addressed are real. The change stays within those paths and does not change settings, submission order or hardware selection.
  • I found no new correctness defect in the table or its callers. The texture lookup comparison exercises both map implementations, and the recording fixtures still pass without losing their existing assertions.

Earlier review

Against #231 (comment):

  • The pick reference: answered in gpu/vk/vk_shader.cpp, lines 1963-1966. The comment records the reference-lifetime condition, and the current program_pick path does not insert into picks.
  • The per-recording clear: answered in gpu/vk/flat_map.h, lines 49-55, and gpu/vk/vk_draw.cpp, lines 5158-5167. c1_at now uses the capacity-preserving reset, its peak is logged, and the differential test checks reset and refill. The remaining memory trade-off is disclosed under Risk.
  • Rebase: it was refreshed onto 5a8ae7e3, but main has moved again. The current merge conflict is only docs/tested-configurations.md; the three modified renderer sources merge automatically. This alone is not a reason to request changes.
  • Measurements: the standalone Deck comparison is now present. There is also a six-core Linux desktop comparison of the exact head at #231 (comment), with no material change in the Market. The old measured PC commit is said to be pushed, but I could not read dukenheimer/Longshot@53a83cca or the named fork through its public API. Please give a working full commit link so that part of the earlier review can be closed. The test-request link is still absent.

Must change

No additional code defect or failing test found.

Measurement and smaller points

  1. Move the added row at docs/tested-configurations.md, line 17, into separate run files under docs/tested/. Current main freezes the old table and requires a self-contained file per measured run. This also removes the present table conflict.
  2. Keep the gain tied to the measured PC commit and configuration. The MP165 table supports the arithmetic: medians of 68.75 and 72.1 fps give +3.35 fps, rounding to +3.4. The respective ranges are 67.5-70.0 and 72.0-72.2 fps. That is evidence for those two retained pairs, not for the current head on every CPU-bound PC. Add the CPU, driver, complete settings, run order and raw run links; keep the excluded, externally loaded run visible. M1152 still shows only one pair, despite the method saying two rounds each.
  3. Bring the description up to date with the new six-core desktop result. On the Deck, keep “no demonstrated gain”, rather than implying equivalence: both candidate runs have worse p95 and more slow frames, with more swapping, so the cause remains unresolved. The sentence saying descriptors are “within the base's range” is not literal: 6.24-7.01 ms extends outside 6.43-6.46 ms.
  4. The new tests against main stop at source-spelling checks, before compiling. That is not a behavioral failing-before result. test_replay_copies.py, lines 59-64, checks the copy form and the c1_at declaration; its executable checks copied bytes, not inlining or c1 upload/reuse. The old payload is expected to produce the same bytes. Describe that scope plainly, and use compiler output for the inlining claim.

Not checked

No game, full suite, Windows execution, visual comparison or long-session memory test here. I checked the published tables and quoted log lines, not raw captures or profiles. A working link to the measured PC source and its per-run evidence would settle the remaining performance provenance; the new Linux desktop result does not establish a gain elsewhere. The current source-check jobs are blocked, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3408.

Read head `9951d916` against main `cf605c85`: the complete diff, cache users and replay dispatch, all comments, and the earlier review with its two inline comments; the five added or changed test drivers and five related drivers pass here, with GCC and Clang for every compiled fixture. No game or renderer build was run. ### What's good - Main still uses the four node-based maps and a variable-size payload copy, so the costs being addressed are real. The change stays within those paths and does not change settings, submission order or hardware selection. - I found no new correctness defect in the table or its callers. The texture lookup comparison exercises both map implementations, and the recording fixtures still pass without losing their existing assertions. ### Earlier review Against https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/231#issuecomment-13935: - **The pick reference:** answered in `gpu/vk/vk_shader.cpp`, lines 1963-1966. The comment records the reference-lifetime condition, and the current `program_pick` path does not insert into `picks`. - **The per-recording clear:** answered in `gpu/vk/flat_map.h`, lines 49-55, and `gpu/vk/vk_draw.cpp`, lines 5158-5167. `c1_at` now uses the capacity-preserving reset, its peak is logged, and the differential test checks reset and refill. The remaining memory trade-off is disclosed under Risk. - **Rebase:** it was refreshed onto `5a8ae7e3`, but main has moved again. The current merge conflict is only `docs/tested-configurations.md`; the three modified renderer sources merge automatically. This alone is not a reason to request changes. - **Measurements:** the standalone Deck comparison is now present. There is also a six-core Linux desktop comparison of the exact head at https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/231#issuecomment-17272, with no material change in the Market. The old measured PC commit is said to be pushed, but I could not read https://git.magicbean.dev/dukenheimer/Longshot/commit/53a83cca or the named fork through its public API. Please give a working full commit link so that part of the earlier review can be closed. The test-request link is still absent. ### Must change No additional code defect or failing test found. ### Measurement and smaller points 1. Move the added row at `docs/tested-configurations.md`, line 17, into separate run files under `docs/tested/`. Current main freezes the old table and requires a self-contained file per measured run. This also removes the present table conflict. 2. Keep the gain tied to the measured PC commit and configuration. The MP165 table supports the arithmetic: medians of 68.75 and 72.1 fps give +3.35 fps, rounding to +3.4. The respective ranges are 67.5-70.0 and 72.0-72.2 fps. That is evidence for those two retained pairs, not for the current head on every CPU-bound PC. Add the CPU, driver, complete settings, run order and raw run links; keep the excluded, externally loaded run visible. M1152 still shows only one pair, despite the method saying two rounds each. 3. Bring the description up to date with the new six-core desktop result. On the Deck, keep “no demonstrated gain”, rather than implying equivalence: both candidate runs have worse p95 and more slow frames, with more swapping, so the cause remains unresolved. The sentence saying descriptors are “within the base's range” is not literal: 6.24-7.01 ms extends outside 6.43-6.46 ms. 4. The new tests against main stop at source-spelling checks, before compiling. That is not a behavioral failing-before result. `test_replay_copies.py`, lines 59-64, checks the copy form and the `c1_at` declaration; its executable checks copied bytes, not inlining or c1 upload/reuse. The old `payload` is expected to produce the same bytes. Describe that scope plainly, and use compiler output for the inlining claim. ### Not checked No game, full suite, Windows execution, visual comparison or long-session memory test here. I checked the published tables and quoted log lines, not raw captures or profiles. A working link to the measured PC source and its per-run evidence would settle the remaining performance provenance; the new Linux desktop result does not establish a gain elsewhere. The current source-check jobs are blocked, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3408.
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 8s
Website / Website on Cloudflare (pull_request_target) Has been skipped
This pull request has changes conflicting with the target branch.
  • docs/tested-configurations.md
Some workflows are waiting to be reviewed.
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.
git fetch -u 'opus/replay-flat-caches:dukenheimer-opus/replay-flat-caches'
git switch 'dukenheimer-opus/replay-flat-caches'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!231
No description provided.