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
pull from: DFN-master/perf/shader-cache-zstd-clean
merge into: MagicBeanDev:main
MagicBeanDev:main
MagicBeanDev:github/pr/2893
MagicBeanDev:github/pr/2890
MagicBeanDev:github/pr/2887
MagicBeanDev:github/pr/2878
MagicBeanDev:github/pr/2875
MagicBeanDev:github/pr/2874
MagicBeanDev:github/pr/2873
MagicBeanDev:github/pr/2871
MagicBeanDev:github/pr/2756
MagicBeanDev:github/pr/2897
MagicBeanDev:github/pr/2896
MagicBeanDev:github/pr/2718
MagicBeanDev:github/pr/2842
MagicBeanDev:github/pr/2691
MagicBeanDev:github/pr/2895
MagicBeanDev:github/pr/2743
MagicBeanDev:github/pr/2765
MagicBeanDev:github/pr/2708
MagicBeanDev:github/pr/2732
MagicBeanDev:github/pr/2755
MagicBeanDev:github/pr/2817
MagicBeanDev:github/pr/2876
MagicBeanDev:release-please--branches--main
MagicBeanDev:release-please--branches--main--release-notes
MagicBeanDev:github/pr/2778
MagicBeanDev:github/pr/2888
MagicBeanDev:github/pr/2866
MagicBeanDev:github/pr/2877
MagicBeanDev:github/pr/2800
MagicBeanDev:github/pr/2829
MagicBeanDev:github/pr/2850
MagicBeanDev:github/pr/2889
MagicBeanDev:github/pr/2882
MagicBeanDev:github/pr/2860
MagicBeanDev:github/pr/2849
MagicBeanDev:github/pr/2886
MagicBeanDev:github/pr/2794
MagicBeanDev:github/pr/2840
MagicBeanDev:github/pr/2830
MagicBeanDev:github/pr/2834
MagicBeanDev:github/pr/2719
MagicBeanDev:github/pr/2742
MagicBeanDev:github/pr/2741
MagicBeanDev:github/pr/2820
MagicBeanDev:github/pr/2771
MagicBeanDev:github/pr/2859
MagicBeanDev:github/pr/2858
MagicBeanDev:github/pr/2823
MagicBeanDev:github/pr/2757
MagicBeanDev:github/pr/2769
MagicBeanDev:github/pr/2844
MagicBeanDev:github/pr/2843
MagicBeanDev:github/pr/2673
MagicBeanDev:github/pr/2855
MagicBeanDev:github/pr/2739
MagicBeanDev:github/pr/2807
MagicBeanDev:github/pr/2851
MagicBeanDev:github/pr/2789
MagicBeanDev:github/pr/2814
MagicBeanDev:github/pr/2818
MagicBeanDev:github/pr/2774
MagicBeanDev:github/pr/2772
MagicBeanDev:github/pr/2750
MagicBeanDev:github/pr/2786
MagicBeanDev:github/pr/2785
MagicBeanDev:github/pr/2782
MagicBeanDev:github/pr/2779
MagicBeanDev:github/pr/2775
MagicBeanDev:github/pr/2706
MagicBeanDev:github/pr/2766
MagicBeanDev:github/pr/2747
MagicBeanDev:github/pr/2759
MagicBeanDev:github/pr/2745
MagicBeanDev:github/pr/2740
MagicBeanDev:github/pr/2733
MagicBeanDev:github/pr/2725
MagicBeanDev:github/pr/2727
MagicBeanDev:github/pr/2728
MagicBeanDev:github/pr/2729
MagicBeanDev:github/pr/2721
MagicBeanDev:github/pr/2722
MagicBeanDev:github/pr/2715
MagicBeanDev:github/pr/2714
MagicBeanDev:github/pr/2707
MagicBeanDev:github/pr/2703
MagicBeanDev:github/pr/2705
MagicBeanDev:github/pr/2697
MagicBeanDev:github/pr/2679
MagicBeanDev:github/pr/2676
MagicBeanDev:github/pr/2669
MagicBeanDev:docs/what-hookshot-connects-to
MagicBeanDev:fix/hdr-encode-test
MagicBeanDev:fix/main-tests
MagicBeanDev:fix/play-cache-files
MagicBeanDev:perf/game-barrier-order
MagicBeanDev:feat/heap-lazy
MagicBeanDev:feat/texture-detail
MagicBeanDev:perf/astc-opaque-bc1
MagicBeanDev:fix/fs-dir-lock
MagicBeanDev:fix/framegen-native-kept
MagicBeanDev:fix/update-no-downgrade
MagicBeanDev:port/k10-gcn-f16-mirrors
MagicBeanDev:fix/framegen-mult-cap
MagicBeanDev:chore/release-as-2.1.1
MagicBeanDev:perf/pcache-load-switch
MagicBeanDev:fix/prepare-progress-log
MagicBeanDev:fix/update-closes-stale-game
MagicBeanDev:fix/hdr-scaled-sampled
MagicBeanDev:perf/first-start-preset
MagicBeanDev:perf/zero-upload
MagicBeanDev:fix/vcore-escape-2s
MagicBeanDev:test/fake-gh-atomic
MagicBeanDev:perf/batch-idle-flush
MagicBeanDev:test/finish-stub-a
MagicBeanDev:chore/record-2.1.0
MagicBeanDev:fix/main-red-tests
MagicBeanDev:fix/test-display-startup-driver-cache
MagicBeanDev:feat/mesh-shader-mode
MagicBeanDev:fix/menu-probes-at-startup
MagicBeanDev:fix/fps-counter-classic
MagicBeanDev:fix/test-log-batch-flake
MagicBeanDev:fix/narrow-screen-hud
MagicBeanDev:ci/prereleases
MagicBeanDev:release/v2.0.15
MagicBeanDev:hotfix/v2.0.15
MagicBeanDev:fix/present-ahead-replay
MagicBeanDev:release/v2.0.13
MagicBeanDev:fix/bracket-paths-2013
MagicBeanDev:fix/hdr-overlays-2013
MagicBeanDev:fix/fps-counter-scale-2013
MagicBeanDev:fix/stick-seen-at-rest
MagicBeanDev:hotfix/menu-fits-screen
MagicBeanDev:fix/f1-always-opens-menu
MagicBeanDev:fix/ini-keep-lines
MagicBeanDev:claude/cranky-pike-7cf247
MagicBeanDev:perf/texture-answer-flat
MagicBeanDev:fix/stamp-test-modules
MagicBeanDev:fix/appimage-fonts
MagicBeanDev:fix/display-awake
MagicBeanDev:release/v2.0.12
MagicBeanDev:fix/discord-windows
MagicBeanDev:rc/v2.0.12-tests
MagicBeanDev:fix/dlss-keep-feature
MagicBeanDev:rc/v2.0.12
MagicBeanDev:button-icons
No reviewers
Labels
Clear labels
accessibility
Barrier affecting people with disabilities
autorelease: pending
autorelease: tagged
bug
Something isn't working
dependencies
Pull requests that update a dependency file
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
github_actions
Pull requests that update GitHub Actions code
good first issue
Good for newcomers
help wanted
Extra attention is needed
invalid
This doesn't seem right
question
Further information is requested
testing wanted
wontfix
This will not be worked on
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1630
Loading…
Reference in a new issue
No description provided.
Delete branch "DFN-master/perf/shader-cache-zstd-clean"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The translated program cache is 6.86 GB on the reference OoT install (46,587 programs). With
RX_VK_PROGRAM_CACHE_ZSTD=1each record is compressed with zstd level 3 intohookshot_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.binis untouched, and the uncompressed format is unchanged. Off unless set. The zstd implementation is the repository'sthird_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 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, thevk_zstd_*.ojobs just above).third_party/zbic/zstd.cis Atmosphere's single-file zstd with the compressor and decoder both, non-static, sovk_zstd.odefinesZSTD_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/zbicon 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.mdsays none of zbic is linked into the game; that stops being true.This duplicates the decoder objects that the
vk_zstd_*.ojobs above already link (ZSTD_decompress, ZSTD_isError, FSE/HUF...). Expect duplicate symbols at link; compile only the compressor sources from third_party/zstd instead.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!
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_SUPPORTdefaults 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:
vk_zstd.ois 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 explicitlyvisibility("default"), so I'd expect the link to fail with "recompile with -fPIC". I haven't built it. The removed job also passed-DNDEBUGand-fvisibility=hidden, and I'd keep those for consistency (DEBUGLEVEL 0 already turns off zstd's asserts, so leaving out NDEBUG changes nothing).disk_taken,program_cache_keys, the fallbacks inprogram_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 throughDiskRecord/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.-I third_party/zstd/libin imgui_flags and those headers in imgui_deps no longer do anything (zbic's header wins). The zstd.c job also hasb""deps, so a change to zbic's headers won't rebuild it.On Linux this object goes into the .so but gets no
-fPIC. The removedzflagshad(["-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). Suggesttarget() + CPU + (["-fPIC"] if LINUX else []) + ["-O2", "-g0", "-DNDEBUG", "-Wno-everything", "-fvisibility=hidden", ...].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.
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.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:
-fPICon Linux forvk_zstd.o(build.py:994, replied in the thread). The appimage job should show whether it links.os.path.isfile(zstd_src)guard (if the file is missing, the fonts fail later with undefined ZSTD_* symbols);-I third_party/zstd/libin imgui_flags and those headers in imgui_deps don't do anything now; the zstd.c job'sb""deps; and third_party/zbic/PROVENANCE.md still says none of zbic is linked in.New:
seed_computereads 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 inseed_compute(vk_shader.cpp around line 2681) readsdisk_records[key]straight fromdisk_inand parsesrd.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 withdisk_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/zbicand 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.The
seed_computepoint from the last review is still open, and I confirmed it on the merge with main.seed_compute(vk_shader.cpp:2110) reads eachdisk_recordsentry straight fromdisk_inand parsesrd.get<uint32_t>(); rd.list(q.answers). A record written byprogram_cache_writewithkCompressedKindstarts with the 8-byte unpacked size and then the zstd frame. So withRX_VK_PROGRAM_CACHE_ZSTD=1, the compute variants already kept are never recognised: each prepare translates them again and appends them, andhookshot_programs.zst.bingrows every time. Please move the unpack into one helper (record bytes in, payload out), use it inprogram_cache_read,program_cache_eachand here, and add a test that writes and reads back one compressed record.The appimage job links, so the missing
-fPIConvk_zstd.o(build.py:993) isn't fatal, but I'd still match the old job's flags. Theos.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.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_computestill reads the records raw (vk_shader.cpp:2110). It readsdisk_records[key]straight fromdisk_inand parsesrd.get<uint32_t>(); rd.list(q.answers). A compressed record starts with the 8-byte unpacked size and then the frame, so withRX_VK_PROGRAM_CACHE_ZSTD=1the kept compute variants aren't recognised, every prepare translates them again, and the .zst.bin grows each time. The unpack is still copied inprogram_cache_readandprogram_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 coversdisk_zstd = false.Smaller, still there:
vk_zstd.ohas 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.os.path.isfile(zstd_src)guard: if the file is missing, the fonts fail later with undefined ZSTD_* symbols.third_party/zbic/PROVENANCE.mdstill says none of zbic is linked into the game.-I third_party/zstd/libin imgui_flags and those headers in imgui_deps no longer do anything, and the zstd.c job hasb""deps.#2309 (still open) picks the other zstd library, so whichever lands second has to switch; that ordering stays with @NotRexed.
Thanks — the open points are addressed in the new head (
7f33ccf1, rebased ona5b955fb):seed_compute: the unpack now lives in oneprogram_cache_payload(stored, stored_size, kind, ...)helper, andprogram_cache_read,program_cache_eachandseed_computeall 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-fPICon Linux,-DNDEBUGand-fvisibility=hidden; the job is unconditional, and the zbic headers are its dependencies.-I third_party/zstd/liband zstd headers are gone; the zbic headers are what it includes and what its deps name.zstd.cwithZSTD_ZBIC_SUPPORToff (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 WindowsRX_VK_ONLYbuild 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/zbicand use its compressor objects; the on-disk record format does not depend on which library compresses it.Test report — zstd program cache
Machine
Build and settings
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.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.HOS_HEADLESS=1,HOS_AUDIO=0), scripted input, 240 s per scene, same save and caches. FPS is the renderer'spresentsrate; 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.pypasses with the fixture'sthird_party/zbicstubs (fixed ind9ec233e; the Ubuntu part 1 failure was that fixture, not the cache).RX_VK_ONLYDLL build: pass; the single zbiczstd.clinks with-fPIC/-DNDEBUG/-fvisibility=hiddenand no duplicate symbols. The full game build uses it too.seed_computenow usesprogram_cache_payload, so compressed compute records are recognised instead of translated and appended again.Cache and prepare
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)
memory syncis 0.8–1.7 ms in these scenes withRX_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.
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):hookshot_programs.zst.bin+ 1 MB index;One note for anyone testing with the force-kill harness: an earlier
VK_ERROR_DEVICE_LOSTin my runs was stalemkw_vk_pipelines_play_*.binfiles from older builds, not the program cache; removing them made the stack stable.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):hookshot_programs.zst.bin+ 1 MB index;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 withRX_VK_IMAGE_PREALLOC=0), but it is worth knowing when a card is tight.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.