Repository navigation
Conversation
Every translated program kept a VkShaderModule per stage for as long as the program lived. With the shipped state lists a start keeps ~12,000 programs (~20,400 modules), and NVIDIA's driver held ~99 KB of its own memory for each: ~2 GB of the game's private memory, in two ~1 GB driver arenas full of SPIR-V copies (driver 621.14, RTX 4070 Ti Super). A module destroyed later gave none of it back, so on a 32 GB PC the game sat at a 15.3 GB working set with 1.1 GB of the system's memory left, and paged. Each stage now keeps its SPIR-V zstd-packed (level 1: the game's SPIR-V packs 3.8 times smaller, 0.14 ms a module) in Stage::code, and a StageModule makes the module from it only while a pipeline is made, destroying it after (0.05 ms to unpack, 0.03 ms to make). Measured with the game's own 20,400 modules: kept alive they cost +1969 MB of private memory; made and destroyed one at a time, +8 MB. The packed SPIR-V of those is ~0.43 GB. - vk_shader.h: StageCode, stage_code(), StageModule; Stage::module is gone. - vk_stage_code.cpp (new, outside the translator stamp): the packing and StageModule. - vk_shader.cpp: the stages keep their code; the module destroys that went with programs, refused cache records and seed variants are gone (the code is the program's own). - vk_draw.cpp: build_graphics makes the modules of the stages each pipeline or library part takes; the compute paths (fallback, prepare warm, precompile, the dispatch's ask, its queued build and its own compile) make theirs for the one create call. A module that is not made leaves that pipeline unmade, as a failed compile does. - third_party/zstd: lib/compress/ from the same zstd 1.5.7 release, unmodified; built single-threaded (ZSTD_MULTITHREAD was defined as 0, which zstd's #ifdef took as on). - tests: the program cache record test checks the stages' code instead of module lifetimes; the precompile guard's fixture has a StageModule stand-in that must be alive during the create; the stamp and DLSS build fixtures know the new zstd files and zstd_users. vk_shader.cpp is part of the translator stamp, so the first start after this translates the program cache again (one prepare). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…o recorded command uses a shader module) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hckrman101
left a comment
There was a problem hiding this 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
StageModulelives in the scope of its create call. That coversbuild_graphics(themodulesvector lives to the end of the function, aftergraphics_with, which is synchronous), the mesh fallback, the warm, the precompile, the dispatch's ask and its queuedbuildlambda (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 theBOUNDARY_OKentry is right. - Threads.
Stage::codeis only read after a program is built. The pack context and the unpack buffer arethread_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_graphicsmatches thepart_stagesfilter 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/vkchainsVkShaderModuleCreateInfointo 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. Sohookshot_programs.binandmkw_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=0catch 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-valwere 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_OPTIMIZATIONretries) now create the pipeline twice from the same live module. That should be fine, but no one has run it.
One nit inline.
| 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()); |
There was a problem hiding this comment.
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.
…cache.cpp (NotRexed#2323), zstd's jobs to zstd_jobs (NotRexed#2270) - vk_progcache.cpp: the cache read keeps each stage's code (stage_code) and a refused record leaves nothing to destroy; program_cache_borrow lets go of the layouts alone. vk_progcache.h no longer declares make_module. - vk_shader.cpp: seed_variant_of's destroys of a failed or already made variant's modules (NotRexed#2358) are gone with the modules. - nx2/build.py: zstd_jobs(encoder=True) builds the encoder for the Vulkan back end, without ZSTD_MULTITHREAD; zstd_headers() is the users' dependency (vk_stage_code.cpp). The runtime's decoder-only zstd is unchanged. - tests: test_seed_variant_cleanup (NotRexed#2358) checks what the cache keeps instead of module lifetimes; test_recorder_sync's BOUNDARY_OK has no vk_shader.cpp entry left and vk_progcache.cpp's names its layouts; test_shader_stamp_cache knows zstd_headers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Joey-Magnus
left a comment
There was a problem hiding this 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:
hckrman101
left a comment
There was a problem hiding this 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_borrowmoved tovk_progcache.cpp(#2323). They now keep the code withstage_codeand destroy no modules,make_moduleis gone fromvk_progcache.h, and theBOUNDARY_OKreason forvk_progcache.cppis updated to match. zstd_jobstakesencoder=Falseby 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_DCtxnit 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.
|
Can you do re-measurement and see if the frame pacing is worst? |
…runtime's decoder job without ZSTD_MULTITHREAD - vk_stage_code.cpp: StageModule unpacks with ZSTD_decompressDCtx and a thread_local ZSTD_DCtx, as the pack side keeps its ZSTD_CCtx: ZSTD_decompress made and freed a whole DCtx (~100 KB) at every call, for every pipeline made in play. - nx2/build.py: the runtime's decoder-only zstd_jobs call no longer passes -DZSTD_MULTITHREAD=0, as zstd_jobs' docstring says its flags leave it out (harmless for the decoder alone; for consistency). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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:
The two arms:
Both are on the same older
¹ the mean change in frame time from one frame to the next. Reading it:
Also pushed
The renderer builds ( Still not done: a run on AMD or Intel. The Linux CI is queued. |
|
Tested on 1. Where
|
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 onmain, perf(vk): keep program stages' SPIR-V packed, make shader modules only per pipeline (~1.5 GB less RAM on NVIDIA) #2309 saves-2,389 MB(-2.33 GB) of Peak Working Set (recovering 95% ofmain's entire+2,506 MBWorking Set growth since49f75dc8) and-2,537 MB(-2.48 GB) of Peak Private Bytes (recovering 87% of the+2,916 MBprecompile Private Bytes growth), while keeping all 25,303 precompiled pipelines. - Startup precompile increases by only
+0.53 s(3.16 svs2.63 s), time to first frame is unchanged (4 s), first-load stutter is0frames over 50 ms, and frame rate/pacing in Kokiri Forest is at least as fast (68.6 fps,p99 = 18.68 ms).
Joey-Magnus
left a comment
There was a problem hiding this comment.
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).
The problem
Every translated program keeps a
VkShaderModulefor each of its stages for as long as the program lives.Measured on this PC in play, before this PR:
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:
The fix
Stages keep their SPIR-V packed. Each stage keeps its SPIR-V packed with zstd in
Stage::code, andStage::moduleis gone.Modules exist only while a pipeline is made. A
StageModuleunpacks the code and makes the module just for the pipeline being made, then destroys it.The new code,
stage_code()andStageModule, is ingpu/vk/vk_stage_code.cpp.Where the modules are made now:
build_graphicsmakes only the modules of the stages that the pipeline, or the library part, takes.FAIL_ON_PIPELINE_COMPILE_REQUIRED;Destroys removed. The module destroys are gone from:
program_cache_borrow;seed_variant_of;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 inPROVENANCE.md.-DZSTD_MULTITHREAD=0was removed fromnx2/build.py: zstd tests it with#ifdef, so defining it as 0 turned multithreading on.zstdmt_compress.cis vendored with the rest of the folder but not built.Every player translates the program cache once more.
gpu/vk/vk_shader.cppis part of the translator stamp, so the first start after updating translates all programs again.hookshot_programs.bincompared record by record, 98,200 of 98,200 modules the same. So nothing in the driver's caches goes stale.vk_shader.cppitself (removingmake_moduleand the destroys) could not be avoided.Only NVIDIA was measured (RTX 5070, driver 621.14, Windows 11).
-fPIClike the decoder's.Translation and loading do a little more work.
Precompile time:
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.bincontents and the same driver cache.The recorder.
vk_stage_code.cppdoes not includevk_work.h, so its module destroys are no recorder boundary: no recorded command uses a shader module. It is listed intest_recorder_sync'sBOUNDARY_OKwith that reason, likevk_shader.cpp's destroys before it. SoRX_VK_RECORDER=deferand=threadneither drain nor wait for them, also when the dispatch's ask makes and destroys a module on the replay thread.Overlap with perf(vk): compress the program cache with zstd (RX_VK_PROGRAM_CACHE_ZSTD) #1630 (compressed program cache, open).
nx2/build.py, andtests/program_cache_cleanup_test.cpp.third_party/zbic's single-file zstd into the game. That file'sPROVENANCE.mdsays 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.Measured
Test setup:
--prepare-shaders.mainfrom the same day, plus open PRs fix(vk): stop the VRAM compaction's hold from evicting textures in use #2198, fix(vk): a readback's wait for its write-back ends after 250 ms #2222 and fix(dlss): a render size a pixel past a mode's range is that mode's, and a refused size keeps the feature #2193, plus local logging (present times, memory map). The comparison build is the same thing without this commit.One run each:
About the frames over 20 ms:
A discarded run. A first run of this PR (not in the table) had 10 frames over 33 ms, the worst 102 ms.
Tests
tests/program_cache_cleanup_test.cppandtests/test_program_cache_cleanup.pynow check which program cache records are taken and that each taken stage keeps its code whole, through the production loader, with astage_codestand-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 aStageModulestand-in. The module must be alive during the create call and gone after it.tests/test_mesh_fallback_failed_once.pyfinds the failure block after the compile, which may now also be the module's.tests/test_shader_stamp_cache.pyandtests/test_dlss_sdk_cache.pyknow the zstd encoder sources andzstd_users.tests/test_recorder_sync.pylistsvk_stage_code.cppinBOUNDARY_OK(its module destroys), andvk_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:
RX_VK_ONLY=1 nx2/build.py --standalone), and the whole game builds withmake_game.py.tests/run_all.py: 311 passed, 60 skipped.test_precompile_compute_guardandtest_fpx_stickywere run with the MSVC headers andlld-linkand pass.main(49f75dc):nx2/test_nup_read_name.pyandtests/test_bake_portability.pyneed thezstandardmodule;nx2/test_nup_paths.pyneeds the symlink privilege.Also seen while measuring (not changed here)
With the shipped state lists,
mkw_vk_pipelines.binstayed 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: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