perf(vk): what play makes goes into a pipeline cache of its own, one file per save (no 0.5-1.2 GB rewrite in play) #1772
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1772
Loading…
Reference in a new issue
No description provided.
Delete branch "perf/pipeline-cache-play"
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?
Builds on #1660 (its commit is the first one here until it is merged).
The problem
In play, every pipeline cache save writes
mkw_vk_pipelines.binwhole again (0.5-1.2 GB) for what play added since the last one.pipeline_cache_saveruns every 300 presents once 32 pipelines were made. In a 4-minute walk from a first start's state that meant 5-6 saves, each writing 497-539 MB to add 0.6-27 MB:hookshot.exe's pages, the program cache) wait as long.The fix
Once the start-up is done (
vk_main'sstart, afterstarted_up), what play makes goes into a pipeline cache of its own, one generation per save.Making a pipeline in play (
create_cached) asks without compiling (VK_PIPELINE_CREATE_FAIL_ON_PIPELINE_COMPILE_REQUIRED_BIT), in this order:The start's cache is asked second on purpose: asked first, NVIDIA copies a pipeline it finds in its own cache into the cache asked, and the start's cache grows again. I tried that order first and the start's cache still grew in play.
A save (
play_cache_save, frompipeline_cache_save):mkw_vk_pipelines_play_<n>.bin;The window's close (
mkwvk_close_asked) writes the current generation too. That takes a few ms. The start's cache is still not written there, as before. Without this, the pipelines made since the last save were lost at every close, and the next start's precompile built them into the start's cache, which then had to be written whole again.The next start (
pcache_init):vkMergePipelineCaches;mkw_vk_pipelines.binthere, once, and removes them.Unchanged:
prepare_run, also when it runs in the background after an update) is bulk work and still goes into the start's cache.Switches:
RX_VK_PLAY_CACHE=0, or a device withoutpipelineCreationCacheControl, puts everything into the one cache, as before.Measured
Test setup:
--prepare-shaders(computes only, no graphics notes). So every graphics pipeline of the route is new, as on a first visit.RX_VK_PLAY_CACHE=0.Two runs of each, in the order P, Q, P, Q:
RX_VK_PLAY_CACHE=0)The play files took 1-69 ms each to write; the whole-cache writes took 1.4-3.2 s each.
The machine runs close to its commit limit in these runs, so page-file reads on the other disk add noise to the frame times; the disk numbers are the direct measure.
A start after such a session merged the play files and wrote nothing whole. After a windowed run closed through its window, the play file written at the close was merged at the next start, and that start's precompile added nothing to the cache. Before the close save, a start after a session wrote the whole cache once (540 MB), because the precompile rebuilt the pipelines lost at the close.
Tests
tests/test_pipeline_play_cache.pyextracts the play caches fromgpu/vk/vk_draw.cppunmodified (struct PlayCacheto the end ofcreate_cached). It runs them on a model of Vulkan pipeline caches with a driver disk cache that behaves as NVIDIA's (it copies what it serves into the cache asked), with real files and threads. 9 cases:RX_VK_PLAY_CACHE=0, and a device withoutpipelineCreationCacheControl, keep the one cache.Each of these deliberate mistakes fails a case:
Also changed:
tests/pipeline_cache_save_test.cppgets aplay_cache_savestub and checks it runs at every save: when the cache part writes, when it has nothing new, and when it fails.tests/test_mesh_fallback.pyandtests/test_mesh_fallback_failed_once.pylook for the compile ascached_compute(.Checked:
RX_VK_ONLY=1 nx2/build.py --standalone) without warnings.tests/run_all.py: 230 passed, 37 skipped. Three fail for reasons of this PC, as onmain:nx2/test_nup_read_name.pyneeds thezstandardmodule;tests/test_precompile_compute_guard.pyneedswindows.hfor an MSVC target;nx2/test_nup_paths.pyneeds the symlink privilege.Only NVIDIA was measured.
🤖 Generated with Claude Code
Nice find, and the generation-per-save design reads well: the ask order (generation, then start's cache, then compile), the users count so a save waits for in-flight builds, keeping an unwritten generation for the next save, and the close-time write all look right to me. I found no logic problem in the renderer code itself. But CI is red on three tests that this PR breaks, so it needs a fix before merge:
tests/test_pipeline_play_cache.pyfails on windows-2022: the test includes<windows.h>withoutNOMINMAX, sostd::max(play_seq, ...)in the extracted code hits themaxmacro (pipeline_play_cache.inc:41: expected unqualified-id). DefiningNOMINMAXbefore the include intests/pipeline_play_cache_test.cppshould do it.tests/test_linux_driver_fault_guard.pyfails on ubuntu (source.index(END)ValueError). It slicesvk_draw.cppfrom the fault-guard#elseup to\n#endif\nVkPipeline build_graphics, andcached_graphics/cached_computenow sit between that#endifandbuild_graphics. Moving the two wrappers belowbuild_graphics' forward declaration block (or afterdriver_cache_only), or updating END in the test, fixes it.tests/test_precompile_compute_guard.pyfails on windows-2022: its fixture compiles theprecompile_runarm, which now callscached_compute, and the fixture only hasguarded_compute(use of undeclared identifier 'cached_compute'). The fixture needscached_compute, or a stand-in that forwards toguarded_compute(pcache, ...). (This one fails on your PC for a different reason, but on CI it is caused by this change.)nx2/test_nup_stand_in.pyalso fails on both OS; that is not from this PR, so a merge of main should clear it.Overlap: #1660 is the first commit here, so this should land after it. Both touch pipeline cache writing.
Thanks for the review. Fixed in
4038ec6, and main merged (f9b9e15, with #1764's fix oftest_nup_stand_in):tests/pipeline_play_cache_test.cppdefinesNOMINMAXbeforewindows.h. I reproduced the windows-2022 error by building the old fixture against awindows.hthat definesmin/maxas the SDK's does (same error atpipeline_play_cache.inc:41); the new one builds.cached_graphics/cached_computenow sit belowbuild_graphics' declaration anddriver_cache_only; the test's START..END slice holds only the Linux guard again.test_precompile_compute_guard.py's fixture forwardscached_computetoguarded_compute(pcache, ...), which is whatcreate_cacheddoes before play (the precompile runs beforeplay_cache_begin).This PC has no MSVC STL, so CI is the check for 1 and 3. Agreed on landing after #1660.
main moved after my last push (#1586, #1574), so I merged it again (
efc73a3). Two things in it touch this PR:cached_graphics(the conflict inbuild_graphics): an ask under the flag still goes to the start's cache, where the warm pass put them; one compiled in play goes into the play generation.test_gpl_library_cache.pychecks that line in its new form.pcachein play, which would grow it again.46efc77routes them throughcached_compute(shared_compute_pipelinereplacesshared_pipeline_cache), which also gives them the driver fault guard.Merged main again (#1660, #1675 and #1765 landed).
8e54706: main's AMD retry inbuild_graphicsmakes both attempts throughgraphics_with(pcache through the play caches).6593de7: #1765 askspcacheunderFAIL_ON_PIPELINE_COMPILE_REQUIREDat the dispatch, in play; NVIDIA answers such an ask from its own disk cache and copies the pipeline into the cache asked, sopcachewould grow again.create_cachednow takes an ask under the flag through the generation thenpcache, without the compile, and the dispatch asks throughcached_compute; the route case checks it.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!
Merged main (
28ded6f). The only conflict wastests/run_all.py: main moved its tables into the tests, sorun_all.pyis main's andtest_pipeline_play_cache.pydeclaresRUN_ALL_ARGS. The renderer builds without warnings and the tests this PR touches pass.On the last CI run, every failure left was main's own:
test_draw_stream_visibilityandtest_sampler_admission: fixed in main by2f4c072.test_draw_parameters: reads onlyemit_spirv.cpp, which this PR does not touch.test_lift_cleanup_error,test_recomplauncher_jsonandtest_nup_out_of_space: they fail on main's own CI too.Thanks for the quick turnaround. All three test breaks from my last review are fixed and green on CI at
28ded6f:test_pipeline_play_cache.pypasses on windows-2022 (NOMINMAX),test_linux_driver_fault_guard.pypasses on ubuntu (the wrappers now sit below the slice), andtest_precompile_compute_guard.pypasses with thecached_computestand-in. #1660 has landed in main, so the ordering question is settled, and the PR's diff now carries only the play-cache part on top of it. The test declaresRUN_ALL_ARGSto match #1822.I also went through what came in with the merges. Stage libraries and the AMD retry both go through
graphics_with, FSR 3.1 goes throughshared_compute_pipeline, and the dispatch's under-flag ask now usescached_computewith the ask-only path, so it stops at step 2. That all looks right. The users count and re-check increate_cachedagainst the swap-then-wait inplay_cache_saveis sound. A thread that sees the old generation after the swap backs out before it touches the cache, so the destroy is safe. The only directpcachecreates left are in the warm pass, which runs before play, so that's fine.CI: the two remaining failures are main's own (
nx2/test_draw_parameters.pyon ubuntu,nx2/test_nup_out_of_space.pyon windows). They're not from this PR. Merging main again later should clear them once main is fixed.One nit inline. Approving.
Nit: this runs on the window thread (
display.c→rx_close_asked), andplay_cache_savespins until the generation'susersdrops to 0. If a long compile is in flight when the window is closed (the comments elsewhere mention multi-second culling compiles on AMD), the close blocks until it finishes. It also waits behind a background save that holdsmu. It's probably fine for a close, but if it ever shows up as a hang on exit, it could skip the swap whenusersis nonzero and let the exit-time forced save handle it.