perf(vk): write the pipeline cache only when its data changed (no 1.1 GB rewrite at every start) #1660

Merged
DUDUYG merged 1 commit from perf/pipeline-cache-unchanged into main 2026-11-10 17:17:19 +00:00
DUDUYG commented 2026-11-10 12:42:42 +00:00 (Migrated from github.com)

The problem

Every start rewrote the whole pipeline cache, mkw_vk_pipelines.bin (1.1 GB here), shortly after the title screen, even when nothing new was in it.

pcache_built_since_save counts every pipeline made with the app's cache, including the ones the cache already held. The precompile makes about 13,000 pipelines at each start, all of them found in the cache. That puts the count over 32, so the first check in play (present 300) fetches and writes the whole cache. The file came out at the same 1146 MB, +0 KB, at every start.

On a SATA SSD (90% full, where this game is installed on my PC), that write took 3.6–9.9 s. It also left the disk's 4 KB reads at 10–450 ms for minutes afterwards. The game's own reads from that disk (code pages of hookshot.exe, the romfs) waited behind them, and those waits are the long frames.

The fix

  • A save asks the driver for the data's size first, which copies no data. It fetches and writes only when the size differs from the one read at start or last written.
  • pcache_init records the size of the data it read.
  • After a write, the new size is recorded only once the file is really in place: the write, the close and the rename must all succeed. A failed write is tried again at the next save.
  • A failed fclose now also counts as a failed write. Before, the file was renamed into place anyway.
  • One log line per write: vk: pipeline cache saved (N KB).

I tried counting only pipelines new to the cache, using VkPipelineCreationFeedback's APPLICATION_PIPELINE_CACHE_HIT_BIT. It cannot tell them apart: NVIDIA's driver reports most precompiled pipelines as not found in the app's cache (about 460 of 12,900 here), because it then serves them from its own. The data's size is what the file would actually gain.

The pipeline cache only grows as pipelines are taken in. If a driver ever kept the same size with different contents, the cost would be a skipped save: those pipelines would compile again at the next start. Nothing breaks.

This works with #1586: its forced save after warming the libraries still writes when the warm added data.

Measured

Test setup:

  • RTX 5070, driver 621.01, Ryzen 7 9800X3D, 31 GB.
  • Game and its log on a SATA SSD; the test harness's own files on another disk.
  • Scripted headless route: load a save, then walk outdoors on the Kokiri path.
  • Frames counted from present 3300 on, in play; about 2,820 frames a run.
  • 3 runs of each version, alternated.
before (main) this PR
cache writes per start 1 (1146 MB, +0 KB, 3.6–3.9 s) 0 (size asked in 0.0 ms)
game-disk 4 KB reads over 10 ms in play 267 / 274 / 324 3 / 0 / 0
worst game-disk read 150 / 118 / 153 ms 68 / 7 / 6 ms
frames over 33.4 ms 8 / 11 / 22 3 / 2 / 1
frames over 25 ms 36 / 41 / 49 27 / 18 / 24
time late in total 883 / 992 / 1260 ms 709 / 581 / 608 ms
1% low 32.7 / 28.9 / 25.8 fps 33.3 / 37.8 / 37.0 fps

The average stays at 60 fps (capped). The "before" runs also had line-by-line log writes; a separate series with this fix in both arms shows those cost much less than the cache rewrite.

Tests

tests/test_pipeline_cache_save.py has a new case, unchanged:

  • Data of the size read is not fetched or written, neither at exit nor in play.
  • After a failed write, and after a failed rename, the same data is written at the next save.
  • Once written, it is not written again.
  • A different size, smaller included, is written.

The existing cases now grow the data between saves, where they expect a second write.

  • With --baseline-source set to 4518a20^'s vk_draw.cpp (the helper the option is meant for), all 12 pass, the negative ones included.
  • main's function fails unchanged.

Also checked:

  • RX_VK_ONLY=1 nx2/build.py --standalone: 205 files, 0 failed, no warnings in vk_draw.cpp.
  • tests/run_all.py: 224 checks, 33 skipped, 2 failed for reasons of this PC, not this change:
    • test_precompile_compute_guard.py needs windows.h for an MSVC target.
    • test_nup_paths.py needs the symlink privilege.

Left for later

A save in play that does have something new still fetches and writes the whole file. A player's first session after an update can make about 12 such writes. A session cache for the pipelines the base cache misses, merged into the base at exit, would remove those. That is a larger change, so it is not in this PR.

🤖 Generated with Claude Code

## The problem Every start rewrote the whole pipeline cache, `mkw_vk_pipelines.bin` (1.1 GB here), shortly after the title screen, even when nothing new was in it. `pcache_built_since_save` counts every pipeline made with the app's cache, including the ones the cache already held. The precompile makes about 13,000 pipelines at each start, all of them found in the cache. That puts the count over 32, so the first check in play (present 300) fetches and writes the whole cache. The file came out at the same 1146 MB, +0 KB, at every start. On a SATA SSD (90% full, where this game is installed on my PC), that write took 3.6–9.9 s. It also left the disk's 4 KB reads at 10–450 ms for minutes afterwards. The game's own reads from that disk (code pages of `hookshot.exe`, the romfs) waited behind them, and those waits are the long frames. ## The fix - A save asks the driver for the data's size first, which copies no data. It fetches and writes only when the size differs from the one read at start or last written. - `pcache_init` records the size of the data it read. - After a write, the new size is recorded only once the file is really in place: the write, the close and the rename must all succeed. A failed write is tried again at the next save. - A failed `fclose` now also counts as a failed write. Before, the file was renamed into place anyway. - One log line per write: `vk: pipeline cache saved (N KB)`. I tried counting only pipelines new to the cache, using `VkPipelineCreationFeedback`'s `APPLICATION_PIPELINE_CACHE_HIT_BIT`. It cannot tell them apart: NVIDIA's driver reports most precompiled pipelines as *not* found in the app's cache (about 460 of 12,900 here), because it then serves them from its own. The data's size is what the file would actually gain. The pipeline cache only grows as pipelines are taken in. If a driver ever kept the same size with different contents, the cost would be a skipped save: those pipelines would compile again at the next start. Nothing breaks. This works with #1586: its forced save after warming the libraries still writes when the warm added data. ## Measured Test setup: - RTX 5070, driver 621.01, Ryzen 7 9800X3D, 31 GB. - Game and its log on a SATA SSD; the test harness's own files on another disk. - Scripted headless route: load a save, then walk outdoors on the Kokiri path. - Frames counted from present 3300 on, in play; about 2,820 frames a run. - 3 runs of each version, alternated. | | before (main) | this PR | |---|---|---| | cache writes per start | 1 (1146 MB, +0 KB, 3.6–3.9 s) | 0 (`size asked in 0.0 ms`) | | game-disk 4 KB reads over 10 ms in play | 267 / 274 / 324 | 3 / 0 / 0 | | worst game-disk read | 150 / 118 / 153 ms | 68 / 7 / 6 ms | | frames over 33.4 ms | 8 / 11 / 22 | 3 / 2 / 1 | | frames over 25 ms | 36 / 41 / 49 | 27 / 18 / 24 | | time late in total | 883 / 992 / 1260 ms | 709 / 581 / 608 ms | | 1% low | 32.7 / 28.9 / 25.8 fps | 33.3 / 37.8 / 37.0 fps | The average stays at 60 fps (capped). The "before" runs also had line-by-line log writes; a separate series with this fix in both arms shows those cost much less than the cache rewrite. ## Tests `tests/test_pipeline_cache_save.py` has a new case, `unchanged`: - Data of the size read is not fetched or written, neither at exit nor in play. - After a failed write, and after a failed rename, the same data is written at the next save. - Once written, it is not written again. - A different size, smaller included, is written. The existing cases now grow the data between saves, where they expect a second write. - With `--baseline-source` set to `4518a20^`'s `vk_draw.cpp` (the helper the option is meant for), all 12 pass, the negative ones included. - `main`'s function fails `unchanged`. Also checked: - `RX_VK_ONLY=1 nx2/build.py --standalone`: 205 files, 0 failed, no warnings in `vk_draw.cpp`. - `tests/run_all.py`: 224 checks, 33 skipped, 2 failed for reasons of this PC, not this change: - `test_precompile_compute_guard.py` needs `windows.h` for an MSVC target. - `test_nup_paths.py` needs the symlink privilege. ## Left for later A save in play that does have something new still fetches and writes the whole file. A player's first session after an update can make about 12 such writes. A session cache for the pipelines the base cache misses, merged into the base at exit, would remove those. That is a larger change, so it is not in this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 16:40:23 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. The size-first check is cheap (no data copied), the saved size is only updated once write, fclose and rename all succeeded, and counting a failed fclose as a failed write is a real fix. The new unchanged case covers the failed-write, failed-rename and smaller-size paths, and the existing cases were adjusted sensibly. CI is green.

Two small notes, nothing blocking:

  • nit: if a driver ever returned the same size for different contents the save is skipped, as you say; fine given the cache only grows.
  • heads-up: #1772 also changes pipeline cache writing, so whichever lands second will need to reconcile with this size check.
Looks good. The size-first check is cheap (no data copied), the saved size is only updated once write, fclose and rename all succeeded, and counting a failed fclose as a failed write is a real fix. The new `unchanged` case covers the failed-write, failed-rename and smaller-size paths, and the existing cases were adjusted sensibly. CI is green. Two small notes, nothing blocking: - nit: if a driver ever returned the same size for different contents the save is skipped, as you say; fine given the cache only grows. - heads-up: #1772 also changes pipeline cache writing, so whichever lands second will need to reconcile with this size check.
Commenting is not possible because the repository is archived.
No description provided.