MagicBeanDev/Longshot

Watch

Fork 71

perf(vk): the shader caches' stamp leaves out vk_api.h, so a menu, input or audio change keeps every player's translated programs #370

Merged

HumanBean merged 1 commit from HumanBean/Longshot:perf/translator-stamp into main

2026-11-14 15:15:44 +00:00

HumanBean commented

2026-11-14 05:47:36 +00:00

Owner

Why

Every change to gpu/vk/vk_api.h, even a new menu, controls or audio option id that has nothing to do with shaders, threw away every player's translated programs and Vulkan pipeline cache at their next start: about 16,000 programs translated again (46.5 s on 12 workers of a Ryzen 9 7900X, plus 49 s of pipeline builds with a warm driver cache, far longer on a slow PC). Since v3.0.0-rc.3, 3 of the 7 commits that did this were menu rows alone.

What changed

stamp: new nx2/translator_files.py names the files the shader caches' translator stamp (LONGSHOT_SHADER_STAMP) is made of and computes it: every file under gpu/vk/{eden,sirit,compat}/, then vk_shader.cpp, vk_bank.cpp, vk_state.h, in the same order and with the same CRLF handling as before. vk_api.h is no longer stamped. Its docstring says why each file is in and why vk_api.h is out.
build: nx2/build.py makes -DLONGSHOT_SHADER_STAMP with translator_files.stamp(vk), replacing the inline hash. translator_stamp() in gpu/vk/vk_shader.cpp and the cache formats are unchanged.
one source of truth: nx2/bake.py, nx2/bake_mimo.py and nx2/bake_ds/bake_all.py each kept their own copy of the old formula. All three now call the module.
dev packs: scripts/dev_packs.py translator_changed() decides "prepares again" from the packed checkout's own nx2/translator_files.py (stamped()), or from a change to that module itself. A checkout from before the module uses the old file list.
checks: tests/test_shader_stamp_inputs.py rewritten: build.py uses the module; a vk_api.h edit or another renderer source keeps the stamp; an edit to each stamped file, a new recompiler file or a vk_state.h field changes it. tests/test_program_cache_format.py, tests/test_shader_stamp_cache.py (its harness execs build.py's Vulkan branch and now gets the module in its namespace), tests/test_dev_packs.py (the prepare rule on an authored tree, the old-checkout fallback) and tests/test_bake_skeleton.py (the appended-order trap, now also checked positively) were updated in the same PR.
docs: docs/development.md (what the stamp holds, under the switches' intro; the dev packs' Prepare rule), docs/BAKE.md, and the comment at the top of gpu/vk/vk_progcache.cpp.

Why taking `vk_api.h` out is safe

What decides a translated program's SPIR-V is the stamped sources, plus what translator_stamp() folds in at run time: the depth mode, the GPU's profile, the vendor fixes, the switches, sizeof(Shader::Info), sizeof(Program) and the C++ library. The guest code and NvnState are read through the host. From vk_api.h, the translator (vk_shader.cpp, vk_bank.cpp, eden, sirit, compat) uses only these MkwVkHost calls:

gpu_read, gpu_guest, guest_base, guest_size: they carry the game's memory. What they return is decided by host/, which was never stamped. A change to how they are called is a change to vk_shader.cpp or vk_bank.cpp, and those stay stamped.
depth_minus_w: its answer is already folded into translator_stamp() at run time.

None of those files uses any HOS_*, MkwVk* or MKWVK_* name (grep). The NVN types NvnState is built from come from gpu/render/rx_nvn.h. vk_api.h includes that header, but the stamp never covered it, before or after this PR. Program records hold no vk_api.h type: vk_progcache.cpp's kProgramCacheFormat and its static_asserts cover what a record holds.

vk_state.h stays stamped. vk_bank.cpp's driver banks and vk_shader.cpp read NvnState's prog, ubo, ssbo, tex, samp, img, vattr, nvattr, cs_push, grid and tex_gen, with their array sizes, and the seeds are translated from a default-constructed NvnState. A field that only the replay uses (cd906265's occlusion counters) therefore still moves the stamp. Splitting NvnState would be a refactor of the replay and is not in this PR.

Considered and left out: ignoring comment-only and whitespace-only edits in stamped files. Over the 127 commits measured below, only 2 were comment-only (f3fe1763, 9cffaeb3). A wrong C/C++ comment stripper could keep a stale cache, which is far worse than the waste it saves, so the stamp still hashes the files byte for byte.

Flags and settings

No switch, menu row or ini key. The existing build define the stamp reaches the renderer through, listed for completeness:

Flag	What it does	Default now	Recommendation	Helps on	Doesn't help on	Untested on
`LONGSHOT_SHADER_STAMP` (build define, set by `nx2/build.py`)	The translator's sources' hash, which `translator_stamp()` folds into the shader caches' stamp: now without `vk_api.h`	Always set by the build, as before	As is: no player-facing setting	Every PC: fewer full re-translations	None	None (a hash of sources)

Measurements

main: 0caf0618 (2026-11-14)
this PR: perf/translator-stamp @ 48bb6d8a
Hardware: none needed. The stamp is a hash of source files, the same on every PC (computed here on Linux, Python 3.14).
Scene: none (game-free)
Method: a script of my own, outside the repo, that reads each revision's files with jj file show -r and computes the old formula (main's nx2/build.py) and the new one (nx2/translator_files.py) for each commit and its parent. It was run twice, and both runs gave the same stamps. translator_files.stamp() on this PR's tree gives fcc354fdcc02cb5c, the new-rule value for main in the table below, so the table's new rule is the module's.
How to reproduce: python3 -I -c "import sys; sys.path.insert(0, 'nx2'); import translator_files as t; print(t.stamp('gpu/vk'))" at any commit with this PR's nx2/translator_files.py (copy it in for an older commit). Compare with the old block in main's nx2/build.py.

	main @ `0caf0618`	this PR @ `48bb6d8a`
Commits v3.0.0-rc.3..main that changed the stamp (each one re-translates every program for every player)	7 of 62	4 of 62
The 120 commits before rc.3 that changed main's stamp	120	68
This tree + a new `HOS_OPT_*` id in `vk_api.h`	fe6ea2e46d6a093e → e8cb7a79e6519769 (dropped)	fcc354fdcc02cb5c → fcc354fdcc02cb5c (kept)
This tree + a one-token edit in `vk_bank.cpp` (`seed_cs_control = nullptr` → `= {}`)	→ ca4e4c272052178f (dropped)	→ 549d51e791fb5a54 (dropped, as it must be)

Every commit since v3.0.0-rc.3 (0c74253d) that touched a stamped file, oldest first:

Commit	What	Files	Main's rule	This PR's rule
`cd906265`	occlusion queries (#197)	`vk_state.h`: new `NvnState` fields	changed	changed (a `vk_state.h` field)
`7315957c`	handle tables as descriptor arrays (#107)	eden texture pass, `vk_shader.cpp`	changed	changed (a translator change)
`f3673ee4`	Damage taken row (#138)	`vk_api.h`: `HOS_OPT_DAMAGE_TAKEN`	changed	kept
`f3fe1763`	occlusion query numbering (#260)	`vk_state.h`: comments only	changed	changed (comments still count)
`e65cb1e0`	hold-to-toggle (#137)	`vk_api.h`: `HOS_OPT_HOLD_TOGGLE`	changed	kept
`9ef17269`	audio Mono row (#136)	`vk_api.h`: `HOS_OPT_AUDIO_OUTPUT`	changed	kept
`57f4eaec`	slow movie / Intel compiler fault (#252)	`vk_shader.cpp`	changed	changed (a translator change)

commit    parent→commit, main's rule               parent→commit, this PR's rule
cd906265  603de7e1f6449dcc → 6be2881b1bb97f96       ed885a1db9fdc0b1 → 20441cedd70e88b9
7315957c  6be2881b1bb97f96 → 3619ac5dbee62271       20441cedd70e88b9 → 8e952d95d08071c4
f3673ee4  3619ac5dbee62271 → 15cd51c57d170515       8e952d95d08071c4 → 8e952d95d08071c4
f3fe1763  15cd51c57d170515 → b18601fc307df034       8e952d95d08071c4 → 1ea51441bd467219
e65cb1e0  b18601fc307df034 → d094ba0c30b740c4       1ea51441bd467219 → 1ea51441bd467219
9ef17269  d094ba0c30b740c4 → 1c549bb5db9d6a4f       1ea51441bd467219 → 1ea51441bd467219
57f4eaec  1c549bb5db9d6a4f → fe6ea2e46d6a093e       1ea51441bd467219 → fcc354fdcc02cb5c
main      fe6ea2e46d6a093e (cf605c85 and 0caf0618)   fcc354fdcc02cb5c (= translator_files.stamp() on this PR's tree)

Before rc.3, the 52 commits that would have kept the caches were all vk_api.h-only: menu rows, controls, prompts, motion, the microphone, the control bridge, the DLSS jitter ids. 9cffaeb3, a comment-only vk_state.h commit, still counts under the new rule.

For players on releases, v3.0.0-rc.3 to main still re-translates (7315957c and 57f4eaec change the translator). The gain is for nightlies and for future releases whose stamped changes are only menu, input or audio ids. This PR itself changes the stamp value once (fe6ea2e4 → fcc354fd, because vk_api.h leaves the hash), so the first build that carries it re-translates one last time.

Testing

Tested: the stamp table and the vk_api.h / vk_bank.cpp edits above (game-free, Linux). By reading: every check under tests/ that reads the code this PR changes (the build.py stamp block, the dev_packs list, the bake mirrors). No tests were run on the author's PC (not tests/run_all.py, not any single check). Forgejo's CI runs them.

Not tested: a game run. The game sees only the stamp's value, a constant compiled into the renderer, and the table shows that value for each tree. Not run: a dev_packs.py pack of a vk_api.h-only change, to show it builds without prepares and the game keeps longshot_programs.bin at start.

Needs other testers: None (the change is a source hash, the same on every GPU and OS)

Screenshots

Nothing on screen

Risk and rollback

The risk is a stale cache if the translator came to depend on something in vk_api.h: the programs would be kept from a translator that read it differently. Today it reads nothing there but the host calls above. A future change that makes the translator use a vk_api.h declaration must stamp it: add the header to FILES in nx2/translator_files.py, or move the declaration into a stamped file. tests/test_shader_stamp_inputs.py pins the list, so changing it means stating why. Rollback: revert the commit, which gives back the old stamp (and one more re-translation).

Reviewer notes

Start with nx2/translator_files.py (the list and the reasons), then scripts/dev_packs.py translator_changed. The tests under tests/ were updated without being run here: please check CI's run of tests/test_shader_stamp_inputs.py, test_shader_stamp_cache.py, test_program_cache_format.py, test_dev_packs.py, test_bake_skeleton.py and test_bake_ds.py.

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). No tests run on the author's PC; CI runs them.
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. (The stamp is deterministic: two runs, identical values.)
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 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. (The one C++ edit is a comment in gpu/vk/vk_progcache.cpp.)
Docs updated.

🤖 Generated with Claude Code

https://claude.ai/code/session_017vw8e34avQ5cTCWow16MXn

## Why Every change to `gpu/vk/vk_api.h`, even a new menu, controls or audio option id that has nothing to do with shaders, threw away every player's translated programs and Vulkan pipeline cache at their next start: about 16,000 programs translated again (46.5 s on 12 workers of a Ryzen 9 7900X, plus 49 s of pipeline builds with a warm driver cache, far longer on a slow PC). Since v3.0.0-rc.3, 3 of the 7 commits that did this were menu rows alone. ## What changed - **stamp**: new `nx2/translator_files.py` names the files the shader caches' translator stamp (`LONGSHOT_SHADER_STAMP`) is made of and computes it: every file under `gpu/vk/{eden,sirit,compat}/`, then `vk_shader.cpp`, `vk_bank.cpp`, `vk_state.h`, in the same order and with the same CRLF handling as before. **`vk_api.h` is no longer stamped.** Its docstring says why each file is in and why `vk_api.h` is out. - **build**: `nx2/build.py` makes `-DLONGSHOT_SHADER_STAMP` with `translator_files.stamp(vk)`, replacing the inline hash. `translator_stamp()` in `gpu/vk/vk_shader.cpp` and the cache formats are unchanged. - **one source of truth**: `nx2/bake.py`, `nx2/bake_mimo.py` and `nx2/bake_ds/bake_all.py` each kept their own copy of the old formula. All three now call the module. - **dev packs**: `scripts/dev_packs.py` `translator_changed()` decides "prepares again" from the packed checkout's own `nx2/translator_files.py` (`stamped()`), or from a change to that module itself. A checkout from before the module uses the old file list. - **checks**: `tests/test_shader_stamp_inputs.py` rewritten: build.py uses the module; a `vk_api.h` edit or another renderer source keeps the stamp; an edit to each stamped file, a new recompiler file or a `vk_state.h` field changes it. `tests/test_program_cache_format.py`, `tests/test_shader_stamp_cache.py` (its harness execs build.py's Vulkan branch and now gets the module in its namespace), `tests/test_dev_packs.py` (the prepare rule on an authored tree, the old-checkout fallback) and `tests/test_bake_skeleton.py` (the appended-order trap, now also checked positively) were updated in the same PR. - **docs**: `docs/development.md` (what the stamp holds, under the switches' intro; the dev packs' Prepare rule), `docs/BAKE.md`, and the comment at the top of `gpu/vk/vk_progcache.cpp`. ### Why taking `vk_api.h` out is safe What decides a translated program's SPIR-V is the stamped sources, plus what `translator_stamp()` folds in at run time: the depth mode, the GPU's profile, the vendor fixes, the switches, `sizeof(Shader::Info)`, `sizeof(Program)` and the C++ library. The guest code and `NvnState` are read through the host. From `vk_api.h`, the translator (`vk_shader.cpp`, `vk_bank.cpp`, eden, sirit, compat) uses only these `MkwVkHost` calls: - `gpu_read`, `gpu_guest`, `guest_base`, `guest_size`: they carry the game's memory. What they return is decided by `host/`, which was never stamped. A change to how they are called is a change to `vk_shader.cpp` or `vk_bank.cpp`, and those stay stamped. - `depth_minus_w`: its answer is already folded into `translator_stamp()` at run time. None of those files uses any `HOS_*`, `MkwVk*` or `MKWVK_*` name (grep). The NVN types `NvnState` is built from come from `gpu/render/rx_nvn.h`. `vk_api.h` includes that header, but the stamp never covered it, before or after this PR. Program records hold no `vk_api.h` type: `vk_progcache.cpp`'s `kProgramCacheFormat` and its static_asserts cover what a record holds. `vk_state.h` stays stamped. `vk_bank.cpp`'s driver banks and `vk_shader.cpp` read `NvnState`'s `prog`, `ubo`, `ssbo`, `tex`, `samp`, `img`, `vattr`, `nvattr`, `cs_push`, `grid` and `tex_gen`, with their array sizes, and the seeds are translated from a default-constructed `NvnState`. A field that only the replay uses (cd906265's occlusion counters) therefore still moves the stamp. Splitting `NvnState` would be a refactor of the replay and is not in this PR. **Considered and left out:** ignoring comment-only and whitespace-only edits in stamped files. Over the 127 commits measured below, only 2 were comment-only (f3fe1763, 9cffaeb3). A wrong C/C++ comment stripper could keep a stale cache, which is far worse than the waste it saves, so the stamp still hashes the files byte for byte. ## Flags and settings No switch, menu row or ini key. The existing build define the stamp reaches the renderer through, listed for completeness: | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `LONGSHOT_SHADER_STAMP` (build define, set by `nx2/build.py`) | The translator's sources' hash, which `translator_stamp()` folds into the shader caches' stamp: now without `vk_api.h` | Always set by the build, as before | As is: no player-facing setting | Every PC: fewer full re-translations | None | None (a hash of sources) | ## Measurements main: 0caf0618 (2026-11-14) this PR: perf/translator-stamp @ 48bb6d8a Hardware: none needed. The stamp is a hash of source files, the same on every PC (computed here on Linux, Python 3.14). Scene: none (game-free) Method: a script of my own, outside the repo, that reads each revision's files with `jj file show -r` and computes the old formula (main's `nx2/build.py`) and the new one (`nx2/translator_files.py`) for each commit and its parent. It was run twice, and both runs gave the same stamps. `translator_files.stamp()` on this PR's tree gives `fcc354fdcc02cb5c`, the new-rule value for main in the table below, so the table's new rule is the module's. How to reproduce: `python3 -I -c "import sys; sys.path.insert(0, 'nx2'); import translator_files as t; print(t.stamp('gpu/vk'))"` at any commit with this PR's `nx2/translator_files.py` (copy it in for an older commit). Compare with the old block in main's `nx2/build.py`. | | main @ 0caf0618 | this PR @ 48bb6d8a | |---|---|---| | Commits v3.0.0-rc.3..main that changed the stamp (each one re-translates every program for every player) | 7 of 62 | 4 of 62 | | The 120 commits before rc.3 that changed main's stamp | 120 | 68 | | This tree + a new `HOS_OPT_*` id in `vk_api.h` | fe6ea2e46d6a093e → e8cb7a79e6519769 (dropped) | fcc354fdcc02cb5c → fcc354fdcc02cb5c (kept) | | This tree + a one-token edit in `vk_bank.cpp` (`seed_cs_control = nullptr` → `= {}`) | → ca4e4c272052178f (dropped) | → 549d51e791fb5a54 (dropped, as it must be) | Every commit since v3.0.0-rc.3 (0c74253d) that touched a stamped file, oldest first: | Commit | What | Files | Main's rule | This PR's rule | |---|---|---|---|---| | cd906265 | occlusion queries ([#197](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/197)) | `vk_state.h`: new `NvnState` fields | changed | changed (a `vk_state.h` field) | | 7315957c | handle tables as descriptor arrays ([#107](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/107)) | eden texture pass, `vk_shader.cpp` | changed | changed (a translator change) | | f3673ee4 | Damage taken row ([#138](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/138)) | `vk_api.h`: `HOS_OPT_DAMAGE_TAKEN` | changed | **kept** | | f3fe1763 | occlusion query numbering ([#260](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/260)) | `vk_state.h`: comments only | changed | changed (comments still count) | | e65cb1e0 | hold-to-toggle ([#137](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/137)) | `vk_api.h`: `HOS_OPT_HOLD_TOGGLE` | changed | **kept** | | 9ef17269 | audio Mono row ([#136](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/136)) | `vk_api.h`: `HOS_OPT_AUDIO_OUTPUT` | changed | **kept** | | 57f4eaec | slow movie / Intel compiler fault ([#252](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/252)) | `vk_shader.cpp` | changed | changed (a translator change) | ``` commit parent→commit, main's rule parent→commit, this PR's rule cd906265 603de7e1f6449dcc → 6be2881b1bb97f96 ed885a1db9fdc0b1 → 20441cedd70e88b9 7315957c 6be2881b1bb97f96 → 3619ac5dbee62271 20441cedd70e88b9 → 8e952d95d08071c4 f3673ee4 3619ac5dbee62271 → 15cd51c57d170515 8e952d95d08071c4 → 8e952d95d08071c4 f3fe1763 15cd51c57d170515 → b18601fc307df034 8e952d95d08071c4 → 1ea51441bd467219 e65cb1e0 b18601fc307df034 → d094ba0c30b740c4 1ea51441bd467219 → 1ea51441bd467219 9ef17269 d094ba0c30b740c4 → 1c549bb5db9d6a4f 1ea51441bd467219 → 1ea51441bd467219 57f4eaec 1c549bb5db9d6a4f → fe6ea2e46d6a093e 1ea51441bd467219 → fcc354fdcc02cb5c main fe6ea2e46d6a093e (cf605c85 and 0caf0618) fcc354fdcc02cb5c (= translator_files.stamp() on this PR's tree) ``` Before rc.3, the 52 commits that would have kept the caches were all `vk_api.h`-only: menu rows, controls, prompts, motion, the microphone, the control bridge, the DLSS jitter ids. 9cffaeb3, a comment-only `vk_state.h` commit, still counts under the new rule. For players on releases, v3.0.0-rc.3 to main still re-translates (7315957c and 57f4eaec change the translator). The gain is for nightlies and for future releases whose stamped changes are only menu, input or audio ids. **This PR itself changes the stamp value once** (fe6ea2e4 → fcc354fd, because `vk_api.h` leaves the hash), so the first build that carries it re-translates one last time. ## Testing **Tested:** the stamp table and the `vk_api.h` / `vk_bank.cpp` edits above (game-free, Linux). By reading: every check under `tests/` that reads the code this PR changes (the build.py stamp block, the dev_packs list, the bake mirrors). **No tests were run on the author's PC** (not `tests/run_all.py`, not any single check). Forgejo's CI runs them. **Not tested:** a game run. The game sees only the stamp's value, a constant compiled into the renderer, and the table shows that value for each tree. Not run: a `dev_packs.py pack` of a `vk_api.h`-only change, to show it builds without `prepares` and the game keeps `longshot_programs.bin` at start. **Needs other testers:** None (the change is a source hash, the same on every GPU and OS) ## Screenshots Nothing on screen ## Risk and rollback The risk is a stale cache if the translator came to depend on something in `vk_api.h`: the programs would be kept from a translator that read it differently. Today it reads nothing there but the host calls above. A future change that makes the translator use a `vk_api.h` declaration must stamp it: add the header to `FILES` in `nx2/translator_files.py`, or move the declaration into a stamped file. `tests/test_shader_stamp_inputs.py` pins the list, so changing it means stating why. Rollback: revert the commit, which gives back the old stamp (and one more re-translation). ## Reviewer notes Start with `nx2/translator_files.py` (the list and the reasons), then `scripts/dev_packs.py` `translator_changed`. The tests under `tests/` were updated without being run here: please check CI's run of `tests/test_shader_stamp_inputs.py`, `test_shader_stamp_cache.py`, `test_program_cache_format.py`, `test_dev_packs.py`, `test_bake_skeleton.py` and `test_bake_ds.py`. ## Issues None. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). No tests run on the author's PC; CI runs them. - [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. (The stamp is deterministic: two runs, identical values.) - [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 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. (The one C++ edit is a comment in `gpu/vk/vk_progcache.cpp`.) - [x] Docs updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017vw8e34avQ5cTCWow16MXn

HumanBean added 1 commit

2026-11-14 05:47:37 +00:00

perf(vk): the shader caches' stamp leaves out vk_api.h, so a menu, input or audio change keeps every player's translated programs

CI / Workflow lint (actionlint) (pull_request) Successful in 14s

Details

Third-party parts / As their notes say (pull_request) Successful in 19s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

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

Details

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

Details

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

Details

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

Details

CI / Conventional Commit title (pull_request) Successful in 9s

Details

PR description / Description follows the template (pull_request_target) Failing after 16s

Details

48bb6d8a18

The translator stamp (LONGSHOT_SHADER_STAMP) hashed gpu/vk/vk_api.h whole, though the
translator reads nothing from it but the host's memory and depth mode through MkwVkHost:
every Insert menu, controls or audio row added there dropped the program cache, the
prepare record and the Vulkan pipeline cache for every player. Of the 7 commits since
v3.0.0-rc.3 that moved the stamp, 3 were vk_api.h's option ids alone; of the 120 before
that, 52.

nx2/translator_files.py now names the stamped files (gpu/vk/{eden,sirit,compat}/,
vk_shader.cpp, vk_bank.cpp, vk_state.h) and makes the stamp; nx2/build.py, the bake
tools and scripts/dev_packs.py's prepare rule all read it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_017vw8e34avQ5cTCWow16MXn

HumanBean commented

2026-11-14 05:48:05 +00:00

Author

Owner

Description: Needs other testers written as None, the LONGSHOT_SHADER_STAMP build define listed under Flags, and the host-code box added for the one C++ edit (a comment in vk_progcache.cpp).

mooki added the

labels

2026-11-14 06:29:03 +00:00

mooki reviewed

2026-11-14 06:41:18 +00:00

mooki left a comment

Read head 48bb6d8a against main 0caf0618: the complete diff, description, sole comment, empty reviews, stamp producers and cache consumers; ran the changed stamp and pack checks, selected bake checks, and cache-record and prepare checks with GCC and Clang. Results and the bake exclusions are below. No game, full build or timing run.

What's good

The cost is real. Main hashes vk_api.h into the value that rejects translated programs, preparation records and pipeline-cache files. The history from 0c74253d to 0caf0618 contains 62 commits and 7 changes to the old input set, versus 4 to the new set. The 3 excluded changes only add Damage taken, Hold-to-toggle and Audio output declarations to this header.
The shared formula keeps relative filenames, file contents, CRLF normalization and the appended-file order. vk_state.h stays included, including its defaults and array sizes. The build, all three bake paths and the development-pack prepare decision consume the new definition.
Removing the header from the shader-cache stamp does not remove it from compilation dependencies. nx2/build.py, lines 1102 and 1211-1212, still makes host and renderer objects depend on it. The executable and renderer also retain their API-version checks.

Cache safety

I found no current shader-affecting declaration that this change silently loses, but the distinction is narrower than saying the translator reads nothing from this header:

Option IDs, input/prompt enums, pointer/settings/jitter structures and the window interface are not serialized shader records. Adding a menu or audio ID does not itself change SPIR-V. Graphics options can still change the work requested: the tone mapper has its own program key, texture/image answers select variants, and target formats and fixed draw state enter pipeline keys (gpu/vk/vk_shader.cpp, lines 968-1001 and 1089-1112; gpu/vk/vk_draw.cpp, lines 2264-2289).
The host callbacks do supply translation inputs. gpu_read supplies shader/control/constant bytes; gpu_guest participates in pointer classification; guest_base and guest_size help validate registration pointers. Their implementations are outside this header. Shader/control bytes are hashed into program identity, and texture/image answers are checked before reuse. depth_minus_w directly changes emitted depth conversion and remains explicitly included in translator_stamp() at lines 1246-1247. These checks should not be described as hashing every possible callback result.
Other guards remain, with distinct jobs. translator_stamp() retains the mesh mode, subgroup/alignment settings, float capabilities, vendor/matrix settings, environment switches and record ABI/library values. program_cache_stamp() adds the cache format; file and index readers require that stamp. Pipeline-cache loading still checks the vendor, device and Vulkan cache UUID, and the warm-pass stamp additionally includes driver version and recipe. The driver receives current shader modules and pipeline creation parameters; its opaque cache is not a substitute for validating Longshot's translated-program cache.
The API-version check is not a persistent-cache version. It rejects mismatched executable/renderer interfaces, but two newly rebuilt, matching binaries can still read older caches. If a future header or callback-contract change alters translation semantics without changing the existing keys, it needs explicit stamp coverage. Likewise, rx_nvn.h is included through this header but was not hashed transitively before this PR either. Neither point is a newly introduced defect at this head.

Must change

No functional defect found in this patch. It merges cleanly with main 0caf0618.

Measurements and smaller points

Correct the CI claim before taking this out of WIP. The description says the checks were left to CI, but the source-check and SPIR-V jobs on this head are skipped, not passed: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/4397. Current CONTRIBUTING.md also says the full checks are not automatic. Record actual local or manually started results, including Windows coverage.
Keep the historical hash result separate from a startup-time saving. The 7-to-4 history comparison supports fewer invalidations. Repeating a deterministic hash is valid evidence for that property, not an interleaved timing comparison. The quoted 46.5 s translation and 49 s pipeline costs have no paired before/after logs, median or spread here. To quantify the saving, compare a populated-cache update containing only a header option change on named builds, on the same PC, with at least two alternating runs per side. Also keep the disclosed one-time invalidation when this new formula first ships.
Add a direct header-edit case to the existing build-key check. tests/test_shader_stamp_inputs.py proves the new membership rule on authored bytes, and test_shader_stamp_cache.py executes the real job-construction branch. A case that edits only vk_api.h and asserts both an unchanged shader stamp and changed host/renderer object keys would pin the important distinction above. The current checks do not prove cache reuse in a running game or that arbitrary future additions to the excluded header are safe.

Overlaps

#357 and #358 are already in this base. Their lift-worker and prompt-generation changes do not supply a missing dependency here.
#30 and #37 also change nx2/build.py; the latter also changes tests/test_shader_stamp_cache.py. Both merge pairwise with this head. Preserve the shared stamp import and its test namespace when combining the dependency-tracking change.
#31 and #38 change the lift, not this shader identity, and merge pairwise. #360 has a documentation conflict at the Build switches table: its added ROMFS-reuse row meets the already-merged lift-worker wording from 357, not this PR's stamp logic.
#371 overlaps the build and development-pack files; #372 builds on 371. Both merge pairwise with this head. Neither is required by 370, which introduces no shared object or lift store. No combined-stack build was run.

Checks and remaining evidence

Passed here: test_shader_stamp_inputs, test_shader_stamp_cache (10 tests), test_program_cache_format, test_dev_packs, test_program_cache_index (5 tests), and test_prepared_stamps and test_program_record_head with both GCC and Clang. test_bake_skeleton passed its 6 selected BakeSkeletonTests and BakeStampTrapTests; test_bake_ds passed enumeration, parity and key checks and explicitly skipped native translation/bank checks because no offline executable was supplied.

Not run: the full suite, full bake translation/verification, test_bake_mimo, Windows execution, real cache-file reuse across an update, simultaneous game processes, or clean/incremental game builds. The stamp helper has no persisted state or shared output; it hashes the files afresh. Existing program-cache locking, partial-record recovery and development-pack serialization are unchanged. The decisive remaining runtime check is a warm-cache header-only update that keeps translated programs, followed by a translator or depth-mode change that rejects them, on Linux and Windows, with retained startup logs and unchanged rendered output.

Read head `48bb6d8a` against main `0caf0618`: the complete diff, description, sole comment, empty reviews, stamp producers and cache consumers; ran the changed stamp and pack checks, selected bake checks, and cache-record and prepare checks with GCC and Clang. Results and the bake exclusions are below. No game, full build or timing run. ### What's good - The cost is real. Main hashes `vk_api.h` into the value that rejects translated programs, preparation records and pipeline-cache files. The history from `0c74253d` to `0caf0618` contains 62 commits and 7 changes to the old input set, versus 4 to the new set. The 3 excluded changes only add Damage taken, Hold-to-toggle and Audio output declarations to this header. - The shared formula keeps relative filenames, file contents, CRLF normalization and the appended-file order. `vk_state.h` stays included, including its defaults and array sizes. The build, all three bake paths and the development-pack prepare decision consume the new definition. - Removing the header from the shader-cache stamp does not remove it from compilation dependencies. `nx2/build.py`, lines 1102 and 1211-1212, still makes host and renderer objects depend on it. The executable and renderer also retain their API-version checks. ### Cache safety I found no current shader-affecting declaration that this change silently loses, but the distinction is narrower than saying the translator reads nothing from this header: - **Option IDs, input/prompt enums, pointer/settings/jitter structures and the window interface are not serialized shader records.** Adding a menu or audio ID does not itself change SPIR-V. Graphics options can still change the work requested: the tone mapper has its own program key, texture/image answers select variants, and target formats and fixed draw state enter pipeline keys (`gpu/vk/vk_shader.cpp`, lines 968-1001 and 1089-1112; `gpu/vk/vk_draw.cpp`, lines 2264-2289). - **The host callbacks do supply translation inputs.** `gpu_read` supplies shader/control/constant bytes; `gpu_guest` participates in pointer classification; `guest_base` and `guest_size` help validate registration pointers. Their implementations are outside this header. Shader/control bytes are hashed into program identity, and texture/image answers are checked before reuse. `depth_minus_w` directly changes emitted depth conversion and remains explicitly included in `translator_stamp()` at lines 1246-1247. These checks should not be described as hashing every possible callback result. - **Other guards remain, with distinct jobs.** `translator_stamp()` retains the mesh mode, subgroup/alignment settings, float capabilities, vendor/matrix settings, environment switches and record ABI/library values. `program_cache_stamp()` adds the cache format; file and index readers require that stamp. Pipeline-cache loading still checks the vendor, device and Vulkan cache UUID, and the warm-pass stamp additionally includes driver version and recipe. The driver receives current shader modules and pipeline creation parameters; its opaque cache is not a substitute for validating Longshot's translated-program cache. - **The API-version check is not a persistent-cache version.** It rejects mismatched executable/renderer interfaces, but two newly rebuilt, matching binaries can still read older caches. If a future header or callback-contract change alters translation semantics without changing the existing keys, it needs explicit stamp coverage. Likewise, `rx_nvn.h` is included through this header but was not hashed transitively before this PR either. Neither point is a newly introduced defect at this head. ### Must change No functional defect found in this patch. It merges cleanly with main `0caf0618`. ### Measurements and smaller points 1. **Correct the CI claim before taking this out of WIP.** The description says the checks were left to CI, but the source-check and SPIR-V jobs on this head are skipped, not passed: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/4397. Current `CONTRIBUTING.md` also says the full checks are not automatic. Record actual local or manually started results, including Windows coverage. 2. **Keep the historical hash result separate from a startup-time saving.** The 7-to-4 history comparison supports fewer invalidations. Repeating a deterministic hash is valid evidence for that property, not an interleaved timing comparison. The quoted 46.5 s translation and 49 s pipeline costs have no paired before/after logs, median or spread here. To quantify the saving, compare a populated-cache update containing only a header option change on named builds, on the same PC, with at least two alternating runs per side. Also keep the disclosed one-time invalidation when this new formula first ships. 3. **Add a direct header-edit case to the existing build-key check.** `tests/test_shader_stamp_inputs.py` proves the new membership rule on authored bytes, and `test_shader_stamp_cache.py` executes the real job-construction branch. A case that edits only `vk_api.h` and asserts both an unchanged shader stamp and changed host/renderer object keys would pin the important distinction above. The current checks do not prove cache reuse in a running game or that arbitrary future additions to the excluded header are safe. ### Overlaps - https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/357 and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/358 are already in this base. Their lift-worker and prompt-generation changes do not supply a missing dependency here. - https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/30 and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/37 also change `nx2/build.py`; the latter also changes `tests/test_shader_stamp_cache.py`. Both merge pairwise with this head. Preserve the shared stamp import and its test namespace when combining the dependency-tracking change. - https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/31 and https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/38 change the lift, not this shader identity, and merge pairwise. https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/360 has a documentation conflict at the Build switches table: its added ROMFS-reuse row meets the already-merged lift-worker wording from 357, not this PR's stamp logic. - https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/371 overlaps the build and development-pack files; https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/372 builds on 371. Both merge pairwise with this head. Neither is required by 370, which introduces no shared object or lift store. No combined-stack build was run. ### Checks and remaining evidence Passed here: `test_shader_stamp_inputs`, `test_shader_stamp_cache` (10 tests), `test_program_cache_format`, `test_dev_packs`, `test_program_cache_index` (5 tests), and `test_prepared_stamps` and `test_program_record_head` with both GCC and Clang. `test_bake_skeleton` passed its 6 selected `BakeSkeletonTests` and `BakeStampTrapTests`; `test_bake_ds` passed enumeration, parity and key checks and explicitly skipped native translation/bank checks because no offline executable was supplied. Not run: the full suite, full bake translation/verification, `test_bake_mimo`, Windows execution, real cache-file reuse across an update, simultaneous game processes, or clean/incremental game builds. The stamp helper has no persisted state or shared output; it hashes the files afresh. Existing program-cache locking, partial-record recovery and development-pack serialization are unchanged. The decisive remaining runtime check is a warm-cache header-only update that keeps translated programs, followed by a translator or depth-mode change that rejects them, on Linux and Windows, with retained startup logs and unchanged rendered output.

HumanBean changed title from ~~WIP: perf(vk): the shader caches' stamp leaves out vk_api.h, so a menu, input or audio change keeps every player's translated programs~~ to perf(vk): the shader caches' stamp leaves out vk_api.h, so a menu, input or audio change keeps every player's translated programs

2026-11-14 15:15:26 +00:00

HumanBean merged commit 26a2eab003 into main

2026-11-14 15:15:44 +00:00

HumanBean referenced this pull request from a commit

2026-11-14 15:15:44 +00:00

perf(vk): the shader caches' stamp leaves out vk_api.h, so a menu, input or audio change keeps every player's translated programs (#370)

HumanBean deleted branch perf/translator-stamp

2026-11-14 15:15:47 +00:00

mooki referenced this pull request

2026-11-14 16:11:54 +00:00

WIP: build(macos): classify Darwin helpers and package a relocatable macOS 14.4 dylib set #479

mooki referenced this pull request