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

Closed
DFN-master wants to merge 7 commits from DFN-master/perf/shader-cache-zstd-clean into main
DFN-master commented 2026-11-10 11:35:15 +00:00 (Migrated from github.com)

The translated program cache is 6.86 GB on the reference OoT install (46,587 programs). With RX_VK_PROGRAM_CACHE_ZSTD=1 each record is compressed with zstd level 3 into hookshot_programs.zst.bin: the same cache is 1.83 GB (3.75x smaller). Records are unpacked when read; the precompile opens 46,587 keys and builds 13,109 programs in 1.8-2.7 s, with no "not whole"/"did not unpack" records in a 300 s run.

The compressed cache is a separate file with a 20-byte record head (key, stored size, kind); an existing hookshot_programs.bin is untouched, and the uncompressed format is unchanged. Off unless set. The zstd implementation is the repository's third_party/zbic/zstd.c (already used for NUP decompression), built into the Vulkan DLL as C.

Tested on an RTX 3080 / i7-11700 at 1080p: no fps regression (59-60 fps in the same scenes as the uncompressed cache), and the cache is 5.0 GB smaller on disk.

The translated program cache is 6.86 GB on the reference OoT install (46,587 programs). With `RX_VK_PROGRAM_CACHE_ZSTD=1` each record is compressed with zstd level 3 into `hookshot_programs.zst.bin`: the same cache is 1.83 GB (3.75x smaller). Records are unpacked when read; the precompile opens 46,587 keys and builds 13,109 programs in 1.8-2.7 s, with no "not whole"/"did not unpack" records in a 300 s run. The compressed cache is a separate file with a 20-byte record head (key, stored size, kind); an existing `hookshot_programs.bin` is untouched, and the uncompressed format is unchanged. Off unless set. The zstd implementation is the repository's `third_party/zbic/zstd.c` (already used for NUP decompression), built into the Vulkan DLL as C. Tested on an RTX 3080 / i7-11700 at 1080p: no fps regression (59-60 fps in the same scenes as the uncompressed cache), and the cache is 5.0 GB smaller on disk.
hckrman101 (Migrated from github.com) reviewed 2026-11-10 16:40:57 +00:00
hckrman101 (Migrated from github.com) left a comment

The record format and the read/write paths look right to me (separate file and idx, 20-byte head, kind bit 16 is free, the old cache is untouched). One build problem I think will stop this from linking, plus a couple of nits.

Main already builds zstd's decoder into the Vulkan DLL (third_party/zstd/lib/common + decompress, the vk_zstd_*.o jobs just above). third_party/zbic/zstd.c is Atmosphere's single-file zstd with the compressor and decoder both, non-static, so vk_zstd.o defines ZSTD_decompress, ZSTD_isError, the FSE/HUF/XXH64 symbols etc. a second time. I haven't built it, but I'd expect duplicate-symbol errors at the DLL link. Also, the two zstd.h files differ, so the -I third_party/zbic on vflags can shadow the other one for any vk file that includes zstd.h.

Suggestion: drop the zbic object and add the compress sources from the third_party/zstd tree (a compress/ directory, which isn't in the tree on main yet) to the existing zstd job list. Or, if you'd rather keep zbic, drop the decoder jobs and use the zbic build for everything, but then vk_imgui's font decoding needs checking.

Nits:

  • third_party/zbic/PROVENANCE.md says none of zbic is linked into the game; that stops being true.
  • zbic's zstd.c is GPLv2 (Atmosphere), worth checking that is fine for the DLL's licence.
The record format and the read/write paths look right to me (separate file and idx, 20-byte head, kind bit 16 is free, the old cache is untouched). One build problem I think will stop this from linking, plus a couple of nits. Main already builds zstd's decoder into the Vulkan DLL (`third_party/zstd/lib/common` + `decompress`, the `vk_zstd_*.o` jobs just above). `third_party/zbic/zstd.c` is Atmosphere's single-file zstd with the compressor and decoder both, non-static, so `vk_zstd.o` defines `ZSTD_decompress`, `ZSTD_isError`, the FSE/HUF/XXH64 symbols etc. a second time. I haven't built it, but I'd expect duplicate-symbol errors at the DLL link. Also, the two zstd.h files differ, so the `-I third_party/zbic` on vflags can shadow the other one for any vk file that includes zstd.h. Suggestion: drop the zbic object and add the compress sources from the third_party/zstd tree (a `compress/` directory, which isn't in the tree on main yet) to the existing zstd job list. Or, if you'd rather keep zbic, drop the decoder jobs and use the zbic build for everything, but then vk_imgui's font decoding needs checking. Nits: - `third_party/zbic/PROVENANCE.md` says none of zbic is linked into the game; that stops being true. - zbic's zstd.c is GPLv2 (Atmosphere), worth checking that is fine for the DLL's licence.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:40:57 +00:00

This duplicates the decoder objects that the vk_zstd_*.o jobs above already link (ZSTD_decompress, ZSTD_isError, FSE/HUF...). Expect duplicate symbols at link; compile only the compressor sources from third_party/zstd instead.

This duplicates the decoder objects that the `vk_zstd_*.o` jobs above already link (ZSTD_decompress, ZSTD_isError, FSE/HUF...). Expect duplicate symbols at link; compile only the compressor sources from third_party/zstd instead.
hckrman101 commented 2026-11-10 17:49:54 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
hckrman101 (Migrated from github.com) reviewed 2026-11-11 02:17:52 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the quick fix. Switching everything to the one single-file zstd fixes the duplicate symbols I flagged, and it's the simpler of the two options. I also checked the things it touches: both zstd.h files are 1.5.7, so vk_imgui.cpp picking up zbic's header first does no harm, and the font reader's calls (ZSTD_getFrameContentSize, the DCtx/DDict ones) are all in zbic's zstd.c. ZSTD_ZBIC_SUPPORT defaults to 0 there, so the GPLv2 Atmosphere ZBIC code isn't compiled into the DLL. Only stock zstd goes in, under its BSD option, so you can drop my licence nit. tests/test_shader_stamp_cache.py still passes on this shape too: its fixture has no zbic/zstd.c, so the new job just doesn't show up.

Still open:

  • Linux: vk_zstd.o is built without -fPIC (inline). The decoder job you removed had it, and so do vflags and the fonts job. This object goes into the Linux .so, and zstd's public functions are explicitly visibility("default"), so I'd expect the link to fail with "recompile with -fPIC". I haven't built it. The removed job also passed -DNDEBUG and -fvisibility=hidden, and I'd keep those for consistency (DEBUGLEVEL 0 already turns off zstd's asserts, so leaving out NDEBUG changes nothing).
  • Conflicts with main (mergeable: dirty). Main changed the cache code in vk_shader.cpp a fair bit today: disk_taken, program_cache_keys, the fallbacks in program_cache_load_some, descriptor_counts_fit. It also reworked build.py around the vk jobs. Please merge main in. CI hasn't run on this PR, probably because of the conflict, so after the merge it would be good to see it go green on both Windows and Linux. From a quick read, main's new readers all go through DiskRecord/program_cache_read/program_cache_each, which you already handle, and bit 16 of the kind word is still free there (main uses bits 0-3).

Nits:

  • if os.path.isfile(zstd_src): the fonts now depend on this object, so if the file is missing the job gets skipped quietly and the link fails with undefined ZSTD_* symbols. Making it unconditional would give a clearer error.
  • With the third_party/zstd objects gone, the -I third_party/zstd/lib in imgui_flags and those headers in imgui_deps no longer do anything (zbic's header wins). The zstd.c job also has b"" deps, so a change to zbic's headers won't rebuild it.
  • Still from last time: third_party/zbic/PROVENANCE.md says none of zbic is linked into the game. That's no longer true. A line saying the DLL builds zstd.c with ZBIC support off would cover it.
Thanks for the quick fix. Switching everything to the one single-file zstd fixes the duplicate symbols I flagged, and it's the simpler of the two options. I also checked the things it touches: both zstd.h files are 1.5.7, so vk_imgui.cpp picking up zbic's header first does no harm, and the font reader's calls (`ZSTD_getFrameContentSize`, the DCtx/DDict ones) are all in zbic's zstd.c. `ZSTD_ZBIC_SUPPORT` defaults to 0 there, so the GPLv2 Atmosphere ZBIC code isn't compiled into the DLL. Only stock zstd goes in, under its BSD option, so you can drop my licence nit. tests/test_shader_stamp_cache.py still passes on this shape too: its fixture has no zbic/zstd.c, so the new job just doesn't show up. Still open: - **Linux: `vk_zstd.o` is built without `-fPIC`** (inline). The decoder job you removed had it, and so do vflags and the fonts job. This object goes into the Linux .so, and zstd's public functions are explicitly `visibility("default")`, so I'd expect the link to fail with "recompile with -fPIC". I haven't built it. The removed job also passed `-DNDEBUG` and `-fvisibility=hidden`, and I'd keep those for consistency (DEBUGLEVEL 0 already turns off zstd's asserts, so leaving out NDEBUG changes nothing). - **Conflicts with main** (mergeable: dirty). Main changed the cache code in vk_shader.cpp a fair bit today: `disk_taken`, `program_cache_keys`, the fallbacks in `program_cache_load_some`, `descriptor_counts_fit`. It also reworked build.py around the vk jobs. Please merge main in. CI hasn't run on this PR, probably because of the conflict, so after the merge it would be good to see it go green on both Windows and Linux. From a quick read, main's new readers all go through `DiskRecord`/`program_cache_read`/`program_cache_each`, which you already handle, and bit 16 of the kind word is still free there (main uses bits 0-3). Nits: - `if os.path.isfile(zstd_src)`: the fonts now depend on this object, so if the file is missing the job gets skipped quietly and the link fails with undefined ZSTD_* symbols. Making it unconditional would give a clearer error. - With the third_party/zstd objects gone, the `-I third_party/zstd/lib` in imgui_flags and those headers in imgui_deps no longer do anything (zbic's header wins). The zstd.c job also has `b""` deps, so a change to zbic's headers won't rebuild it. - Still from last time: third_party/zbic/PROVENANCE.md says none of zbic is linked into the game. That's no longer true. A line saying the DLL builds zstd.c with ZBIC support off would cover it.
hckrman101 (Migrated from github.com) commented 2026-11-11 02:17:53 +00:00

On Linux this object goes into the .so but gets no -fPIC. The removed zflags had (["-fPIC"] if LINUX else []), and vflags and the fonts job just above have it too. I'd expect a relocation error at the link (zstd's API symbols are default visibility). Suggest target() + CPU + (["-fPIC"] if LINUX else []) + ["-O2", "-g0", "-DNDEBUG", "-Wno-everything", "-fvisibility=hidden", ...].

On Linux this object goes into the .so but gets no `-fPIC`. The removed `zflags` had `(["-fPIC"] if LINUX else [])`, and vflags and the fonts job just above have it too. I'd expect a relocation error at the link (zstd's API symbols are default visibility). Suggest `target() + CPU + (["-fPIC"] if LINUX else []) + ["-O2", "-g0", "-DNDEBUG", "-Wno-everything", "-fvisibility=hidden", ...]`.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 02:18:00 +00:00
hckrman101 (Migrated from github.com) commented 2026-11-11 02:18:00 +00:00

Fixed in 93b11c8: the third_party/zstd decoder objects are gone and zbic's single file serves both. One thing left on the new job is the missing -fPIC on Linux, see the new comment.

Fixed in 93b11c8: the third_party/zstd decoder objects are gone and zbic's single file serves both. One thing left on the new job is the missing -fPIC on Linux, see the new comment.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 17:35:41 +00:00
hckrman101 (Migrated from github.com) commented 2026-11-11 17:35:41 +00:00

Still missing after the rebase: the job at line 994 still has no (["-fPIC"] if LINUX else []) (nor -DNDEBUG/-fvisibility=hidden). The appimage job should show it.

Still missing after the rebase: the job at line 994 still has no `(["-fPIC"] if LINUX else [])` (nor `-DNDEBUG`/`-fvisibility=hidden`). The appimage job should show it.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 17:35:43 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the rebase. It merges cleanly now, and the Windows llvm-mingw link check is green, so the single-zstd build links on Windows. The rest of CI is still running.

Still open from last time:

  • -fPIC on Linux for vk_zstd.o (build.py:994, replied in the thread). The appimage job should show whether it links.
  • Nits, all still there: the os.path.isfile(zstd_src) guard (if the file is missing, the fonts fail later with undefined ZSTD_* symbols); -I third_party/zstd/lib in imgui_flags and those headers in imgui_deps don't do anything now; the zstd.c job's b"" deps; and third_party/zbic/PROVENANCE.md still says none of zbic is linked in.

New:

  • seed_compute reads the cache records raw. The rebase brought in a third reader of the records, and it doesn't unpack them. The "variants kept already" loop in seed_compute (vk_shader.cpp around line 2681) reads disk_records[key] straight from disk_in and parses rd.get<uint32_t>(); rd.list(q.answers) from it. With the zstd cache, a compressed compute record gives the low word of the unpacked size, then frame bytes. Reader's bounds checks keep this from crashing, but the kept variants aren't recognised. So I'd expect every prepare to translate the compute variants again and append them again, and the .zst.bin grows each time. The unpack step now exists twice (program_cache_read, program_cache_each). Could you make it one helper (record bytes in, payload out) and call it here as well? Then the next reader can't miss it. A test that writes and reads back one compressed record (with real zstd, or a stub that copies) would also catch this. The cleanup test only runs with disk_zstd = false.

How this fits with #2309: the two PRs choose opposite zstd libraries for the same DLL. #2309 keeps the third_party/zstd decoder and adds its compress/ sources. This PR removes those and builds zbic's zstd.c instead. With both merged as they are, the DLL gets the same duplicate symbols again, so whichever lands second has to switch to the other's library. If #2309 goes first, the simplest route here is to drop the zbic job and the -I third_party/zbic and use its objects. That also fixes the -fPIC, visibility and PROVENANCE points above. The formats don't clash: #2309 packs the SPIR-V only in memory and leaves the disk record unchanged, and this PR compresses the record on disk. Its stamp bump only means the .zst.bin is rebuilt once too. @NotRexed, which order you take them in is your call.

Thanks for the rebase. It merges cleanly now, and the Windows llvm-mingw link check is green, so the single-zstd build links on Windows. The rest of CI is still running. Still open from last time: - **`-fPIC` on Linux** for `vk_zstd.o` (build.py:994, replied in the thread). The appimage job should show whether it links. - Nits, all still there: the `os.path.isfile(zstd_src)` guard (if the file is missing, the fonts fail later with undefined ZSTD_* symbols); `-I third_party/zstd/lib` in imgui_flags and those headers in imgui_deps don't do anything now; the zstd.c job's `b""` deps; and third_party/zbic/PROVENANCE.md still says none of zbic is linked in. New: - **`seed_compute` reads the cache records raw.** The rebase brought in a third reader of the records, and it doesn't unpack them. The "variants kept already" loop in `seed_compute` (vk_shader.cpp around line 2681) reads `disk_records[key]` straight from `disk_in` and parses `rd.get<uint32_t>(); rd.list(q.answers)` from it. With the zstd cache, a compressed compute record gives the low word of the unpacked size, then frame bytes. Reader's bounds checks keep this from crashing, but the kept variants aren't recognised. So I'd expect every prepare to translate the compute variants again and append them again, and the .zst.bin grows each time. The unpack step now exists twice (`program_cache_read`, `program_cache_each`). Could you make it one helper (record bytes in, payload out) and call it here as well? Then the next reader can't miss it. A test that writes and reads back one compressed record (with real zstd, or a stub that copies) would also catch this. The cleanup test only runs with `disk_zstd = false`. How this fits with #2309: the two PRs choose opposite zstd libraries for the same DLL. #2309 keeps the third_party/zstd decoder and adds its `compress/` sources. This PR removes those and builds zbic's zstd.c instead. With both merged as they are, the DLL gets the same duplicate symbols again, so whichever lands second has to switch to the other's library. If #2309 goes first, the simplest route here is to drop the zbic job and the `-I third_party/zbic` and use its objects. That also fixes the -fPIC, visibility and PROVENANCE points above. The formats don't clash: #2309 packs the SPIR-V only in memory and leaves the disk record unchanged, and this PR compresses the record on disk. Its stamp bump only means the .zst.bin is rebuilt once too. @NotRexed, which order you take them in is your call.
Joey-Magnus (Migrated from github.com) requested changes 2026-11-11 21:46:03 +00:00
Joey-Magnus (Migrated from github.com) left a comment

The seed_compute point from the last review is still open, and I confirmed it on the merge with main. seed_compute (vk_shader.cpp:2110) reads each disk_records entry straight from disk_in and parses rd.get<uint32_t>(); rd.list(q.answers). A record written by program_cache_write with kCompressedKind starts with the 8-byte unpacked size and then the zstd frame. So with RX_VK_PROGRAM_CACHE_ZSTD=1, the compute variants already kept are never recognised: each prepare translates them again and appends them, and hookshot_programs.zst.bin grows every time. Please move the unpack into one helper (record bytes in, payload out), use it in program_cache_read, program_cache_each and here, and add a test that writes and reads back one compressed record.

The appimage job links, so the missing -fPIC on vk_zstd.o (build.py:993) isn't fatal, but I'd still match the old job's flags. The os.path.isfile(zstd_src) guard and the PROVENANCE line are still open.

#2309 picks the other zstd library (third_party/zstd plus compress/). Whichever lands second has to switch, and that ordering is the maintainer's call.

The `seed_compute` point from the last review is still open, and I confirmed it on the merge with main. `seed_compute` (vk_shader.cpp:2110) reads each `disk_records` entry straight from `disk_in` and parses `rd.get<uint32_t>(); rd.list(q.answers)`. A record written by `program_cache_write` with `kCompressedKind` starts with the 8-byte unpacked size and then the zstd frame. So with `RX_VK_PROGRAM_CACHE_ZSTD=1`, the compute variants already kept are never recognised: each prepare translates them again and appends them, and `hookshot_programs.zst.bin` grows every time. Please move the unpack into one helper (record bytes in, payload out), use it in `program_cache_read`, `program_cache_each` and here, and add a test that writes and reads back one compressed record. The appimage job links, so the missing `-fPIC` on `vk_zstd.o` (build.py:993) isn't fatal, but I'd still match the old job's flags. The `os.path.isfile(zstd_src)` guard and the PROVENANCE line are still open. #2309 picks the other zstd library (third_party/zstd plus `compress/`). Whichever lands second has to switch, and that ordering is the maintainer's call.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 22:21:09 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for following main's move of the cache code into vk_progcache.cpp. The 20-byte head now carries through the index check, the scan and the read path, the tests follow it, and CI is all green (appimage included). It also merges cleanly with main right now.

Still open, same as Joey-Magnus's review on this head:

  • seed_compute still reads the records raw (vk_shader.cpp:2110). It reads disk_records[key] straight from disk_in and parses rd.get<uint32_t>(); rd.list(q.answers). A compressed record starts with the 8-byte unpacked size and then the frame, so with RX_VK_PROGRAM_CACHE_ZSTD=1 the kept compute variants aren't recognised, every prepare translates them again, and the .zst.bin grows each time. The unpack is still copied in program_cache_read and program_cache_each. One helper (record bytes in, payload out) used by all three, plus a test that writes and reads back one compressed record, is what I'd want before this goes in. The cleanup test only covers disk_zstd = false.

Smaller, still there:

  • vk_zstd.o has no -fPIC/-DNDEBUG/-fvisibility=hidden (build.py:994). The appimage job links, so it isn't fatal, but I'd still match the old job's flags.
  • The os.path.isfile(zstd_src) guard: if the file is missing, the fonts fail later with undefined ZSTD_* symbols.
  • third_party/zbic/PROVENANCE.md still says none of zbic is linked into the game.
  • Nits: -I third_party/zstd/lib in imgui_flags and those headers in imgui_deps no longer do anything, and the zstd.c job has b"" deps.

#2309 (still open) picks the other zstd library, so whichever lands second has to switch; that ordering stays with @NotRexed.

Thanks for following main's move of the cache code into vk_progcache.cpp. The 20-byte head now carries through the index check, the scan and the read path, the tests follow it, and CI is all green (appimage included). It also merges cleanly with main right now. Still open, same as Joey-Magnus's review on this head: - **`seed_compute` still reads the records raw** (vk_shader.cpp:2110). It reads `disk_records[key]` straight from `disk_in` and parses `rd.get<uint32_t>(); rd.list(q.answers)`. A compressed record starts with the 8-byte unpacked size and then the frame, so with `RX_VK_PROGRAM_CACHE_ZSTD=1` the kept compute variants aren't recognised, every prepare translates them again, and the .zst.bin grows each time. The unpack is still copied in `program_cache_read` and `program_cache_each`. One helper (record bytes in, payload out) used by all three, plus a test that writes and reads back one compressed record, is what I'd want before this goes in. The cleanup test only covers `disk_zstd = false`. Smaller, still there: - `vk_zstd.o` has no `-fPIC`/`-DNDEBUG`/`-fvisibility=hidden` (build.py:994). The appimage job links, so it isn't fatal, but I'd still match the old job's flags. - The `os.path.isfile(zstd_src)` guard: if the file is missing, the fonts fail later with undefined ZSTD_* symbols. - `third_party/zbic/PROVENANCE.md` still says none of zbic is linked into the game. - Nits: `-I third_party/zstd/lib` in imgui_flags and those headers in imgui_deps no longer do anything, and the zstd.c job has `b""` deps. #2309 (still open) picks the other zstd library, so whichever lands second has to switch; that ordering stays with @NotRexed.
DFN-master commented 2026-11-11 22:42:18 +00:00 (Migrated from github.com)

Thanks — the open points are addressed in the new head (7f33ccf1, rebased on a5b955fb):

  • seed_compute: the unpack now lives in one program_cache_payload(stored, stored_size, kind, ...) helper, and program_cache_read, program_cache_each and seed_compute all use it. The cleanup fixture now writes and reads back one compressed record through the production loader (a copy stub for zstd), so the compressed round trip is exercised.
  • vk_zstd.o: built with -fPIC on Linux, -DNDEBUG and -fvisibility=hidden; the job is unconditional, and the zbic headers are its dependencies.
  • imgui: the unused -I third_party/zstd/lib and zstd headers are gone; the zbic headers are what it includes and what its deps name.
  • PROVENANCE.md now says the DLL builds zstd.c with ZSTD_ZBIC_SUPPORT off (stock zstd, BSD; not the GPLv2 ZBIC parts).

Local checks: test_program_cache_index, test_program_cache_reader, test_program_record_head, test_program_cache_cleanup (9 cases, including the compressed one), and a Windows RX_VK_ONLY build of the DLL pass.

On #2309: whichever lands second switches libraries — agreed. If #2309 goes first, I'll drop the zbic job and the -I third_party/zbic and use its compressor objects; the on-disk record format does not depend on which library compresses it.

Thanks — the open points are addressed in the new head (`7f33ccf1`, rebased on `a5b955fb`): - **`seed_compute`**: the unpack now lives in one `program_cache_payload(stored, stored_size, kind, ...)` helper, and `program_cache_read`, `program_cache_each` and `seed_compute` all use it. The cleanup fixture now writes and reads back one compressed record through the production loader (a copy stub for zstd), so the compressed round trip is exercised. - **`vk_zstd.o`**: built with `-fPIC` on Linux, `-DNDEBUG` and `-fvisibility=hidden`; the job is unconditional, and the zbic headers are its dependencies. - **imgui**: the unused `-I third_party/zstd/lib` and zstd headers are gone; the zbic headers are what it includes and what its deps name. - **PROVENANCE.md** now says the DLL builds `zstd.c` with `ZSTD_ZBIC_SUPPORT` off (stock zstd, BSD; not the GPLv2 ZBIC parts). Local checks: `test_program_cache_index`, `test_program_cache_reader`, `test_program_record_head`, `test_program_cache_cleanup` (9 cases, including the compressed one), and a Windows `RX_VK_ONLY` build of the DLL pass. On **#2309**: whichever lands second switches libraries — agreed. If #2309 goes first, I'll drop the zbic job and the `-I third_party/zbic` and use its compressor objects; the on-disk record format does not depend on which library compresses it.
DFN-master commented 2026-11-11 23:43:32 +00:00 (Migrated from github.com)

Test report — zstd program cache

Machine

  • Windows 11, 64-bit.
  • Intel Core i7-11700, 8 cores / 16 threads, 2.5 GHz base; 32 GB RAM.
  • NVIDIA GeForce RTX 3080 10 GB; 1920x1080 @ 60 Hz.

Build and settings

  • Game: v2.0.11-1024-g9bc83387 (source 5d0560d3), i.e. 4a53c897 + the zstd commit + BC7, installed from this branch's DLL.
  • hookshot_env.txt: RX_VK_PROGRAM_CACHE_ZSTD=1, RX_VK_ASTC_BC7=1, RX_VK_SYNC_LIVE=1, RX_VK_READBACK_NO_CATCHUP=1, RX_VK_WB_MAX_KB=1024, RX_VK_VRAM_MB=4096, RX_VK_COMPACT_PCT=90.
  • Options: quality_preset=0, 1920x1080, frame_generation=3, frame_pacing=1, nvidia_reflex=1, hdr=0, frame_rate_cap=0, dlss=0, upscaling=2, sharpness=40.
  • Runs: headless (HOS_HEADLESS=1, HOS_AUDIO=0), scripted input, 240 s per scene, same save and caches. FPS is the renderer's presents rate; VRAM/texture/readback numbers are the renderer's own lines.

Checks

  • test_program_cache_index.py, test_program_cache_reader.py, test_program_record_head.py, test_program_cache_cleanup.py (9 cases, including a compressed record written and read back through the production loader) pass locally.
  • test_shader_stamp_cache.py passes with the fixture's third_party/zbic stubs (fixed in d9ec233e; the Ubuntu part 1 failure was that fixture, not the cache).
  • Windows RX_VK_ONLY DLL build: pass; the single zbic zstd.c links with -fPIC/-DNDEBUG/-fvisibility=hidden and no duplicate symbols. The full game build uses it too.
  • seed_compute now uses program_cache_payload, so compressed compute records are recognised instead of translated and appended again.

Cache and prepare

zstd cache uncompressed reference
program cache after prepare 1,803,015,235 B (1.80 GB) ~6.9 GB
program cache after the play tests 2,064,169,947 B (2.06 GB; new programs met in play) —
prepare time 102 s 412 s (older 7759 build)

The 53bc translator update (not zstd) prepared 50k+ pipelines in 1,192 s in a separate test; that is a translator change, not this cache.

In game (4a53 + zstd + BC7, 4 GB texture budget, pct 90)

scene fps VRAM textures readbacks
Kakariko Village (2,4xx draws) 50.6–52.0 6.86 / 8.85 GB 3.66 GB 0.02 ms
Hyrule Field 59.5–60.0 7.0 / 8.5 GB 3.2–3.4 GB 0.02 ms
Lake Hylia (adult, 3,3xx draws) 45–47 7.5 / 9.0 GB 3.4 GB 0.02 ms

memory sync is 0.8–1.7 ms in these scenes with RX_VK_READBACK_NO_CATCHUP=1; without it the newer 53bc base waits 1.3–1.6 ms per read-back and Hyrule Field falls to 50–52 fps (0.02 ms and 60 fps with it on 4a53). That read-back difference is why my playable fork stays on 4a53; it is not a zstd effect.

Caveat: one Windows/NVIDIA machine at 1080p; no AMD/Intel run. The program cache includes programs the play tests met, so its size grows with play.

On #2309: if it lands first I will drop the zbic job and use its compressor objects; the record format is independent of the library.

## Test report — zstd program cache ### Machine - Windows 11, 64-bit. - Intel Core i7-11700, 8 cores / 16 threads, 2.5 GHz base; 32 GB RAM. - NVIDIA GeForce RTX 3080 10 GB; 1920x1080 @ 60 Hz. ### Build and settings - Game: `v2.0.11-1024-g9bc83387` (source 5d0560d3), i.e. `4a53c897` + the zstd commit + BC7, installed from this branch's DLL. - `hookshot_env.txt`: `RX_VK_PROGRAM_CACHE_ZSTD=1`, `RX_VK_ASTC_BC7=1`, `RX_VK_SYNC_LIVE=1`, `RX_VK_READBACK_NO_CATCHUP=1`, `RX_VK_WB_MAX_KB=1024`, `RX_VK_VRAM_MB=4096`, `RX_VK_COMPACT_PCT=90`. - Options: `quality_preset=0`, 1920x1080, `frame_generation=3`, `frame_pacing=1`, `nvidia_reflex=1`, `hdr=0`, `frame_rate_cap=0`, `dlss=0`, `upscaling=2`, `sharpness=40`. - Runs: headless (`HOS_HEADLESS=1`, `HOS_AUDIO=0`), scripted input, 240 s per scene, same save and caches. FPS is the renderer's `presents` rate; VRAM/texture/readback numbers are the renderer's own lines. ### Checks - `test_program_cache_index.py`, `test_program_cache_reader.py`, `test_program_record_head.py`, `test_program_cache_cleanup.py` (9 cases, including a compressed record written and read back through the production loader) pass locally. - `test_shader_stamp_cache.py` passes with the fixture's `third_party/zbic` stubs (fixed in `d9ec233e`; the Ubuntu part 1 failure was that fixture, not the cache). - Windows `RX_VK_ONLY` DLL build: pass; the single zbic `zstd.c` links with `-fPIC`/`-DNDEBUG`/`-fvisibility=hidden` and no duplicate symbols. The full game build uses it too. - `seed_compute` now uses `program_cache_payload`, so compressed compute records are recognised instead of translated and appended again. ### Cache and prepare | | zstd cache | uncompressed reference | |---|---:|---:| | program cache after prepare | 1,803,015,235 B (1.80 GB) | ~6.9 GB | | program cache after the play tests | 2,064,169,947 B (2.06 GB; new programs met in play) | — | | prepare time | 102 s | 412 s (older 7759 build) | The 53bc translator update (not zstd) prepared 50k+ pipelines in 1,192 s in a separate test; that is a translator change, not this cache. ### In game (4a53 + zstd + BC7, 4 GB texture budget, pct 90) | scene | fps | VRAM | textures | readbacks | |---|---:|---:|---:|---:| | Kakariko Village (2,4xx draws) | 50.6–52.0 | 6.86 / 8.85 GB | 3.66 GB | 0.02 ms | | Hyrule Field | 59.5–60.0 | 7.0 / 8.5 GB | 3.2–3.4 GB | 0.02 ms | | Lake Hylia (adult, 3,3xx draws) | 45–47 | 7.5 / 9.0 GB | 3.4 GB | 0.02 ms | `memory sync` is 0.8–1.7 ms in these scenes with `RX_VK_READBACK_NO_CATCHUP=1`; without it the newer 53bc base waits 1.3–1.6 ms per read-back and Hyrule Field falls to 50–52 fps (0.02 ms and 60 fps with it on 4a53). That read-back difference is why my playable fork stays on 4a53; it is not a zstd effect. Caveat: one Windows/NVIDIA machine at 1080p; no AMD/Intel run. The program cache includes programs the play tests met, so its size grows with play. On #2309: if it lands first I will drop the zbic job and use its compressor objects; the record format is independent of the library.
Owner

@DFN-master commented on GitHub · 2026-11-12 10:14 UTC · original

Cache numbers on the newest main, same machine (16 GB/s-class RAM, 10 GB NVIDIA, 1080p), with this PR plus the readback and BC7 changes in one build (55b976c0 + #2602 + this cache + BC7):

  • fresh cache: 46,666 programs prepared in 84.7 s (106.6 s on the first run with the caches cold), 1.68 GB hookshot_programs.zst.bin + 1 MB index;
  • the next starts read 46,670 keys and 34,327 programs from it; Hyrule Field, Kakariko and Lake Hylia then ran with no cache errors;
  • prepare on the same build without compression was 237.8 s and 6.45 GB for the same 38,809 state-list programs, so this is ~2.8x faster and ~3.8x smaller here.

One note for anyone testing with the force-kill harness: an earlier VK_ERROR_DEVICE_LOST in my runs was stale mkw_vk_pipelines_play_*.bin files from older builds, not the program cache; removing them made the stack stable.

> [@DFN-master](https://github.com/DFN-master) commented on GitHub · 2026-11-12 10:14 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1630#issuecomment-6820822906) Cache numbers on the newest main, same machine (16 GB/s-class RAM, 10 GB NVIDIA, 1080p), with this PR plus the readback and BC7 changes in one build (`55b976c0` + #2602 + this cache + BC7): - fresh cache: **46,666 programs prepared in 84.7 s** (106.6 s on the first run with the caches cold), **1.68 GB** `hookshot_programs.zst.bin` + 1 MB index; - the next starts read 46,670 keys and 34,327 programs from it; Hyrule Field, Kakariko and Lake Hylia then ran with no cache errors; - prepare on the same build without compression was 237.8 s and 6.45 GB for the same 38,809 state-list programs, so this is ~2.8x faster and ~3.8x smaller here. One note for anyone testing with the force-kill harness: an earlier `VK_ERROR_DEVICE_LOST` in my runs was stale `mkw_vk_pipelines_play_*.bin` files from older builds, not the program cache; removing them made the stack stable. <!-- github-sync:c:6820822906 -->
Owner

@DFN-master commented on GitHub · 2026-11-12 11:19 UTC · original

Rebased onto main (6dcc15c7); CI is now all green (Ubuntu and Windows Python, Windows Vulkan, DLSS/FSR link check, appimage, SPIR-V, builder, dav1d, lint).

On the newest main, with BC7 (#2226) and the readback fix (#2602) already merged, the cache on this machine (i7-11700, RTX 3080 10 GB, 32 GB, 1080p):

  • fresh prepare: 46,666 programs in 85.3 s, 1.68 GB hookshot_programs.zst.bin + 1 MB index;
  • next starts: 46,670 keys and 34,378 programs read from it;
  • Hyrule Field: 59.99 fps avg (59.1-60.3), 1352-1361/3000 readbacks at 0.01-0.02 ms;
  • Kakariko Village: 51.9 fps, 0.11-0.13 ms (BC7's GPU unswizzle makes the readback part wait fall back there; town is CPU-bound);
  • Lake Hylia: 46.75 fps avg (42.7-47.9), 0.02-0.03 ms.

No cache errors, no device loss. Config: 4 GB texture budget, RX_VK_COMPACT_PCT=90, RX_VK_READBACK_NO_CATCHUP=1, RX_VK_WB_MAX_KB=1024, RX_VK_SYNC_LIVE=0, BC7 on.

One note for the 10 GB card: main now preallocates 15% of VRAM as 128 MB image-pool blocks (#2438, 11 blocks = 1.4 GB here). With the 4 GB texture budget that sits near the 90% hold; the default is still better here (Hyrule Field 59.99 against 57.9 with RX_VK_IMAGE_PREALLOC=0), but it is worth knowing when a card is tight.

> [@DFN-master](https://github.com/DFN-master) commented on GitHub · 2026-11-12 11:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1630#issuecomment-6822166297) Rebased onto main (`6dcc15c7`); CI is now all green (Ubuntu and Windows Python, Windows Vulkan, DLSS/FSR link check, appimage, SPIR-V, builder, dav1d, lint). On the newest main, with BC7 (`#2226`) and the readback fix (`#2602`) already merged, the cache on this machine (i7-11700, RTX 3080 10 GB, 32 GB, 1080p): - fresh prepare: **46,666 programs in 85.3 s**, **1.68 GB** `hookshot_programs.zst.bin` + 1 MB index; - next starts: 46,670 keys and 34,378 programs read from it; - Hyrule Field: **59.99 fps avg** (59.1-60.3), 1352-1361/3000 readbacks at 0.01-0.02 ms; - Kakariko Village: 51.9 fps, 0.11-0.13 ms (BC7's GPU unswizzle makes the readback part wait fall back there; town is CPU-bound); - Lake Hylia: **46.75 fps avg** (42.7-47.9), 0.02-0.03 ms. No cache errors, no device loss. Config: 4 GB texture budget, `RX_VK_COMPACT_PCT=90`, `RX_VK_READBACK_NO_CATCHUP=1`, `RX_VK_WB_MAX_KB=1024`, `RX_VK_SYNC_LIVE=0`, BC7 on. One note for the 10 GB card: main now preallocates 15% of VRAM as 128 MB image-pool blocks (`#2438`, 11 blocks = 1.4 GB here). With the 4 GB texture budget that sits near the 90% hold; the default is still better here (Hyrule Field 59.99 against 57.9 with `RX_VK_IMAGE_PREALLOC=0`), but it is worth knowing when a card is tight. <!-- github-sync:c:6822166297 -->
Owner

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

Everything from my last review is in. seed_compute (vk_shader.cpp:2211), program_cache_read and program_cache_each all go through program_cache_payload now, so compressed compute records are recognised instead of being translated and appended again. The cleanup test writes a compressed record and reads it back. The zstd job has -fPIC, -DNDEBUG and -fvisibility=hidden, and PROVENANCE.md says what is linked. I merged d61c65c into current main (clean) and ran the program_cache, program_record and shader_stamp checks: 7 pass. CI is green on this head.

I also checked that nothing changes without the switch. The 16-byte head, the HSP1 magic and hookshot_programs.idx are as before. The zstd cache has its own .zst.bin and .zst.idx, so turning it on or off never reads the other file. zbic's zstd.c defaults ZSTD_ZBIC_SUPPORT to 0, so the Atmosphere code stays out.

One thing for maintainers: this replaces the third_party/zstd decoder objects that the fonts used with zbic's zstd.c, for everyone. #2309 uses third_party/zstd, so whichever lands second has to switch.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1630#pullrequestreview-5876200973) Everything from my last review is in. seed_compute (vk_shader.cpp:2211), program_cache_read and program_cache_each all go through program_cache_payload now, so compressed compute records are recognised instead of being translated and appended again. The cleanup test writes a compressed record and reads it back. The zstd job has -fPIC, -DNDEBUG and -fvisibility=hidden, and PROVENANCE.md says what is linked. I merged d61c65c into current main (clean) and ran the program_cache, program_record and shader_stamp checks: 7 pass. CI is green on this head. I also checked that nothing changes without the switch. The 16-byte head, the HSP1 magic and hookshot_programs.idx are as before. The zstd cache has its own .zst.bin and .zst.idx, so turning it on or off never reads the other file. zbic's zstd.c defaults ZSTD_ZBIC_SUPPORT to 0, so the Atmosphere code stays out. One thing for maintainers: this replaces the third_party/zstd decoder objects that the fonts used with zbic's zstd.c, for everyone. #2309 uses third_party/zstd, so whichever lands second has to switch. <!-- github-sync:r:5876200973 -->
github-sync closed this pull request 2026-11-12 17:57:42 +00:00
Commenting is not possible because the repository is archived.
No description provided.