perf(vk): ask the start's cache first for stage libraries in play #1961

github-sync · 2026-11-10T21:58:26Z

jarynclouatre commented

2026-11-10 21:58:26 +00:00

(Migrated from github.com)

Changes

A follow-up to #1772. With it, a stage library asked without compiling goes to the play generation first and to pcache second. The generation starts empty, so on NVIDIA every library ask in play was answered from the driver's own disk cache and copied in, though #1586's warm pass had already put the library in pcache. That is the slow path: on the route below, stage library links in play took 0.6 s before #1772 and 7.3 s after, about 2.2 ms a pipeline.

The comment on #1772 said the libraries would still go to the start's cache, where the warm pass put them. This makes the code do that: in create_cached, a library asked without compiling now goes to pcache first, then the generation, and is never compiled there. A dispatch's compute ask keeps #1772's order, so pcache does not grow from those. RX_VK_PLAY_ASK_START_FIRST=0 puts the libraries back on the old order.

The second part is the save. pcache still grows a little in play, and pipeline_cache_save writes it whole whenever its size differs from the last write. On the route below that was 25 writes of the 2.4 GB file for 9 MB of growth. In play it is now written once it has grown 64 MB. The window's close does not write it (it saves the play generation only), so growth under 64 MB is written only by a forced save, Restart now or the prepare; the next start can still get those libraries from the driver's own cache. A run prints how much it grew, up to four times. RX_VK_PLAY_SAVE_START_MB=<n> sets the threshold, and 0 writes on any change as before.

The play cache and cache save tests are updated for the new order and the threshold.

Testing

RTX 4070 Ti SUPER, driver 621.14, Windows 11, Ryzen 7 9800X3D.

Build: the Windows DLL builds on fc375e9 with this change. test_pipeline_cache_save, test_pipeline_play_cache and test_gpl_library_cache pass on Linux.

Route: the scripted 10-minute first visit through five areas, from the same prepared folder each time (stage libraries warmed by the prepare), one run each. The runs were on 51de681 with #1858's lines. Nothing in create_cached or pipeline_cache_save has changed on main since.

	`51de681`	`RX_VK_PLAY_CACHE=0`	this change
time lost against 60 fps, s	16.2	9.9	9.7
stage library links in play, s	7.3	1.4	1.3
frames over 40 ms	82	53	63
pcache written in play	25 times	13 times	0
pcache growth in play	9 MB	131 MB	under 1 MB

The play files' own counts show the libraries landing in pcache: 4105 pipelines "from the start's cache" against 543 on 51de681, and 1120 "from the driver's cache" against 4711. The "compiled" count is the same in both (2222, 2226).

Not tested: AMD and Intel, and a folder prepared before the warm pass.

## Changes A follow-up to #1772. With it, a stage library asked without compiling goes to the play generation first and to pcache second. The generation starts empty, so on NVIDIA every library ask in play was answered from the driver's own disk cache and copied in, though #1586's warm pass had already put the library in pcache. That is the slow path: on the route below, stage library links in play took 0.6 s before #1772 and 7.3 s after, about 2.2 ms a pipeline. The comment on #1772 said the libraries would still go to the start's cache, where the warm pass put them. This makes the code do that: in `create_cached`, a library asked without compiling now goes to pcache first, then the generation, and is never compiled there. A dispatch's compute ask keeps #1772's order, so pcache does not grow from those. `RX_VK_PLAY_ASK_START_FIRST=0` puts the libraries back on the old order. The second part is the save. pcache still grows a little in play, and `pipeline_cache_save` writes it whole whenever its size differs from the last write. On the route below that was 25 writes of the 2.4 GB file for 9 MB of growth. In play it is now written once it has grown 64 MB. The window's close does not write it (it saves the play generation only), so growth under 64 MB is written only by a forced save, Restart now or the prepare; the next start can still get those libraries from the driver's own cache. A run prints how much it grew, up to four times. `RX_VK_PLAY_SAVE_START_MB=<n>` sets the threshold, and 0 writes on any change as before. The play cache and cache save tests are updated for the new order and the threshold. ## Testing RTX 4070 Ti SUPER, driver 621.14, Windows 11, Ryzen 7 9800X3D. Build: the Windows DLL builds on fc375e9 with this change. `test_pipeline_cache_save`, `test_pipeline_play_cache` and `test_gpl_library_cache` pass on Linux. Route: the scripted 10-minute first visit through five areas, from the same prepared folder each time (stage libraries warmed by the prepare), one run each. The runs were on 51de681 with #1858's lines. Nothing in `create_cached` or `pipeline_cache_save` has changed on main since. | | 51de681 | `RX_VK_PLAY_CACHE=0` | this change | |---|---|---|---| | time lost against 60 fps, s | 16.2 | 9.9 | 9.7 | | stage library links in play, s | 7.3 | 1.4 | 1.3 | | frames over 40 ms | 82 | 53 | 63 | | pcache written in play | 25 times | 13 times | 0 | | pcache growth in play | 9 MB | 131 MB | under 1 MB | The play files' own counts show the libraries landing in pcache: 4105 pipelines "from the start's cache" against 543 on 51de681, and 1120 "from the driver's cache" against 4711. The "compiled" count is the same in both (2222, 2226). Not tested: AMD and Intel, and a folder prepared before the warm pass.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 02:18:37 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, the numbers make a good case, and the library ask order looks right to me. A cold library ask now goes to pcache, then the generation, and is never compiled. A dispatch's compute ask and a worker's compiling ask keep #1772's order. The counters match the play file log, and the new checks in pipeline_play_cache_test.cpp cover each branch (pcache hit, a driver copy into pcache, generation-only, nowhere).

One thing doesn't match the code. The description, the two new comments and the new log line all say pcache "is written at the close if it changed at all". The window's close doesn't do that. mkwvk_close_asked calls pipeline_cache_close(), and that only runs play_cache_save() (the generation). It never calls pipeline_cache_save(true). vk_menu.cpp says so too: "as the window's own close does not force the pipeline cache". Only Restart now, the menu's restart path and the prepare force it. So in a normal session, pcache's in-play growth under 64 MB is never written. The in-play test passes because it calls pipeline_cache_save(true) itself, as if that were the close.

The cost is probably small. What pcache picks up in play is mostly libraries NVIDIA's own disk cache answered, and the next start can still get those from there. A folder prepared before the warm pass should get re-warmed through the warmed stamp anyway. Still, it's a change from 51de681: the old order put those driver copies into the generation, and from there into a play file. Either way works for me:

make the close write pcache when it grew in play. That's a 2.4 GB write at the close, which may be exactly what the close was avoiding, so it's your call.
or keep the behaviour and fix the wording: the comment at 873, the one at 8230, the log line, the test's "at the close" comment, and the description's "at the close if it changed at all" / "Not tested: ... the close-time save".

CI: Linux, the Windows back-end build and actionlint pass. Windows Python fails test_log_batch (a timing bound) and test_recorder_sync, neither of which touches this code. Main's latest run has other unrelated failures. No conflicts with main in create_cached or pipeline_cache_save, even though the branch is 54 commits behind.

Nits: a negative RX_VK_PLAY_SAVE_START_MB wraps to a huge threshold (never written in play). That's harmless, but std::max(0LL, ...) would make it obvious. Also, AGENTS.md asks the description to open with why the PR is needed. Yours gets there in its second and third sentences, so it's close enough.

Thanks, the numbers make a good case, and the library ask order looks right to me. A cold library ask now goes to pcache, then the generation, and is never compiled. A dispatch's compute ask and a worker's compiling ask keep #1772's order. The counters match the play file log, and the new checks in `pipeline_play_cache_test.cpp` cover each branch (pcache hit, a driver copy into pcache, generation-only, nowhere). One thing doesn't match the code. The description, the two new comments and the new log line all say pcache "is written at the close if it changed at all". The window's close doesn't do that. `mkwvk_close_asked` calls `pipeline_cache_close()`, and that only runs `play_cache_save()` (the generation). It never calls `pipeline_cache_save(true)`. vk_menu.cpp says so too: "as the window's own close does not force the pipeline cache". Only Restart now, the menu's restart path and the prepare force it. So in a normal session, pcache's in-play growth under 64 MB is never written. The `in-play` test passes because it calls `pipeline_cache_save(true)` itself, as if that were the close. The cost is probably small. What pcache picks up in play is mostly libraries NVIDIA's own disk cache answered, and the next start can still get those from there. A folder prepared before the warm pass should get re-warmed through the warmed stamp anyway. Still, it's a change from 51de681: the old order put those driver copies into the generation, and from there into a play file. Either way works for me: - make the close write pcache when it grew in play. That's a 2.4 GB write at the close, which may be exactly what the close was avoiding, so it's your call. - or keep the behaviour and fix the wording: the comment at 873, the one at 8230, the log line, the test's "at the close" comment, and the description's "at the close if it changed at all" / "Not tested: ... the close-time save". CI: Linux, the Windows back-end build and actionlint pass. Windows Python fails `test_log_batch` (a timing bound) and `test_recorder_sync`, neither of which touches this code. Main's latest run has other unrelated failures. No conflicts with main in `create_cached` or `pipeline_cache_save`, even though the branch is 54 commits behind. Nits: a negative `RX_VK_PLAY_SAVE_START_MB` wraps to a huge threshold (never written in play). That's harmless, but `std::max(0LL, ...)` would make it obvious. Also, AGENTS.md asks the description to open with why the PR is needed. Yours gets there in its second and third sentences, so it's close enough.

gpu/vk/vk_draw.cpp

					
				@ -8197,12 +8219,26 @@ void pipeline_cache_save(bool force) {

				        while (busy.load()) Sleep(1);   // (the one under way, then this one)

The window's close doesn't reach this. mkwvk_close_asked → pipeline_cache_close() runs only play_cache_save(), so nothing forces this save at the close. Only Restart now, the menu's restart and the prepare call pipeline_cache_save(true). Either add the write to pipeline_cache_close() (if a full pcache write at the close is acceptable) or drop the "at the close" part here, at line 873 and in the log line below.

The window's close doesn't reach this. `mkwvk_close_asked` → `pipeline_cache_close()` runs only `play_cache_save()`, so nothing forces this save at the close. Only Restart now, the menu's restart and the prepare call `pipeline_cache_save(true)`. Either add the write to `pipeline_cache_close()` (if a full pcache write at the close is acceptable) or drop the "at the close" part here, at line 873 and in the log line below.

Same here: "written at the close" isn't true for a normal window close (see above), so this line will mislead whoever reads the log.

jarynclouatre commented

2026-11-11 02:32:50 +00:00

(Migrated from github.com)

Thanks. I kept the behaviour and fixed the wording, since a 2.4 GB write at every close is what the threshold was meant to avoid. d67f855 corrects the comments at 873 and 8230, the log line ("written once it is N MB larger, or by a forced save"), and the comments in both tests, and the description now says the close does not write pcache. I also took the nit: std::max(0LL, ...) on RX_VK_PLAY_SAVE_START_MB. test_pipeline_cache_save, test_pipeline_play_cache and test_gpl_library_cache pass, and the Windows DLL builds.

Thanks. I kept the behaviour and fixed the wording, since a 2.4 GB write at every close is what the threshold was meant to avoid. d67f855 corrects the comments at 873 and 8230, the log line ("written once it is N MB larger, or by a forced save"), and the comments in both tests, and the description now says the close does not write pcache. I also took the nit: `std::max(0LL, ...)` on `RX_VK_PLAY_SAVE_START_MB`. `test_pipeline_cache_save`, `test_pipeline_play_cache` and `test_gpl_library_cache` pass, and the Windows DLL builds.

mooki000 commented

2026-11-11 13:05:09 +00:00

(Migrated from github.com)

Linux run with NVIDIA's driver: no regression, but on this route the play generation is never asked. RTX 4090, NVIDIA 615.78.08, i9-13900K, Arch Linux. I merged d67f8558 onto main bdfe09c (it merges cleanly). Headless, RX_VK_GPL=1 in all runs.

Both builds started from a fresh folder: the pipeline caches and the driver's shader cache deleted, then --prepare-shaders with RX_VK_GPL=1 (the warm pass logged warmed 86974 stage libraries). Then a scripted first visit: Kokiri Forest from a save, then the debug teleports to Hyrule Field, Kakariko, the Market, the Lost Woods and Death Mountain Trail, about 22 s each with a camera turn. Load average 1-6.

	pipeline waits	120-frame log lines with any library link	pcache written in play	frames over 50 / 100 ms
main `bdfe09c` (three runs)	0, 0, 0	0, 2, 0 of 77-78	0, 0, 1 (at the title, before play)	24 / 9, 31 / 11, 21 / 9
main + this PR	0	2 of 77	0	33 / 10

After the warm pass this route needs no library link worth counting, so the new ask order and the 64 MB threshold have nothing to act on here. The frame counts are within what two runs of main gave each other.

Not tested: Windows (where your numbers come from), AMD, Intel, a folder prepared before the warm pass, or a longer route.

**Linux run with NVIDIA's driver: no regression, but on this route the play generation is never asked.** RTX 4090, NVIDIA 615.78.08, i9-13900K, Arch Linux. I merged d67f8558 onto main bdfe09c (it merges cleanly). Headless, `RX_VK_GPL=1` in all runs. Both builds started from a fresh folder: the pipeline caches and the driver's shader cache deleted, then `--prepare-shaders` with `RX_VK_GPL=1` (the warm pass logged `warmed 86974 stage libraries`). Then a scripted first visit: Kokiri Forest from a save, then the debug teleports to Hyrule Field, Kakariko, the Market, the Lost Woods and Death Mountain Trail, about 22 s each with a camera turn. Load average 1-6. | | pipeline waits | 120-frame log lines with any library link | pcache written in play | frames over 50 / 100 ms | |---|---|---|---|---| | main bdfe09c (three runs) | 0, 0, 0 | 0, 2, 0 of 77-78 | 0, 0, 1 (at the title, before play) | 24 / 9, 31 / 11, 21 / 9 | | main + this PR | 0 | 2 of 77 | 0 | 33 / 10 | After the warm pass this route needs no library link worth counting, so the new ask order and the 64 MB threshold have nothing to act on here. The frame counts are within what two runs of main gave each other. Not tested: Windows (where your numbers come from), AMD, Intel, a folder prepared before the warm pass, or a longer route.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 16:50:05 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for d67f855. The wording is fixed everywhere I pointed at: the comments at 873 and 8230, the log line ("written once it is N MB larger, or by a forced save"), both tests' comments and the description now say the close doesn't write pcache. Keeping the behaviour is fine with me, for the reason you gave. Both earlier inline threads are resolved.

One new problem, from the nit fix. std::max(0LL, ...) breaks test_pipeline_cache_save on Windows: the test builds with windows.h in scope, and its max macro eats std::max(. CI (Windows part 2/2): pipeline_cache_save.inc:22:97: error: expected unqualified-id ... expanded from macro 'max'. That test passed on 0e25963. The DLL build itself is fine (the Windows back-end job passes), but the check fails. std::max<long long>(0, ...) or (std::max)(0LL, ...) avoids the macro, the same way the std::min<size_t> in the log line below already does. Sorry, that nit cost more than it saved.

mooki000's Linux run (RTX 4090, after the warm pass) shows no regression, though the route never reaches a library link, so it doesn't test the new order either. Your Windows numbers are still the evidence for the gain.

Other CI: Linux Python fails test_finish_output and test_linux_update (a NameError in the tests themselves), Windows part 1 fails those plus test_recorder_sync, and actionlint flags a shellcheck info in linux-appimage.yml. None of those touch this PR's files. No conflicts with main.

With the max fixed, I'd approve.

Thanks for d67f855. The wording is fixed everywhere I pointed at: the comments at 873 and 8230, the log line ("written once it is N MB larger, or by a forced save"), both tests' comments and the description now say the close doesn't write pcache. Keeping the behaviour is fine with me, for the reason you gave. Both earlier inline threads are resolved. One new problem, from the nit fix. `std::max(0LL, ...)` breaks `test_pipeline_cache_save` on Windows: the test builds with `windows.h` in scope, and its `max` macro eats `std::max(`. CI (Windows part 2/2): `pipeline_cache_save.inc:22:97: error: expected unqualified-id ... expanded from macro 'max'`. That test passed on 0e25963. The DLL build itself is fine (the Windows back-end job passes), but the check fails. `std::max<long long>(0, ...)` or `(std::max)(0LL, ...)` avoids the macro, the same way the `std::min<size_t>` in the log line below already does. Sorry, that nit cost more than it saved. mooki000's Linux run (RTX 4090, after the warm pass) shows no regression, though the route never reaches a library link, so it doesn't test the new order either. Your Windows numbers are still the evidence for the gain. Other CI: Linux Python fails `test_finish_output` and `test_linux_update` (a `NameError` in the tests themselves), Windows part 1 fails those plus `test_recorder_sync`, and actionlint flags a shellcheck info in `linux-appimage.yml`. None of those touch this PR's files. No conflicts with main. With the `max` fixed, I'd approve.

gpu/vk/vk_draw.cpp

					
				@ -8197,12 +8219,26 @@ void pipeline_cache_save(bool force) {

				        while (busy.load()) Sleep(1);   // (the one under way, then this one)

On Windows the test build has windows.h's max macro in scope, so this fails to compile in test_pipeline_cache_save (CI Windows part 2/2). std::max<long long>(0, atoll(...)) or (std::max)(0LL, ...) avoids the macro.

On Windows the test build has `windows.h`'s `max` macro in scope, so this fails to compile in `test_pipeline_cache_save` (CI Windows part 2/2). `std::max<long long>(0, atoll(...))` or `(std::max)(0LL, ...)` avoids the macro.

jarynclouatre commented

2026-11-11 16:54:21 +00:00

(Migrated from github.com)

Fixed in 39d5d05 with std::max<long long>(0, ...). I checked that form compiles with a function-like max macro defined after <algorithm>, the way windows.h defines it, and that the old std::max(0LL, ...) fails the same way as CI did. tests/test_pipeline_cache_save.py passes on Linux.

Fixed in 39d5d05 with `std::max<long long>(0, ...)`. I checked that form compiles with a function-like `max` macro defined after `<algorithm>`, the way `windows.h` defines it, and that the old `std::max(0LL, ...)` fails the same way as CI did. `tests/test_pipeline_cache_save.py` passes on Linux.

hckrman101 (Migrated from github.com) approved these changes

2026-11-11 17:34:46 +00:00

hckrman101 (Migrated from github.com) left a comment

The blocker is fixed: std::max<long long>(0, atoll(...)) in 39d5d05 no longer goes through windows.h's max macro, and the Windows Vulkan back end job passes. That commit is the only change since d67f855.

The red Python source checks fail in tests/test_vcore_video_acquire.py (env_on undeclared in its fixture), a file this PR doesn't touch, so I don't hold it against the PR. The appimage failure is a runner/apt problem in the system packages step. Both look unrelated, but worth a re-run before merging.

mooki000's NVIDIA/Linux report (play generation not asked on that route) is a separate finding about route coverage, not a regression from this change; it's for the author and maintainer to weigh. Nothing else from me.

The blocker is fixed: `std::max<long long>(0, atoll(...))` in 39d5d05 no longer goes through windows.h's `max` macro, and the Windows Vulkan back end job passes. That commit is the only change since d67f855. The red Python source checks fail in `tests/test_vcore_video_acquire.py` (`env_on` undeclared in its fixture), a file this PR doesn't touch, so I don't hold it against the PR. The appimage failure is a runner/apt problem in the system packages step. Both look unrelated, but worth a re-run before merging. mooki000's NVIDIA/Linux report (play generation not asked on that route) is a separate finding about route coverage, not a regression from this change; it's for the author and maintainer to weigh. Nothing else from me.

~~github-sync referenced this pull request 2026-11-12 06:22:54 +00:00~~

chore(main): release 2.2.0-rc.1 #2071

Commenting is not possible because the repository is archived.

No reviewers

hckrman101