perf(vk): keep program stages' SPIR-V packed, make shader modules only per pipeline (~1.5 GB less RAM on NVIDIA) #2309

github-sync · 2026-11-11T14:16:32Z

DUDUYG commented

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

(Migrated from github.com)

The problem

Every translated program keeps a VkShaderModule for each of its stages for as long as the program lives.

With the shipped state lists, a start keeps about 12,000 programs, which is about 20,400 modules.
NVIDIA's driver holds about 99 KB of its own memory for each module. That is about 2 GB of the game's private memory, in driver arenas full of SPIR-V copies.
A module destroyed later gives none of that memory back.

Measured on this PC in play, before this PR:

the game's working set peaked at 15.05 GB;
the system had 1.31 GB available at the lowest point, and paged.

In the memory map, two of the driver's ~1 GB allocations held 11,970 and 8,430 SPIR-V headers.

Microbenchmark with the game's own 20,400 modules (1636 MB of SPIR-V), driver 621.14:

	private memory
all made and kept alive	+1969 MB
then all destroyed	+1969 MB (nothing given back)
made and destroyed one at a time	+8 MB (0.025 ms each)

The fix

Stages keep their SPIR-V packed. Each stage keeps its SPIR-V packed with zstd in Stage::code, and Stage::module is gone.

zstd level 1 packs the game's SPIR-V 3.77 times smaller: 1636 MB becomes 434 MB.
Packing costs 0.137 ms per module, unpacking 0.054 ms.

Modules exist only while a pipeline is made. A StageModule unpacks the code and makes the module just for the pipeline being made, then destroys it.

The new code, stage_code() and StageModule, is in gpu/vk/vk_stage_code.cpp.

Where the modules are made now:

build_graphics makes only the modules of the stages that the pipeline, or the library part, takes.
Each compute path makes its module for its one create call:
- the mesh fallback;
- the prepare's warm;
- the precompile;
- the dispatch's ask under FAIL_ON_PIPELINE_COMPILE_REQUIRED;
- its queued build;
- its own compile.
A module that is not made leaves that pipeline unmade, as a failed compile does. It is logged.

Destroys removed. The module destroys are gone from:

program cache reads (the refused records);
program_cache_borrow;
the seed variants;
seed_variant_of;
the state list and the prepare.

The code now belongs to the program.

zstd.

third_party/zstd/lib/compress/ comes from the same zstd 1.5.7 release that is already vendored. It is unmodified, BSD, and has the same tarball SHA-256 as recorded in PROVENANCE.md.
It is built single-threaded. Note that -DZSTD_MULTITHREAD=0 was removed from nx2/build.py: zstd tests it with #ifdef, so defining it as 0 turned multithreading on.
zstdmt_compress.c is vendored with the rest of the folder but not built.

⚠️ Costs and things to know before merging

Every player translates the program cache once more.
- gpu/vk/vk_shader.cpp is part of the translator stamp, so the first start after updating translates all programs again.
- Here that prepare took 211 s for 38,809 programs (Ryzen 7 9800X3D); slower CPUs take longer.
- The SPIR-V it produces is byte-identical to before: hookshot_programs.bin compared record by record, 98,200 of 98,200 modules the same. So nothing in the driver's caches goes stale.
- I put the new code in its own file to keep later changes to it out of the stamp. The changes to vk_shader.cpp itself (removing make_module and the destroys) could not be avoided.
Only NVIDIA was measured (RTX 5070, driver 621.14, Windows 11).
- AMD, Intel and Linux were not run.
- Making a module per pipeline should be cheap on other drivers too: on RADV and ANV a module is essentially a copy of the SPIR-V. I have not measured it.
- The Linux build was not compiled locally. The zstd encoder files get -fPIC like the decoder's.
Translation and loading do a little more work.
- Full translation in the build's prepare: 211.0 s here, against 185.1 s for the previous build's full translation. That is one run each, so part of the gap may be noise.
- Each stage is now packed as the program cache is read, on up to 16 threads. The precompile's "caches opened, notes and programs read" part measured 3.7 s against 1.6 s in the runs without the driver's cache, and 0.8 s against 1.6 s in the runs with it.
Precompile time:

precompile, 15,456 graphics + 1,182 compute pipelines before this PR

NVIDIA's disk cache present (the normal case) 6.7 s 6.8 s

NVIDIA's disk cache moved aside 176.8 s 185.9 s

These are not a clean A/B: "before" ran from a copy on an NVMe disk and "this PR" from the SATA SSD. Both used the same mkw_vk_pipelines.bin contents and the same driver cache.
The recorder. vk_stage_code.cpp does not include vk_work.h, so its module destroys are no recorder boundary: no recorded command uses a shader module. It is listed in test_recorder_sync's BOUNDARY_OK with that reason, like vk_shader.cpp's destroys before it. So RX_VK_RECORDER=defer and =thread neither drain nor wait for them, also when the dispatch's ask makes and destroys a module on the replay thread.
Overlap with #1630 (compressed program cache, open).
- Both PRs change the zstd block of nx2/build.py, and tests/program_cache_cleanup_test.cpp.
- #1630 builds third_party/zbic's single-file zstd into the game. That file's PROVENANCE.md says it carries GPLv2 parts from Atmosphere-NX and is not linked into the game. This PR instead vendors the official encoder next to the decoder already used.
- Whichever lands second needs a rebase, and I'll adapt this one to whatever you prefer.

Measured

Test setup:

RTX 5070 (driver 621.14), Ryzen 7 9800X3D, 31 GB RAM, Windows 11.
The game is on a SATA SSD. The test harness's files and the game's log are on another disk.
Scripted headless route: load a save, then walk outdoors on the Kokiri path. "In play" means from present 3300 on.
The caches are in their normal state, after --prepare-shaders.
The build measured is this commit on top of a main from the same day, plus open PRs #2198, #2222 and #2193, plus local logging (present times, memory map). The comparison build is the same thing without this commit.

One run each:

	before	this PR
game's largest working set	15.05 GB	13.51 GB
system memory available, lowest	1.31 GB	2.67 GB
game's private memory in RAM (80 s)	7.59 GB	6.07 GB
commit charge (80 s)	26.37 GB	24.47 GB
the game's private allocations of 0.6 GB and over (80 s)	6, 6.86 GB	5, 4.70 GB
SPIR-V headers in the three largest	20,400	2
system page reads during the run	1.41 M	1.34 M
mean frame time, in play	17.14 ms	17.18 ms
frame time p99	24.64 ms	24.52 ms
1% low	37.5 fps	36.7 fps
frames over 33.4 ms	0	1
worst frame	31.8 ms	35.6 ms
frames over 20 ms	157	236
time lost against 60 fps, in play	886 ms	1318 ms

About the frames over 20 ms:

This PR's run had more of them, and they cluster in the same parts of the route as before's.
The replay's own times are the same in both runs: "pipelines" is 0.15 ms a frame, and there are 0 waits for shaders or pipelines.
On this machine, identical configurations have differed by as much between runs (56 against 95 frames over 25 ms).
With one run each, I cannot rule this out as a cost.

A discarded run. A first run of this PR (not in the table) had 10 frames over 33 ms, the worst 102 ms.

Every one of them lined up with a read stall on the game's SATA disk: 168 reads over 10 ms in play, against 0 in the before run.
Just before it, I had read and written about 20 GB on that disk: three prepares, and comparing the program caches.
I waited for the disk to settle (idle probe: median 0.44 ms, none over 10 ms) and ran it again. That second run is the table.

Tests

tests/program_cache_cleanup_test.cpp and tests/test_program_cache_cleanup.py now check which program cache records are taken and that each taken stage keeps its code whole, through the production loader, with a stage_code stand-in that fails when told to. They used to count module lifetimes. Cases: first or second stage not kept, truncated, empty, answers past the stages or slots, valid, mixed.
tests/test_precompile_compute_guard.py's fixture has a StageModule stand-in. The module must be alive during the create call and gone after it.
tests/test_mesh_fallback_failed_once.py finds the failure block after the compile, which may now also be the module's.
tests/test_shader_stamp_cache.py and tests/test_dlss_sdk_cache.py know the zstd encoder sources and zstd_users.
tests/test_recorder_sync.py lists vk_stage_code.cpp in BOUNDARY_OK (its module destroys), and vk_shader.cpp's reason no longer names modules. The first CI run failed here: my local run had the new file still untracked, so the test did not see it.

Checked:

The renderer builds with llvm-mingw (RX_VK_ONLY=1 nx2/build.py --standalone), and the whole game builds with make_game.py.
tests/run_all.py: 311 passed, 60 skipped. test_precompile_compute_guard and test_fpx_sticky were run with the MSVC headers and lld-link and pass.
Three fail for reasons of this PC, exactly as on untouched main (49f75dc8):
- nx2/test_nup_read_name.py and tests/test_bake_portability.py need the zstandard module;
- nx2/test_nup_paths.py needs the symlink privilege.
The prepare's log has no "did not unpack" and no "shader module was not made" lines.

Also seen while measuring (not changed here)

With the shipped state lists, mkw_vk_pipelines.bin stayed at exactly 999,490 KB while the precompile compiled its 15,456 graphics pipelines, so it was never saved again. Those pipelines came from NVIDIA's own disk cache:

with that cache moved aside, the precompile took 177-186 s on both builds;
after a new hookshot.exe, the first one or two starts took 135-137 s.

I haven't found why the app cache stops growing near 1 GB. It is a separate issue.

🤖 Generated with Claude Code

## The problem Every translated program keeps a `VkShaderModule` for each of its stages for as long as the program lives. - With the shipped state lists, a start keeps about 12,000 programs, which is about 20,400 modules. - NVIDIA's driver holds about 99 KB of its own memory for each module. That is about 2 GB of the game's private memory, in driver arenas full of SPIR-V copies. - A module destroyed later gives none of that memory back. Measured on this PC in play, before this PR: - the game's working set peaked at 15.05 GB; - the system had 1.31 GB available at the lowest point, and paged. In the memory map, two of the driver's ~1 GB allocations held 11,970 and 8,430 SPIR-V headers. Microbenchmark with the game's own 20,400 modules (1636 MB of SPIR-V), driver 621.14: | | private memory | |---|---| | all made and kept alive | +1969 MB | | then all destroyed | +1969 MB (nothing given back) | | made and destroyed one at a time | +8 MB (0.025 ms each) | ## The fix **Stages keep their SPIR-V packed.** Each stage keeps its SPIR-V packed with zstd in `Stage::code`, and `Stage::module` is gone. - zstd level 1 packs the game's SPIR-V 3.77 times smaller: 1636 MB becomes 434 MB. - Packing costs 0.137 ms per module, unpacking 0.054 ms. **Modules exist only while a pipeline is made.** A `StageModule` unpacks the code and makes the module just for the pipeline being made, then destroys it. The new code, `stage_code()` and `StageModule`, is in `gpu/vk/vk_stage_code.cpp`. **Where the modules are made now:** - `build_graphics` makes only the modules of the stages that the pipeline, or the library part, takes. - Each compute path makes its module for its one create call: - the mesh fallback; - the prepare's warm; - the precompile; - the dispatch's ask under `FAIL_ON_PIPELINE_COMPILE_REQUIRED`; - its queued build; - its own compile. - A module that is not made leaves that pipeline unmade, as a failed compile does. It is logged. **Destroys removed.** The module destroys are gone from: - program cache reads (the refused records); - `program_cache_borrow`; - the seed variants; - `seed_variant_of`; - the state list and the prepare. The code now belongs to the program. **zstd.** - `third_party/zstd/lib/compress/` comes from the same zstd 1.5.7 release that is already vendored. It is unmodified, BSD, and has the same tarball SHA-256 as recorded in `PROVENANCE.md`. - It is built single-threaded. Note that `-DZSTD_MULTITHREAD=0` was removed from `nx2/build.py`: zstd tests it with `#ifdef`, so defining it as 0 turned multithreading on. - `zstdmt_compress.c` is vendored with the rest of the folder but not built. ## ⚠️ Costs and things to know before merging 1. **Every player translates the program cache once more.** - `gpu/vk/vk_shader.cpp` is part of the translator stamp, so the first start after updating translates all programs again. - Here that prepare took 211 s for 38,809 programs (Ryzen 7 9800X3D); slower CPUs take longer. - The SPIR-V it produces is byte-identical to before: `hookshot_programs.bin` compared record by record, 98,200 of 98,200 modules the same. So nothing in the driver's caches goes stale. - I put the new code in its own file to keep later changes to it out of the stamp. The changes to `vk_shader.cpp` itself (removing `make_module` and the destroys) could not be avoided. 2. **Only NVIDIA was measured** (RTX 5070, driver 621.14, Windows 11). - AMD, Intel and Linux were not run. - Making a module per pipeline should be cheap on other drivers too: on RADV and ANV a module is essentially a copy of the SPIR-V. I have not measured it. - The Linux build was not compiled locally. The zstd encoder files get `-fPIC` like the decoder's. 3. **Translation and loading do a little more work.** - Full translation in the build's prepare: 211.0 s here, against 185.1 s for the previous build's full translation. That is one run each, so part of the gap may be noise. - Each stage is now packed as the program cache is read, on up to 16 threads. The precompile's "caches opened, notes and programs read" part measured 3.7 s against 1.6 s in the runs without the driver's cache, and 0.8 s against 1.6 s in the runs with it. 4. **Precompile time:** | precompile, 15,456 graphics + 1,182 compute pipelines | before | this PR | |---|---|---| | NVIDIA's disk cache present (the normal case) | 6.7 s | 6.8 s | | NVIDIA's disk cache moved aside | 176.8 s | 185.9 s | These are not a clean A/B: "before" ran from a copy on an NVMe disk and "this PR" from the SATA SSD. Both used the same `mkw_vk_pipelines.bin` contents and the same driver cache. 5. **The recorder.** `vk_stage_code.cpp` does not include `vk_work.h`, so its module destroys are no recorder boundary: no recorded command uses a shader module. It is listed in `test_recorder_sync`'s `BOUNDARY_OK` with that reason, like `vk_shader.cpp`'s destroys before it. So `RX_VK_RECORDER=defer` and `=thread` neither drain nor wait for them, also when the dispatch's ask makes and destroys a module on the replay thread. 6. **Overlap with #1630** (compressed program cache, open). - Both PRs change the zstd block of `nx2/build.py`, and `tests/program_cache_cleanup_test.cpp`. - #1630 builds `third_party/zbic`'s single-file zstd into the game. That file's `PROVENANCE.md` says it carries GPLv2 parts from Atmosphere-NX and is not linked into the game. This PR instead vendors the official encoder next to the decoder already used. - Whichever lands second needs a rebase, and I'll adapt this one to whatever you prefer. ## Measured **Test setup:** - RTX 5070 (driver 621.14), Ryzen 7 9800X3D, 31 GB RAM, Windows 11. - The game is on a SATA SSD. The test harness's files and the game's log are on another disk. - Scripted headless route: load a save, then walk outdoors on the Kokiri path. "In play" means from present 3300 on. - The caches are in their normal state, after `--prepare-shaders`. - The build measured is this commit on top of a `main` from the same day, plus open PRs #2198, #2222 and #2193, plus local logging (present times, memory map). The comparison build is the same thing without this commit. One run each: | | before | this PR | |---|---|---| | game's largest working set | 15.05 GB | **13.51 GB** | | system memory available, lowest | 1.31 GB | **2.67 GB** | | game's private memory in RAM (80 s) | 7.59 GB | **6.07 GB** | | commit charge (80 s) | 26.37 GB | **24.47 GB** | | the game's private allocations of 0.6 GB and over (80 s) | 6, 6.86 GB | 5, 4.70 GB | | SPIR-V headers in the three largest | 20,400 | 2 | | system page reads during the run | 1.41 M | 1.34 M | | mean frame time, in play | 17.14 ms | 17.18 ms | | frame time p99 | 24.64 ms | 24.52 ms | | 1% low | 37.5 fps | 36.7 fps | | frames over 33.4 ms | 0 | 1 | | worst frame | 31.8 ms | 35.6 ms | | **frames over 20 ms** | **157** | **236** | | **time lost against 60 fps, in play** | **886 ms** | **1318 ms** | **About the frames over 20 ms:** - This PR's run had more of them, and they cluster in the same parts of the route as before's. - The replay's own times are the same in both runs: "pipelines" is 0.15 ms a frame, and there are 0 waits for shaders or pipelines. - On this machine, identical configurations have differed by as much between runs (56 against 95 frames over 25 ms). - With one run each, I cannot rule this out as a cost. **A discarded run.** A first run of this PR (not in the table) had 10 frames over 33 ms, the worst 102 ms. - Every one of them lined up with a read stall on the game's SATA disk: 168 reads over 10 ms in play, against 0 in the before run. - Just before it, I had read and written about 20 GB on that disk: three prepares, and comparing the program caches. - I waited for the disk to settle (idle probe: median 0.44 ms, none over 10 ms) and ran it again. That second run is the table. ## Tests - `tests/program_cache_cleanup_test.cpp` and `tests/test_program_cache_cleanup.py` now check which program cache records are taken and that each taken stage keeps its code whole, through the production loader, with a `stage_code` stand-in that fails when told to. They used to count module lifetimes. Cases: first or second stage not kept, truncated, empty, answers past the stages or slots, valid, mixed. - `tests/test_precompile_compute_guard.py`'s fixture has a `StageModule` stand-in. The module must be alive during the create call and gone after it. - `tests/test_mesh_fallback_failed_once.py` finds the failure block after the compile, which may now also be the module's. - `tests/test_shader_stamp_cache.py` and `tests/test_dlss_sdk_cache.py` know the zstd encoder sources and `zstd_users`. - `tests/test_recorder_sync.py` lists `vk_stage_code.cpp` in `BOUNDARY_OK` (its module destroys), and `vk_shader.cpp`'s reason no longer names modules. The first CI run failed here: my local run had the new file still untracked, so the test did not see it. Checked: - The renderer builds with llvm-mingw (`RX_VK_ONLY=1 nx2/build.py --standalone`), and the whole game builds with `make_game.py`. - `tests/run_all.py`: 311 passed, 60 skipped. `test_precompile_compute_guard` and `test_fpx_sticky` were run with the MSVC headers and `lld-link` and pass. - Three fail for reasons of this PC, exactly as on untouched `main` (49f75dc8): - `nx2/test_nup_read_name.py` and `tests/test_bake_portability.py` need the `zstandard` module; - `nx2/test_nup_paths.py` needs the symlink privilege. - The prepare's log has no "did not unpack" and no "shader module was not made" lines. ## Also seen while measuring (not changed here) With the shipped state lists, `mkw_vk_pipelines.bin` stayed at exactly 999,490 KB while the precompile compiled its 15,456 graphics pipelines, so it was never saved again. Those pipelines came from NVIDIA's own disk cache: - with that cache moved aside, the precompile took 177-186 s on both builds; - after a new `hookshot.exe`, the first one or two starts took 135-137 s. I haven't found why the app cache stops growing near 1 GB. It is a separate issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) reviewed

2026-11-11 16:51:09 +00:00

hckrman101 (Migrated from github.com) left a comment

Nice work. The memory map is good evidence, and so is the one-at-a-time microbenchmark. I went through the lifetimes and found nothing wrong:

Module lifetime. Every StageModule lives in the scope of its create call. That covers build_graphics (the modules vector lives to the end of the function, after graphics_with, which is synchronous), the mesh fallback, the warm, the precompile, the dispatch's ask and its queued build lambda (the module is made inside the lambda, on the worker), and the dispatch's own compile. No recorded command or later link uses a module: GPL linking takes the library pipelines, not modules. So the BOUNDARY_OK entry is right.
Threads. Stage::code is only read after a program is built. The pack context and the unpack buffer are thread_local, and the log counter is atomic. Parallel prepares and cache reads (16 threads) share nothing new.
GPL parts. The new early filter in build_graphics matches the part_stages filter below it. Parts 0 and 3 now make no modules, and parts 1 and 2 make only their own stages. That is also a small win, because before, every part got every stage's module.
VK_KHR_maintenance5. Nothing in gpu/vk chains VkShaderModuleCreateInfo into the stage info. Every path goes through a module, so no maintenance5 path is affected.
Caches. The disk record format is unchanged (the SPIR-V is still saved from kept), and you checked that the SPIR-V is byte-identical. So hookshot_programs.bin and mkw_vk_pipelines.bin/driver caches stay valid. The one cost is the stamp bump: every player translates everything again once (~3.5 min on a 9800X3D, more on slower CPUs). @NotRexed should decide whether that's fine, and also how this fits with #1630's zstd.
The ZSTD_MULTITHREAD=0 catch is a good one.

Before this goes in:

CI. The red Linux jobs are runner failures, not this PR: the apt mirrors didn't resolve, so glslang/spirv-val were missing and the appimage job stopped at System packages. But that means nothing has compiled the new zstd encoder on Linux yet, in CI or locally. Please rerun the jobs so the appimage/Linux build goes green.
Frame times. 157 → 236 frames over 20 ms and 886 → 1318 ms lost is a big gap for one run each. You say it could be noise, and the replay's own times match, but a second A/B pair (or the order swapped) would settle it. The extra per-pipeline work in play (unpack + module create, ~0.1 ms) shouldn't explain it.
Nobody has tested AMD or Intel. A short run on RADV or the AMD Windows driver would help, since the AMD Windows paths (DISABLE_OPTIMIZATION retries) now create the pipeline twice from the same live module. That should be fine, but no one has run it.

One nit inline.

Nice work. The memory map is good evidence, and so is the one-at-a-time microbenchmark. I went through the lifetimes and found nothing wrong: - **Module lifetime.** Every `StageModule` lives in the scope of its create call. That covers `build_graphics` (the `modules` vector lives to the end of the function, after `graphics_with`, which is synchronous), the mesh fallback, the warm, the precompile, the dispatch's ask and its queued `build` lambda (the module is made inside the lambda, on the worker), and the dispatch's own compile. No recorded command or later link uses a module: GPL linking takes the library pipelines, not modules. So the `BOUNDARY_OK` entry is right. - **Threads.** `Stage::code` is only read after a program is built. The pack context and the unpack buffer are `thread_local`, and the log counter is atomic. Parallel prepares and cache reads (16 threads) share nothing new. - **GPL parts.** The new early filter in `build_graphics` matches the `part_stages` filter below it. Parts 0 and 3 now make no modules, and parts 1 and 2 make only their own stages. That is also a small win, because before, every part got every stage's module. - **VK_KHR_maintenance5.** Nothing in `gpu/vk` chains `VkShaderModuleCreateInfo` into the stage info. Every path goes through a module, so no maintenance5 path is affected. - **Caches.** The disk record format is unchanged (the SPIR-V is still saved from `kept`), and you checked that the SPIR-V is byte-identical. So `hookshot_programs.bin` and `mkw_vk_pipelines.bin`/driver caches stay valid. The one cost is the stamp bump: every player translates everything again once (~3.5 min on a 9800X3D, more on slower CPUs). @NotRexed should decide whether that's fine, and also how this fits with #1630's zstd. - The `ZSTD_MULTITHREAD=0` catch is a good one. Before this goes in: 1. **CI.** The red Linux jobs are runner failures, not this PR: the apt mirrors didn't resolve, so `glslang`/`spirv-val` were missing and the appimage job stopped at System packages. But that means nothing has compiled the new zstd encoder on Linux yet, in CI or locally. Please rerun the jobs so the appimage/Linux build goes green. 2. **Frame times.** 157 → 236 frames over 20 ms and 886 → 1318 ms lost is a big gap for one run each. You say it could be noise, and the replay's own times match, but a second A/B pair (or the order swapped) would settle it. The extra per-pipeline work in play (unpack + module create, ~0.1 ms) shouldn't explain it. 3. Nobody has tested AMD or Intel. A short run on RADV or the AMD Windows driver would help, since the AMD Windows paths (`DISABLE_OPTIMIZATION` retries) now create the pipeline twice from the same live module. That should be fine, but no one has run it. One nit inline.

gpu/vk/vk_stage_code.cpp

					
				@ -0,0 +53,4 @@

				        return;

				    }

				    static std::atomic<int> told{0};

				    const size_t n = ZSTD_decompress(words.data(), words.size() * sizeof(uint32_t), st.code.packed.data(), st.code.packed.size());

nit: ZSTD_decompress makes and frees a whole ZSTD_DCtx on the heap on every call (ZSTD_HEAPMODE is 1 by default), so that's one ~100 KB malloc/free per module made. A thread_local ZSTD_DCtx with ZSTD_decompressDCtx would match the pack side's Ctx. It's small next to vkCreateShaderModule, but this now runs on every pipeline compiled in play.

nit: `ZSTD_decompress` makes and frees a whole `ZSTD_DCtx` on the heap on every call (`ZSTD_HEAPMODE` is 1 by default), so that's one ~100 KB malloc/free per module made. A `thread_local ZSTD_DCtx` with `ZSTD_decompressDCtx` would match the pack side's `Ctx`. It's small next to `vkCreateShaderModule`, but this now runs on every pipeline compiled in play.

Joey-Magnus (Migrated from github.com) reviewed

2026-11-11 21:26:18 +00:00

Joey-Magnus (Migrated from github.com) left a comment

The size is vendored zstd: about 23,350 lines of third_party/zstd/lib/compress/, byte-identical to the official v1.5.7 tarball (the SHA-256 matches PROVENANCE.md), BSD-licensed. The real change is about 290 lines. Every StageModule lives within its create call, nothing outside gpu/vk uses Stage::module, and dropping -DZSTD_MULTITHREAD=0 is right.

Still open:

Linux CI has never compiled the encoder.
The A/B is one run each (157 vs 236 frames over 20 ms), with no AMD or Intel.
It forces a full re-translation for every player (211 s on a 9800X3D), as do #2343, #2306, #2442 and #2457. Shipping them together means one re-prepare.

The size is vendored zstd: about 23,350 lines of `third_party/zstd/lib/compress/`, byte-identical to the official v1.5.7 tarball (the SHA-256 matches `PROVENANCE.md`), BSD-licensed. The real change is about 290 lines. Every `StageModule` lives within its create call, nothing outside `gpu/vk` uses `Stage::module`, and dropping `-DZSTD_MULTITHREAD=0` is right. Still open: - Linux CI has never compiled the encoder. - The A/B is one run each (157 vs 236 frames over 20 ms), with no AMD or Intel. - It forces a full re-translation for every player (211 s on a 9800X3D), as do #2343, #2306, #2442 and #2457. Shipping them together means one re-prepare.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 22:23:06 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-review at 67bb1f7. The only change since my last review is the merge of main, and it is resolved correctly:

The program cache reads and program_cache_borrow moved to vk_progcache.cpp (#2323). They now keep the code with stage_code and destroy no modules, make_module is gone from vk_progcache.h, and the BOUNDARY_OK reason for vk_progcache.cpp is updated to match.
zstd_jobs takes encoder=False by default, so the runtime's decoder job from #2270 builds as before. Only the Vulkan back end gets the encoder.
Nothing at the head still reads Stage::module. Main has moved 53 commits since this merge. Those commits add no new uses of it, and a three-way merge of the files both sides touch (vk_draw.cpp, vk_shader.cpp/.h, nx2/build.py, test_recorder_sync.py, seed_variant_cleanup_test.cpp) applies cleanly.

Still open from last time:

CI. The workflows at this head are queued and have not run yet, and the earlier Linux run failed. So nothing has compiled the zstd encoder on Linux yet. Once the AppImage/Linux jobs are green, this point is done.
Frame times. There is still only one A/B pair (157 vs 236 frames over 20 ms). A second pair, or the same pair run in the other order, would settle whether that gap is noise.
AMD/Intel. Still no run on either.
The ZSTD_DCtx nit inline is unchanged. It's still only a nit.

I agree with Joey-Magnus that this should ship together with the other PRs that force a full re-translation, so players re-prepare once. The stamp bump and how this fits with #1630's zstd are still for @NotRexed to decide.

Nit: the runtime's decoder job (nx2/build.py, the "zstd_" call) still passes -DZSTD_MULTITHREAD=0. That's harmless for the decoder alone, but now that the docstring says the flags leave it out, you could drop it there too for consistency.

Re-review at 67bb1f7. The only change since my last review is the merge of main, and it is resolved correctly: - The program cache reads and `program_cache_borrow` moved to `vk_progcache.cpp` (#2323). They now keep the code with `stage_code` and destroy no modules, `make_module` is gone from `vk_progcache.h`, and the `BOUNDARY_OK` reason for `vk_progcache.cpp` is updated to match. - `zstd_jobs` takes `encoder=False` by default, so the runtime's decoder job from #2270 builds as before. Only the Vulkan back end gets the encoder. - Nothing at the head still reads `Stage::module`. Main has moved 53 commits since this merge. Those commits add no new uses of it, and a three-way merge of the files both sides touch (`vk_draw.cpp`, `vk_shader.cpp/.h`, `nx2/build.py`, `test_recorder_sync.py`, `seed_variant_cleanup_test.cpp`) applies cleanly. Still open from last time: 1. **CI.** The workflows at this head are queued and have not run yet, and the earlier Linux run failed. So nothing has compiled the zstd encoder on Linux yet. Once the AppImage/Linux jobs are green, this point is done. 2. **Frame times.** There is still only one A/B pair (157 vs 236 frames over 20 ms). A second pair, or the same pair run in the other order, would settle whether that gap is noise. 3. **AMD/Intel.** Still no run on either. 4. The `ZSTD_DCtx` nit inline is unchanged. It's still only a nit. I agree with Joey-Magnus that this should ship together with the other PRs that force a full re-translation, so players re-prepare once. The stamp bump and how this fits with #1630's zstd are still for @NotRexed to decide. Nit: the runtime's decoder job (`nx2/build.py`, the `"zstd_"` call) still passes `-DZSTD_MULTITHREAD=0`. That's harmless for the decoder alone, but now that the docstring says the flags leave it out, you could drop it there too for consistency.

github-sync commented

2026-11-12 09:32:17 +00:00

@hckrman101 commented on GitHub · 2026-11-12 08:36 UTC · original

Can you do re-measurement and see if the frame pacing is worst?

> [@hckrman101](https://github.com/hckrman101) commented on GitHub · 2026-11-12 08:36 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2309#issuecomment-6818818113) Can you do re-measurement and see if the frame pacing is worst?

github-sync commented

2026-11-12 12:37:11 +00:00

@DUDUYG commented on GitHub · 2026-11-12 11:42 UTC · original

I re-measured with the two builds alternated A B A B A B on the same disk. Frame pacing is not worse with this change, and the earlier 157 vs 236 gap was noise.

Setup:

RTX 5070 (driver 621.14), Ryzen 7 9800X3D, Windows 11.
Both game folders are on the same NVMe disk. The romfs is on the SATA disk, and the runs' files are on the NVMe.
Headless at the 60 fps cap, with the same options file (DLAA), on the scripted Kokiri route. "In play" means presents ≥ 3300.

The two arms:

A: without this PR (test build 4123715e).
B: with it, plus this push's ZSTD_DCtx change (test build c05ce25b).

Both are on the same older main (ebbba2e), with the same extra PRs (#2198, #2222, #2193) and the same logging. The only difference is this PR's change.

run	fps	frame time sd (ms)	jitter (ms)¹	p99 (ms)	> 20 ms	> 33.4 ms	> 50 ms	1% low	worst (ms)	lost vs 60 fps (ms)
A1	59.77	3.46	1.98	23.04	72	4	2	29.4	155.0	1801
B1	60.00	1.62	1.67	22.70	70	1	0	39.2	40.5	1467
A2	59.95	1.93	1.74	23.51	79	1	1	37.1	69.6	1582
B2	59.50	4.91	2.02	23.60	80	5	3	24.4	188.7	1937
A3	59.11	10.36	2.19	22.81	71	4	3	19.8	538.2	2175
B3	59.94	1.74	1.68	23.98	80	2	0	37.8	38.1	1590
A mean	59.61	5.25	1.97	23.12	74	3	2	28.8		1853
B mean	59.82	2.76	1.79	23.43	77	3	1	33.8		1665

¹ the mean change in frame time from one frame to the next.

Reading it:

Frames over 20 ms range 71-79 without the change and 70-80 with it. Runs of the same build differ as much as the arms do.
The earlier pair ran from the SATA disk. There, the game's reads stall now and then: the discarded run before it had 168 game-disk reads over 10 ms.
The few big spikes (538 ms in A3, 189 ms in B2) are single hitches, and they show up in both arms.

Also pushed c251eab2:

vk_stage_code.cpp: a thread_local ZSTD_DCtx with ZSTD_decompressDCtx, like the pack side's context (your inline nit).
nx2/build.py: the runtime's decoder-only zstd_jobs call without -DZSTD_MULTITHREAD=0.

The renderer builds (RX_VK_ONLY=1 nx2/build.py --standalone, llvm-mingw, no warnings). These pass: shader_stamp_cache, dlss_sdk_cache, program_cache_cleanup, seed_variant_cleanup, precompile_compute_guard, recorder_sync, game_font, mods_size_table and mesh_fallback_failed_once. The B runs above include the DCtx change. Their prepare made all 15,456 graphics and 1,200 compute pipelines through it, with no unpack or module error.

Still not done: a run on AMD or Intel. The Linux CI is queued.

> [@DUDUYG](https://github.com/DUDUYG) commented on GitHub · 2026-11-12 11:42 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2309#issuecomment-6822636625) I re-measured with the two builds alternated A B A B A B on the same disk. Frame pacing is not worse with this change, and the earlier 157 vs 236 gap was noise. **Setup:** - RTX 5070 (driver 621.14), Ryzen 7 9800X3D, Windows 11. - Both game folders are on the same NVMe disk. The romfs is on the SATA disk, and the runs' files are on the NVMe. - Headless at the 60 fps cap, with the same options file (DLAA), on the scripted Kokiri route. "In play" means presents ≥ 3300. **The two arms:** - A: without this PR (test build `4123715e`). - B: with it, plus this push's `ZSTD_DCtx` change (test build `c05ce25b`). Both are on the same older `main` (ebbba2e), with the same extra PRs (#2198, #2222, #2193) and the same logging. The only difference is this PR's change. | run | fps | frame time sd (ms) | jitter (ms)¹ | p99 (ms) | > 20 ms | > 33.4 ms | > 50 ms | 1% low | worst (ms) | lost vs 60 fps (ms) | |---|---|---|---|---|---|---|---|---|---|---| | A1 | 59.77 | 3.46 | 1.98 | 23.04 | 72 | 4 | 2 | 29.4 | 155.0 | 1801 | | B1 | 60.00 | 1.62 | 1.67 | 22.70 | 70 | 1 | 0 | 39.2 | 40.5 | 1467 | | A2 | 59.95 | 1.93 | 1.74 | 23.51 | 79 | 1 | 1 | 37.1 | 69.6 | 1582 | | B2 | 59.50 | 4.91 | 2.02 | 23.60 | 80 | 5 | 3 | 24.4 | 188.7 | 1937 | | A3 | 59.11 | 10.36 | 2.19 | 22.81 | 71 | 4 | 3 | 19.8 | 538.2 | 2175 | | B3 | 59.94 | 1.74 | 1.68 | 23.98 | 80 | 2 | 0 | 37.8 | 38.1 | 1590 | | **A mean** | 59.61 | 5.25 | 1.97 | 23.12 | 74 | 3 | 2 | 28.8 | | 1853 | | **B mean** | 59.82 | 2.76 | 1.79 | 23.43 | 77 | 3 | 1 | 33.8 | | 1665 | ¹ the mean change in frame time from one frame to the next. **Reading it:** - Frames over 20 ms range 71-79 without the change and 70-80 with it. Runs of the same build differ as much as the arms do. - The earlier pair ran from the SATA disk. There, the game's reads stall now and then: the discarded run before it had 168 game-disk reads over 10 ms. - The few big spikes (538 ms in A3, 189 ms in B2) are single hitches, and they show up in both arms. **Also pushed `c251eab2`:** - `vk_stage_code.cpp`: a `thread_local` `ZSTD_DCtx` with `ZSTD_decompressDCtx`, like the pack side's context (your inline nit). - `nx2/build.py`: the runtime's decoder-only `zstd_jobs` call without `-DZSTD_MULTITHREAD=0`. The renderer builds (`RX_VK_ONLY=1 nx2/build.py --standalone`, llvm-mingw, no warnings). These pass: `shader_stamp_cache`, `dlss_sdk_cache`, `program_cache_cleanup`, `seed_variant_cleanup`, `precompile_compute_guard`, `recorder_sync`, `game_font`, `mods_size_table` and `mesh_fallback_failed_once`. The B runs above include the DCtx change. Their prepare made all 15,456 graphics and 1,200 compute pipelines through it, with no unpack or module error. **Still not done:** a run on AMD or Intel. The Linux CI is queued.

github-sync commented

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

@Brett-D commented on GitHub · 2026-11-12 13:03 UTC · original

Tested on RTX 5090 (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) with RX_RELEASE=1 (17,888 programs, 25,303 graphics and 799 compute pipelines precompiled at startup), as part of isolating main's +5.86 GB Private Bytes / +2.51 GB Working Set increase since 49f75dc8.

1. Where `main`'s `+5.86 GB` Private / `+2.51 GB` Working Set increase goes

Comparing 49f75dc8 (15,004 MB Peak WS / 26,280 MB Peak Private) to current main (17,510 MB Peak WS / 32,140 MB Peak Private) in Kokiri Forest (2048x1152 uncapped, RX_VK_FRAMEGEN=0, HOS_FPS_MAX=240):

+2,944 MB (2.88 GB) Private Bytes / VRAM (0 MB Working Set): Stranded images_pool() startup pre-allocation (#2438, 867ddd7f), which pre-allocates minBlockCount = 24 (3,072 MB) in image_pool even though RX_VK_COMPACT (textures_pool()) and RX_VK_TARGET_BLOCKS (target_images_pool()) are on by default (23 of 24 blocks = 2,944 MB stay permanently empty; covered by #2711 / #2759).
+2,916 MB (2.85 GB) Private Bytes and +2,506 MB (2.45 GB) Working Set: Expanding the shipped pipeline state list from 15,290 -> 25,303 graphics pipelines (+10,013 pipelines) and 11,748 -> 17,888 loaded programs (+6,140 programs), each holding unpacked Stage::code SPIR-V and live VkShaderModule handles across the run. Replacing hookshot_pipeline_states_shipped.bin on main with 49f75dc8's list (15,290 pipelines) + RX_VK_IMAGE_PREALLOC=0 brings main down to 14,727 MB Peak WS / 25,863 MB Peak Private (-2,783 MB WS / -6,277 MB Private).

2. #2309 A/B on `main` (`RX_RELEASE=1`, all 25,303 graphics pipelines precompiled)

Arm (`RX_RELEASE=1`, 25,303 pipelines)	Peak Working Set	Δ WS vs `main`	Peak Private Bytes	Δ Private vs `main`	Precompile Time (`17,888` progs / `25,303` pipes)	Uncapped FPS (`p95` / `p99` ms)	Frames > 50 ms
`main` control (`c544828f`)	17,458 MB	baseline	33,818 MB	baseline	2,630 ms (`2,048 ms` cache read)	67.2 (`17.07` / `19.61`)	0
#2309 (`DUDUYG`)	15,069 MB	-2,389 MB (-2.33 GB)	31,281 MB	-2,537 MB (-2.48 GB)	3,163 ms (`2,374 ms` cache read + zstd pack)	68.6 (`16.81` / `18.68`)	0

Verdict:

Across the full 17,888-program / 25,303-pipeline precompile on main, #2309 saves -2,389 MB (-2.33 GB) of Peak Working Set (recovering 95% of main's entire +2,506 MB Working Set growth since 49f75dc8) and -2,537 MB (-2.48 GB) of Peak Private Bytes (recovering 87% of the +2,916 MB precompile Private Bytes growth), while keeping all 25,303 precompiled pipelines.
Startup precompile increases by only +0.53 s (3.16 s vs 2.63 s), time to first frame is unchanged (4 s), first-load stutter is 0 frames over 50 ms, and frame rate/pacing in Kokiri Forest is at least as fast (68.6 fps, p99 = 18.68 ms).

> [@Brett-D](https://github.com/Brett-D) commented on GitHub · 2026-11-12 13:03 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2309#issuecomment-6824380793) Tested on `RTX 5090` (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) with `RX_RELEASE=1` (`17,888` programs, `25,303` graphics and `799` compute pipelines precompiled at startup), as part of isolating `main`'s `+5.86 GB` Private Bytes / `+2.51 GB` Working Set increase since `49f75dc8`. ### 1. Where `main`'s `+5.86 GB` Private / `+2.51 GB` Working Set increase goes Comparing `49f75dc8` (`15,004 MB` Peak WS / `26,280 MB` Peak Private) to current `main` (`17,510 MB` Peak WS / `32,140 MB` Peak Private) in Kokiri Forest (`2048x1152` uncapped, `RX_VK_FRAMEGEN=0`, `HOS_FPS_MAX=240`): 1. **`+2,944 MB` (`2.88 GB`) Private Bytes / VRAM (`0 MB` Working Set):** Stranded `images_pool()` startup pre-allocation (`#2438`, `867ddd7f`), which pre-allocates `minBlockCount = 24` (`3,072 MB`) in `image_pool` even though `RX_VK_COMPACT` (`textures_pool()`) and `RX_VK_TARGET_BLOCKS` (`target_images_pool()`) are on by default (`23 of 24 blocks = 2,944 MB` stay permanently empty; covered by #2711 / #2759). 2. **`+2,916 MB` (`2.85 GB`) Private Bytes and `+2,506 MB` (`2.45 GB`) Working Set:** Expanding the shipped pipeline state list from `15,290 -> 25,303` graphics pipelines (`+10,013` pipelines) and `11,748 -> 17,888` loaded programs (`+6,140` programs), each holding unpacked `Stage::code` SPIR-V and live `VkShaderModule` handles across the run. Replacing `hookshot_pipeline_states_shipped.bin` on `main` with `49f75dc8`'s list (`15,290` pipelines) + `RX_VK_IMAGE_PREALLOC=0` brings `main` down to `14,727 MB` Peak WS / `25,863 MB` Peak Private (`-2,783 MB` WS / `-6,277 MB` Private). ### 2. #2309 A/B on `main` (`RX_RELEASE=1`, all 25,303 graphics pipelines precompiled) | Arm (`RX_RELEASE=1`, 25,303 pipelines) | Peak Working Set | Δ WS vs `main` | Peak Private Bytes | Δ Private vs `main` | Precompile Time (`17,888` progs / `25,303` pipes) | Uncapped FPS (`p95` / `p99` ms) | Frames > 50 ms | |---|---|---|---|---|---|---|---| | `main` control (`c544828f`) | 17,458 MB | baseline | 33,818 MB | baseline | 2,630 ms (`2,048 ms` cache read) | 67.2 (`17.07` / `19.61`) | 0 | | **#2309 (`DUDUYG`)** | **15,069 MB** | **-2,389 MB (-2.33 GB)** | **31,281 MB** | **-2,537 MB (-2.48 GB)** | **3,163 ms (`2,374 ms` cache read + zstd pack)** | **68.6 (`16.81` / `18.68`)** | **0** | **Verdict:** - Across the full `17,888`-program / `25,303`-pipeline precompile on `main`, #2309 saves **`-2,389 MB` (`-2.33 GB`) of Peak Working Set** (recovering **95%** of `main`'s entire `+2,506 MB` Working Set growth since `49f75dc8`) and **`-2,537 MB` (`-2.48 GB`) of Peak Private Bytes** (recovering **87%** of the `+2,916 MB` precompile Private Bytes growth), while keeping all 25,303 precompiled pipelines. - Startup precompile increases by only `+0.53 s` (`3.16 s` vs `2.63 s`), time to first frame is unchanged (`4 s`), first-load stutter is `0` frames over 50 ms, and frame rate/pacing in Kokiri Forest is at least as fast (`68.6 fps`, `p99 = 18.68 ms`).

github-sync referenced this pull request

2026-11-12 14:41:33 +00:00

perf(vk): compress the program cache with zstd (RX_VK_PROGRAM_CACHE_ZSTD) #1630

github-sync referenced this pull request

2026-11-12 14:42:18 +00:00

perf(vk): add floats to storage buffers with the GPU's own atomic, not a compare-and-swap loop #2306

github-sync commented

2026-11-12 14:42:20 +00:00

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:32 UTC · original

The re-measurement settles the frame-pacing question. Three alternated A/B pairs on the same disk show B no worse, and the 157 vs 236 gap was noise. The ZSTD_DCtx nit is fixed with a per-thread context (vk_stage_code.cpp).

It conflicts with main again, in one place. #2525 added st.phis = fallback_size::spirv_phis(spirv...) to program_cache_read (vk_progcache.cpp:~431), and fallback_compute_pipelines reads st.phis on RADV GCN. When you resolve it, keep that line before stage_code(st.code, spirv), because the SPIR-V isn't kept afterwards. I resolved it that way locally. tests/run_all.py passes apart from test_lift_cache, which fails on main here, and test_recorder_sync passes when run alone. I also compiled the vendored lib/compress with gcc on Linux, since no CI run has built it yet.

#2760 and #2800 (open) still use Stage::module, so whichever lands second needs porting. AMD and Intel are still untested. The stamp bump should ship together with the other PRs that force a re-translation (#2306, #1975).

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2309#pullrequestreview-5876208940) The re-measurement settles the frame-pacing question. Three alternated A/B pairs on the same disk show B no worse, and the 157 vs 236 gap was noise. The `ZSTD_DCtx` nit is fixed with a per-thread context (`vk_stage_code.cpp`). It conflicts with main again, in one place. #2525 added `st.phis = fallback_size::spirv_phis(spirv...)` to `program_cache_read` (`vk_progcache.cpp:~431`), and `fallback_compute_pipelines` reads `st.phis` on RADV GCN. When you resolve it, keep that line before `stage_code(st.code, spirv)`, because the SPIR-V isn't kept afterwards. I resolved it that way locally. `tests/run_all.py` passes apart from `test_lift_cache`, which fails on main here, and `test_recorder_sync` passes when run alone. I also compiled the vendored `lib/compress` with gcc on Linux, since no CI run has built it yet. #2760 and #2800 (open) still use `Stage::module`, so whichever lands second needs porting. AMD and Intel are still untested. The stamp bump should ship together with the other PRs that force a re-translation (#2306, #1975).

Commenting is not possible because the repository is archived.

No reviewers