perf(vk): drop the pipeline cache's files when another translator made them #2684

Closed
github-sync wants to merge 7 commits from github/pr/2684 into main
Owner

@SadTransGirl opened this pull request on GitHub · 2026-11-12 06:09 UTC · original
Merged on GitHub in f5c9acc15b

mkw_vk_pipelines.bin only ever grows: every translator update leaves its old pipelines in it, where they can never be hit again, and the whole file is read at every start and held in RAM by the driver. This drops the files when the translator that made them is not this one, as the program cache already does.

Cause. The pipeline cache file is checked against the GPU, the driver and the cache UUID only (pcache_file_read). A new translator gives every program new SPIR-V, so hookshot_programs.bin and the prepare record start over (program_cache_stamp), but the pipeline cache kept everything. In one test folder it reached 2.4 GB after a few dozen builds.

Change.

  • gpu/vk/pcache_stamp.h: mkw_vk_pipelines.stamp beside the cache holds the program_cache_stamp() its files were made for.
  • pcache_init: when the stamp differs, or there is none (a folder from before this), mkw_vk_pipelines.bin and the play files (mkw_vk_pipelines_play_<n>.bin, with their .tmp) are removed before anything reads them, and the stamp is written. Logged as "pipeline cache: N files made for another translator dropped (M MB)".
  • prepare_run: the warm pass keeps the stage libraries in that file (kWarmRecipe 2), so when files were dropped at this start the record's warmed is read as not made and the libraries are built again, once a run. Without this, a folder from before the stamp file with the same translator lost its libraries while the record said they were there, and fell back to the driver's slower cache until the next translator change.
  • The folder scan never throws at start: the iterator is stepped with an error_code, and a file name the code page cannot hold is skipped.
  • RX_VK_PCACHE_STAMP=0 keeps the files whatever made them, as before. RX_VK_PCACHE_LOAD=0 is unchanged.
  • docs/development.md: a row for the switch.

What it costs. The first start after a translator update builds its pipelines again. On NVIDIA the driver's own disk cache serves most of them; on AMD, which relies on the app's cache for compute pipelines, that start is slower once. The program cache already starts over at the same moment, so the translations are redone then anyway.

Tests. tests/test_pcache_stamp.py runs the header against a temporary folder (cache files dropped for another stamp or none, kept for the same one, other files untouched, a damaged stamp counted as none) and checks that pcache_init drops the files before it reads the cache and before play_files_merge, and that prepare_run warms again after a drop.

Measured. RTX 4070 SUPER, Windows 11, DLAA 2560x1440, a five-minute walk across Hyrule Field, the same build with and without the change, in a folder whose cache file had reached 2,364 MB: the first start dropped it ("1 files made for another translator dropped (2364 MB)"), the prepare made a new one of 681 MB for this translator, and the working set went from 18.7 GB to 17.6-17.7 GB on average (peak 19.3 to 18.3-18.5 GB). No pipeline waits, no draws left out. (Measured on main 15646d18; the branch is rebased onto edaef2a9.) On edaef2a9, with the stamp file removed from a folder of the same translator: the start dropped the 1,233 MB cache and logged that the driver's part was made again; the next start dropped nothing.

Not tested. AMD and Intel (the one-time rebuild after an update), Linux, a folder shared by two GPUs (the stamp includes the GPU's mesh and lane settings, as the program cache's does, so switching GPUs drops the cache as it drops the programs).

Part of #993.

🤖 Generated with Claude Code

> [@SadTransGirl](https://github.com/SadTransGirl) opened this pull request on GitHub · 2026-11-12 06:09 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2684) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/f5c9acc15bbadc88e32bdfb7c4e1eed782c1c0e4 `mkw_vk_pipelines.bin` only ever grows: every translator update leaves its old pipelines in it, where they can never be hit again, and the whole file is read at every start and held in RAM by the driver. This drops the files when the translator that made them is not this one, as the program cache already does. **Cause.** The pipeline cache file is checked against the GPU, the driver and the cache UUID only (`pcache_file_read`). A new translator gives every program new SPIR-V, so `hookshot_programs.bin` and the prepare record start over (`program_cache_stamp`), but the pipeline cache kept everything. In one test folder it reached 2.4 GB after a few dozen builds. **Change.** - `gpu/vk/pcache_stamp.h`: `mkw_vk_pipelines.stamp` beside the cache holds the `program_cache_stamp()` its files were made for. - `pcache_init`: when the stamp differs, or there is none (a folder from before this), `mkw_vk_pipelines.bin` and the play files (`mkw_vk_pipelines_play_<n>.bin`, with their `.tmp`) are removed before anything reads them, and the stamp is written. Logged as "pipeline cache: N files made for another translator dropped (M MB)". - `prepare_run`: the warm pass keeps the stage libraries in that file (`kWarmRecipe` 2), so when files were dropped at this start the record's `warmed` is read as not made and the libraries are built again, once a run. Without this, a folder from before the stamp file with the same translator lost its libraries while the record said they were there, and fell back to the driver's slower cache until the next translator change. - The folder scan never throws at start: the iterator is stepped with an `error_code`, and a file name the code page cannot hold is skipped. - `RX_VK_PCACHE_STAMP=0` keeps the files whatever made them, as before. `RX_VK_PCACHE_LOAD=0` is unchanged. - `docs/development.md`: a row for the switch. **What it costs.** The first start after a translator update builds its pipelines again. On NVIDIA the driver's own disk cache serves most of them; on AMD, which relies on the app's cache for compute pipelines, that start is slower once. The program cache already starts over at the same moment, so the translations are redone then anyway. **Tests.** `tests/test_pcache_stamp.py` runs the header against a temporary folder (cache files dropped for another stamp or none, kept for the same one, other files untouched, a damaged stamp counted as none) and checks that `pcache_init` drops the files before it reads the cache and before `play_files_merge`, and that `prepare_run` warms again after a drop. **Measured.** RTX 4070 SUPER, Windows 11, DLAA 2560x1440, a five-minute walk across Hyrule Field, the same build with and without the change, in a folder whose cache file had reached 2,364 MB: the first start dropped it ("1 files made for another translator dropped (2364 MB)"), the prepare made a new one of 681 MB for this translator, and the working set went from 18.7 GB to 17.6-17.7 GB on average (peak 19.3 to 18.3-18.5 GB). No pipeline waits, no draws left out. (Measured on main 15646d18; the branch is rebased onto edaef2a9.) On edaef2a9, with the stamp file removed from a folder of the same translator: the start dropped the 1,233 MB cache and logged that the driver's part was made again; the next start dropped nothing. **Not tested.** AMD and Intel (the one-time rebuild after an update), Linux, a folder shared by two GPUs (the stamp includes the GPU's mesh and lane settings, as the program cache's does, so switching GPUs drops the cache as it drops the programs). Part of #993. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:item:2684 -->
mkw_vk_pipelines.bin is checked against the GPU, the driver and the cache
UUID only. A new translator gives every program new SPIR-V, so the program
cache and the prepare record start over (program_cache_stamp), but the
pipeline cache kept every older translator's pipelines, which can no longer
be hit. It only grew: 2.4 GB after a few dozen builds in one folder, read
whole at every start and held by the driver.

The stamp the files were made for is now kept beside them in
mkw_vk_pipelines.stamp; at a start, files of another stamp (or of none: a
folder from before this) are dropped before they are read, the play files
with them. RX_VK_PCACHE_STAMP=0 keeps them as before.

The warm pass keeps the stage libraries in that file, so when files were
dropped prepare_run makes them again even where its record says they are
made (a folder from before the stamp file, with the same translator).

tests/test_pcache_stamp.py runs the header against a temporary folder and
checks that pcache_init drops the files before it reads them and that
prepare_run then warms again.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Author
Owner

@Arkex-ai commented on GitHub · 2026-11-12 06:49 UTC · original

I checked ed9c5ad8 alongside #2649 (096de9a5) in a private combined tree. There is one conflict, in pcache_init; keeping the stale-file check before the mapped read resolves it:

pcache_drop_stale();
auto data = off ? PipelineCacheFile{} : pcache_file_read(pcache_path);

All five focused checks pass on both Windows and Linux, including your stamp/order check and the mapped-file, save, play-cache and header-reader checks. Your preparation rewarm rule stays intact. This is source/helper validation, not a combined backend build or game run; neither public branch was changed.

> [@Arkex-ai](https://github.com/Arkex-ai) commented on GitHub · 2026-11-12 06:49 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2684#issuecomment-6816877124) I checked `ed9c5ad8` alongside #2649 (`096de9a5`) in a private combined tree. There is one conflict, in `pcache_init`; keeping the stale-file check before the mapped read resolves it: ```cpp pcache_drop_stale(); auto data = off ? PipelineCacheFile{} : pcache_file_read(pcache_path); ``` All five focused checks pass on both Windows and Linux, including your stamp/order check and the mapped-file, save, play-cache and header-reader checks. Your preparation rewarm rule stays intact. This is source/helper validation, not a combined backend build or game run; neither public branch was changed. <!-- github-sync:c:6816877124 -->
Author
Owner

@Joey-Magnus requested changes on GitHub · 2026-11-12 14:34 UTC · original

The idea is right and the measurement (2.4 GB file dropped, working set 18.7 GB down to 17.6 GB on the RTX 4070 SUPER) shows it. I merged this into current main and ran tests/test_pcache_stamp.py with the prepare/play-cache/env checks: 14 passed, and the header-read check was skipped for lack of Vulkan headers. The deletion code only removes mkw_vk_pipelines.bin, its .tmp and mkw_vk_pipelines_play_<n>.bin(.tmp) in the exe's folder, and nothing in it throws. That's what I'd want from code that deletes files at every start.

One repo rule needs fixing: gpu/vk/vk_draw.cpp:949 reads the new RX_VK_PCACHE_STAMP with getenv/atoi. Switches go through env_value_off from compat/hookshot_env.h (e.g. const char* e = getenv(...); return !e || !env_value_off(e);), so that off/false work the same way they do everywhere else. You moved RX_VK_PCACHE_LOAD into pcache_load_off(), so it's worth switching that one at the same time.

Also, #2649 conflicts in pcache_init, as Arkex-ai found. Calling pcache_drop_stale() before the mapped pcache_file_read resolves it, and whichever PR merges second needs that.

> [@Joey-Magnus](https://github.com/Joey-Magnus) requested changes on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2684#pullrequestreview-5876232583) The idea is right and the measurement (2.4 GB file dropped, working set 18.7 GB down to 17.6 GB on the RTX 4070 SUPER) shows it. I merged this into current main and ran `tests/test_pcache_stamp.py` with the prepare/play-cache/env checks: 14 passed, and the header-read check was skipped for lack of Vulkan headers. The deletion code only removes `mkw_vk_pipelines.bin`, its `.tmp` and `mkw_vk_pipelines_play_<n>.bin(.tmp)` in the exe's folder, and nothing in it throws. That's what I'd want from code that deletes files at every start. One repo rule needs fixing: `gpu/vk/vk_draw.cpp:949` reads the new `RX_VK_PCACHE_STAMP` with `getenv`/`atoi`. Switches go through `env_value_off` from `compat/hookshot_env.h` (e.g. `const char* e = getenv(...); return !e || !env_value_off(e);`), so that `off`/`false` work the same way they do everywhere else. You moved `RX_VK_PCACHE_LOAD` into `pcache_load_off()`, so it's worth switching that one at the same time. Also, #2649 conflicts in `pcache_init`, as Arkex-ai found. Calling `pcache_drop_stale()` before the mapped `pcache_file_read` resolves it, and whichever PR merges second needs that. <!-- github-sync:r:5876232583 -->
RX_VK_PCACHE_STAMP and RX_VK_PCACHE_LOAD were read with atoi, so off/false did not work as in the other switches.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Author
Owner

@SadTransGirl commented on GitHub · 2026-11-12 15:44 UTC · original

Done in c2cdb6ff: RX_VK_PCACHE_STAMP and RX_VK_PCACHE_LOAD both go through env_value_off now, so off/false/no work as in the other switches. test_pcache_stamp and test_env_on pass. On #2649: agreed. If this lands second I'll merge it with pcache_drop_stale() before the mapped pcache_file_read, as Arkex-ai showed.

> [@SadTransGirl](https://github.com/SadTransGirl) commented on GitHub · 2026-11-12 15:44 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2684#issuecomment-6828287481) Done in c2cdb6ff: `RX_VK_PCACHE_STAMP` and `RX_VK_PCACHE_LOAD` both go through `env_value_off` now, so `off`/`false`/`no` work as in the other switches. test_pcache_stamp and test_env_on pass. On #2649: agreed. If this lands second I'll merge it with `pcache_drop_stale()` before the mapped `pcache_file_read`, as Arkex-ai showed. <!-- github-sync:c:6828287481 -->
github-sync closed this pull request 2026-11-12 17:58:32 +00:00
Author
Owner

@Joey-Magnus approved on GitHub · 2026-11-12 17:02 UTC · original

c2cdb6ff fixes what I asked. RX_VK_PCACHE_STAMP is read as !e || !env_value_off(e) and RX_VK_PCACHE_LOAD as e && env_value_off(e), so 0, off, false and no behave as everywhere else, and test_env_on passes. Since my review, the rest is cd88e1c4 (tested-configurations rows for the runs in the description) and main merges. Both rows match the description: 2,364 MB dropped, working set 17.6-17.7 GB against 18.7 GB, and the next start dropped nothing.

It merges cleanly with current main. #2649 is still open, and you've said that if this lands second you'll call pcache_drop_stale() before the mapped pcache_file_read, which is the right resolution.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 17:02 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2684#pullrequestreview-5878652160) c2cdb6ff fixes what I asked. RX_VK_PCACHE_STAMP is read as `!e || !env_value_off(e)` and RX_VK_PCACHE_LOAD as `e && env_value_off(e)`, so 0, off, false and no behave as everywhere else, and test_env_on passes. Since my review, the rest is cd88e1c4 (tested-configurations rows for the runs in the description) and main merges. Both rows match the description: 2,364 MB dropped, working set 17.6-17.7 GB against 18.7 GB, and the next start dropped nothing. It merges cleanly with current main. #2649 is still open, and you've said that if this lands second you'll call pcache_drop_stale() before the mapped pcache_file_read, which is the right resolution. <!-- github-sync:r:5878652160 -->
Commenting is not possible because the repository is archived.
No description provided.