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

Merged
DUDUYG merged 10 commits from perf/pipeline-cache-play into main 2026-11-10 18:26:21 +00:00
DUDUYG commented 2026-11-10 16:20:02 +00:00 (Migrated from github.com)

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.bin whole again (0.5-1.2 GB) for what play added since the last one.

  • NVIDIA's driver copies into the cache a pipeline is made with each pipeline it takes from its own disk cache. So on a first visit the cache read at the start grows at every new pipeline, even when nothing is compiled.
  • pipeline_cache_save runs 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:
    • each write took 1.4-3.2 s;
    • each run wrote about 3 GB to keep 42 MB;
    • each save also fetched the whole cache into a new buffer, about 0.5 GB of extra working set while it lasted.
  • On a SATA SSD, a write that size leaves the disk slow for minutes, and the game's own reads from that disk (hookshot.exe's pages, the program cache) wait as long.
  • #1660 stops the rewrite at every start, when nothing changed. This is the case where something did change.

The fix

Once the start-up is done (vk_main's start, after started_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:

  1. The generation's cache. A pipeline the driver has in its own cache comes back, copied into the generation.
  2. The start's cache. A pipeline it holds comes from it unchanged.
  3. Otherwise it is compiled into the generation's cache.

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, from pipeline_cache_save):

  • puts a new generation in place and waits for any pipeline still being made in the old one;
  • writes the old generation alone to mkw_vk_pipelines_play_<n>.bin;
  • keeps a generation whose write did not land and tries it again at the next save.

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):

  • merges the play files into the cache as it reads them, with vkMergePipelineCaches;
  • removes a play file made by another driver;
  • at 16 play files, writes them into mkw_vk_pipelines.bin there, once, and removes them.

Unchanged:

  • The prepare's warm of the compute pipelines (prepare_run, also when it runs in the background after an update) is bulk work and still goes into the start's cache.
  • Before play, everything is as it was.

Switches: RX_VK_PLAY_CACHE=0, or a device without pipelineCreationCacheControl, puts everything into the one cache, as before.

Measured

Test setup:

  • RTX 5070, driver 621.01, Ryzen 7 9800X3D, 31 GB.
  • Game and its log on a SATA SSD; the test harness's files on another disk.
  • Scripted headless route: load a save, then walk outdoors on the Kokiri path. "In play" = from present 3300 on.
  • Before each run, the pipeline cache and the pipeline notes go back to their state right after --prepare-shaders (computes only, no graphics notes). So every graphics pipeline of the route is new, as on a first visit.
  • Same build in both arms, runs alternated. "Old" is RX_VK_PLAY_CACHE=0.

Two runs of each, in the order P, Q, P, Q:

old (RX_VK_PLAY_CACHE=0) this
whole-cache writes in a run 6 / 6 (3085 / 3079 MB) 0 / 0
play files written none 6 / 6 (55 MB each run)
game-disk 4 KB reads over 10 ms, in play 147 / 130 2 / 1
game-disk 4 KB reads over 50 ms 7 / 8 0 / 0
worst game-disk read 157 / 540 ms 13.5 / 12.6 ms
frames over 33.4 ms 33 / 22 9 / 12
frames over 50 ms 11 / 7 2 / 2
1% low 15.0 / 21.4 fps 27.6 / 28.1 fps
time lost against 60 fps, in play 2183 / 1592 ms 969 / 1097 ms
game's largest working set 13.64 / 13.66 GB 13.04 / 13.08 GB

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.py extracts the play caches from gpu/vk/vk_draw.cpp unmodified (struct PlayCache to the end of create_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:

  • route: in play the start's cache is left as it was read; one it holds comes from it; one the driver has goes into the generation; one nothing has is compiled into the generation; one already asked without compiling goes to the start's cache alone.
  • save: each save writes the generation alone; nothing new writes nothing.
  • retry: a write that did not land is tried again first.
  • users: a save waits for a pipeline being compiled in the generation it takes away, and a pipeline made meanwhile goes into the new one.
  • merge: the next start merges the play files; another driver's file is removed; other names are left alone; the numbering goes on from the highest.
  • compact: 16 play files are written into the one file and removed.
  • compact-failed: that write failing keeps the play files.
  • off and no-control: RX_VK_PLAY_CACHE=0, and a device without pipelineCreationCacheControl, keep the one cache.

Each of these deliberate mistakes fails a case:

  • asking the start's cache first;
  • not waiting for the users;
  • dropping a generation whose write failed;
  • never compacting.

Also changed:

  • tests/pipeline_cache_save_test.cpp gets a play_cache_save stub 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.py and tests/test_mesh_fallback_failed_once.py look for the compile as cached_compute(.

Checked:

  • The renderer builds with llvm-mingw (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 on main:
    • nx2/test_nup_read_name.py needs the zstandard module;
    • tests/test_precompile_compute_guard.py needs windows.h for an MSVC target;
    • nx2/test_nup_paths.py needs the symlink privilege.

Only NVIDIA was measured.

🤖 Generated with Claude Code

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.bin` whole again (0.5-1.2 GB) for what play added since the last one. - NVIDIA's driver copies into the cache a pipeline is made with each pipeline it takes from its own disk cache. So on a first visit the cache read at the start grows at every new pipeline, even when nothing is compiled. - `pipeline_cache_save` runs 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: - each write took 1.4-3.2 s; - each run wrote about 3 GB to keep 42 MB; - each save also fetched the whole cache into a new buffer, about 0.5 GB of extra working set while it lasted. - On a SATA SSD, a write that size leaves the disk slow for minutes, and the game's own reads from that disk (`hookshot.exe`'s pages, the program cache) wait as long. - #1660 stops the rewrite at every start, when nothing changed. This is the case where something did change. ## The fix **Once the start-up is done** (`vk_main`'s `start`, after `started_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: 1. The generation's cache. A pipeline the driver has in its own cache comes back, copied into the generation. 2. The start's cache. A pipeline it holds comes from it unchanged. 3. Otherwise it is compiled into the generation's cache. 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`, from `pipeline_cache_save`): - puts a new generation in place and waits for any pipeline still being made in the old one; - writes the old generation alone to `mkw_vk_pipelines_play_<n>.bin`; - keeps a generation whose write did not land and tries it again at the next save. **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`): - merges the play files into the cache as it reads them, with `vkMergePipelineCaches`; - removes a play file made by another driver; - at 16 play files, writes them into `mkw_vk_pipelines.bin` there, once, and removes them. **Unchanged:** - The prepare's warm of the compute pipelines (`prepare_run`, also when it runs in the background after an update) is bulk work and still goes into the start's cache. - Before play, everything is as it was. **Switches:** `RX_VK_PLAY_CACHE=0`, or a device without `pipelineCreationCacheControl`, puts everything into the one cache, as before. ## Measured Test setup: - RTX 5070, driver 621.01, Ryzen 7 9800X3D, 31 GB. - Game and its log on a SATA SSD; the test harness's files on another disk. - Scripted headless route: load a save, then walk outdoors on the Kokiri path. "In play" = from present 3300 on. - Before each run, the pipeline cache and the pipeline notes go back to their state right after `--prepare-shaders` (computes only, no graphics notes). So every graphics pipeline of the route is new, as on a first visit. - Same build in both arms, runs alternated. "Old" is `RX_VK_PLAY_CACHE=0`. Two runs of each, in the order P, Q, P, Q: | | old (`RX_VK_PLAY_CACHE=0`) | this | |---|---|---| | whole-cache writes in a run | 6 / 6 (3085 / 3079 MB) | 0 / 0 | | play files written | none | 6 / 6 (55 MB each run) | | game-disk 4 KB reads over 10 ms, in play | 147 / 130 | 2 / 1 | | game-disk 4 KB reads over 50 ms | 7 / 8 | 0 / 0 | | worst game-disk read | 157 / 540 ms | 13.5 / 12.6 ms | | frames over 33.4 ms | 33 / 22 | 9 / 12 | | frames over 50 ms | 11 / 7 | 2 / 2 | | 1% low | 15.0 / 21.4 fps | 27.6 / 28.1 fps | | time lost against 60 fps, in play | 2183 / 1592 ms | 969 / 1097 ms | | game's largest working set | 13.64 / 13.66 GB | 13.04 / 13.08 GB | 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.py` extracts the play caches from `gpu/vk/vk_draw.cpp` unmodified (`struct PlayCache` to the end of `create_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: - **route:** in play the start's cache is left as it was read; one it holds comes from it; one the driver has goes into the generation; one nothing has is compiled into the generation; one already asked without compiling goes to the start's cache alone. - **save:** each save writes the generation alone; nothing new writes nothing. - **retry:** a write that did not land is tried again first. - **users:** a save waits for a pipeline being compiled in the generation it takes away, and a pipeline made meanwhile goes into the new one. - **merge:** the next start merges the play files; another driver's file is removed; other names are left alone; the numbering goes on from the highest. - **compact:** 16 play files are written into the one file and removed. - **compact-failed:** that write failing keeps the play files. - **off** and **no-control:** `RX_VK_PLAY_CACHE=0`, and a device without `pipelineCreationCacheControl`, keep the one cache. Each of these deliberate mistakes fails a case: - asking the start's cache first; - not waiting for the users; - dropping a generation whose write failed; - never compacting. Also changed: - `tests/pipeline_cache_save_test.cpp` gets a `play_cache_save` stub 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.py` and `tests/test_mesh_fallback_failed_once.py` look for the compile as `cached_compute(`. Checked: - The renderer builds with llvm-mingw (`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 on `main`: - `nx2/test_nup_read_name.py` needs the `zstandard` module; - `tests/test_precompile_compute_guard.py` needs `windows.h` for an MSVC target; - `nx2/test_nup_paths.py` needs the symlink privilege. Only NVIDIA was measured. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) reviewed 2026-11-10 16:42:18 +00:00
hckrman101 (Migrated from github.com) left a comment

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:

  1. tests/test_pipeline_play_cache.py fails on windows-2022: the test includes <windows.h> without NOMINMAX, so std::max(play_seq, ...) in the extracted code hits the max macro (pipeline_play_cache.inc:41: expected unqualified-id). Defining NOMINMAX before the include in tests/pipeline_play_cache_test.cpp should do it.
  2. tests/test_linux_driver_fault_guard.py fails on ubuntu (source.index(END) ValueError). It slices vk_draw.cpp from the fault-guard #else up to \n#endif\nVkPipeline build_graphics, and cached_graphics/cached_compute now sit between that #endif and build_graphics. Moving the two wrappers below build_graphics' forward declaration block (or after driver_cache_only), or updating END in the test, fixes it.
  3. tests/test_precompile_compute_guard.py fails on windows-2022: its fixture compiles the precompile_run arm, which now calls cached_compute, and the fixture only has guarded_compute (use of undeclared identifier 'cached_compute'). The fixture needs cached_compute, or a stand-in that forwards to guarded_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.py also 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.

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: 1. `tests/test_pipeline_play_cache.py` fails on windows-2022: the test includes `<windows.h>` without `NOMINMAX`, so `std::max(play_seq, ...)` in the extracted code hits the `max` macro (`pipeline_play_cache.inc:41`: expected unqualified-id). Defining `NOMINMAX` before the include in `tests/pipeline_play_cache_test.cpp` should do it. 2. `tests/test_linux_driver_fault_guard.py` fails on ubuntu (`source.index(END)` ValueError). It slices `vk_draw.cpp` from the fault-guard `#else` up to `\n#endif\nVkPipeline build_graphics`, and `cached_graphics`/`cached_compute` now sit between that `#endif` and `build_graphics`. Moving the two wrappers below `build_graphics`' forward declaration block (or after `driver_cache_only`), or updating END in the test, fixes it. 3. `tests/test_precompile_compute_guard.py` fails on windows-2022: its fixture compiles the `precompile_run` arm, which now calls `cached_compute`, and the fixture only has `guarded_compute` (`use of undeclared identifier 'cached_compute'`). The fixture needs `cached_compute`, or a stand-in that forwards to `guarded_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.py` also 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.
DUDUYG commented 2026-11-10 16:57:09 +00:00 (Migrated from github.com)

Thanks for the review. Fixed in 4038ec6, and main merged (f9b9e15, with #1764's fix of test_nup_stand_in):

  1. tests/pipeline_play_cache_test.cpp defines NOMINMAX before windows.h. I reproduced the windows-2022 error by building the old fixture against a windows.h that defines min/max as the SDK's does (same error at pipeline_play_cache.inc:41); the new one builds.
  2. cached_graphics/cached_compute now sit below build_graphics' declaration and driver_cache_only; the test's START..END slice holds only the Linux guard again.
  3. test_precompile_compute_guard.py's fixture forwards cached_compute to guarded_compute(pcache, ...), which is what create_cached does before play (the precompile runs before play_cache_begin).

This PC has no MSVC STL, so CI is the check for 1 and 3. Agreed on landing after #1660.

Thanks for the review. Fixed in 4038ec6, and main merged (f9b9e15, with #1764's fix of `test_nup_stand_in`): 1. `tests/pipeline_play_cache_test.cpp` defines `NOMINMAX` before `windows.h`. I reproduced the windows-2022 error by building the old fixture against a `windows.h` that defines `min`/`max` as the SDK's does (same error at `pipeline_play_cache.inc:41`); the new one builds. 2. `cached_graphics`/`cached_compute` now sit below `build_graphics`' declaration and `driver_cache_only`; the test's START..END slice holds only the Linux guard again. 3. `test_precompile_compute_guard.py`'s fixture forwards `cached_compute` to `guarded_compute(pcache, ...)`, which is what `create_cached` does before play (the precompile runs before `play_cache_begin`). This PC has no MSVC STL, so CI is the check for 1 and 3. Agreed on landing after #1660.
DUDUYG commented 2026-11-10 17:06:15 +00:00 (Migrated from github.com)

main moved after my last push (#1586, #1574), so I merged it again (efc73a3). Two things in it touch this PR:

  • #1586's stage libraries now go through cached_graphics (the conflict in build_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.py checks that line in its new form.
  • #1574's FSR 3.1 passes were made with pcache in play, which would grow it again. 46efc77 routes them through cached_compute (shared_compute_pipeline replaces shared_pipeline_cache), which also gives them the driver fault guard.
main moved after my last push (#1586, #1574), so I merged it again (efc73a3). Two things in it touch this PR: - #1586's stage libraries now go through `cached_graphics` (the conflict in `build_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.py` checks that line in its new form. - #1574's FSR 3.1 passes were made with `pcache` in play, which would grow it again. 46efc77 routes them through `cached_compute` (`shared_compute_pipeline` replaces `shared_pipeline_cache`), which also gives them the driver fault guard.
DUDUYG commented 2026-11-10 17:27:44 +00:00 (Migrated from github.com)

Merged main again (#1660, #1675 and #1765 landed). 8e54706: main's AMD retry in build_graphics makes both attempts through graphics_with (pcache through the play caches). 6593de7: #1765 asks pcache under FAIL_ON_PIPELINE_COMPILE_REQUIRED at the dispatch, in play; NVIDIA answers such an ask from its own disk cache and copies the pipeline into the cache asked, so pcache would grow again. create_cached now takes an ask under the flag through the generation then pcache, without the compile, and the dispatch asks through cached_compute; the route case checks it.

Merged main again (#1660, #1675 and #1765 landed). 8e54706: main's AMD retry in `build_graphics` makes both attempts through `graphics_with` (pcache through the play caches). 6593de7: #1765 asks `pcache` under `FAIL_ON_PIPELINE_COMPILE_REQUIRED` at the dispatch, in play; NVIDIA answers such an ask from its own disk cache and copies the pipeline into the cache asked, so `pcache` would grow again. `create_cached` now takes an ask under the flag through the generation then `pcache`, without the compile, and the dispatch asks through `cached_compute`; the route case checks it.
hckrman101 commented 2026-11-10 17:50:35 +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!
DUDUYG commented 2026-11-10 17:56:01 +00:00 (Migrated from github.com)

Merged main (28ded6f). The only conflict was tests/run_all.py: main moved its tables into the tests, so run_all.py is main's and test_pipeline_play_cache.py declares RUN_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_visibility and test_sampler_admission: fixed in main by 2f4c072.
  • test_draw_parameters: reads only emit_spirv.cpp, which this PR does not touch.
  • test_lift_cleanup_error, test_recomplauncher_json and test_nup_out_of_space: they fail on main's own CI too.
Merged main (28ded6f). The only conflict was `tests/run_all.py`: main moved its tables into the tests, so `run_all.py` is main's and `test_pipeline_play_cache.py` declares `RUN_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_visibility` and `test_sampler_admission`: fixed in main by 2f4c072. - `test_draw_parameters`: reads only `emit_spirv.cpp`, which this PR does not touch. - `test_lift_cleanup_error`, `test_recomplauncher_json` and `test_nup_out_of_space`: they fail on main's own CI too.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 18:14:36 +00:00
hckrman101 (Migrated from github.com) left a comment

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.py passes on windows-2022 (NOMINMAX), test_linux_driver_fault_guard.py passes on ubuntu (the wrappers now sit below the slice), and test_precompile_compute_guard.py passes with the cached_compute stand-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 declares RUN_ALL_ARGS to 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 through shared_compute_pipeline, and the dispatch's under-flag ask now uses cached_compute with the ask-only path, so it stops at step 2. That all looks right. The users count and re-check in create_cached against the swap-then-wait in play_cache_save is 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 direct pcache creates 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.py on ubuntu, nx2/test_nup_out_of_space.py on windows). They're not from this PR. Merging main again later should clear them once main is fixed.

One nit inline. Approving.

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.py` passes on windows-2022 (NOMINMAX), `test_linux_driver_fault_guard.py` passes on ubuntu (the wrappers now sit below the slice), and `test_precompile_compute_guard.py` passes with the `cached_compute` stand-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 declares `RUN_ALL_ARGS` to 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 through `shared_compute_pipeline`, and the dispatch's under-flag ask now uses `cached_compute` with the ask-only path, so it stops at step 2. That all looks right. The users count and re-check in `create_cached` against the swap-then-wait in `play_cache_save` is 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 direct `pcache` creates 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.py` on ubuntu, `nx2/test_nup_out_of_space.py` on windows). They're not from this PR. Merging main again later should clear them once main is fixed. One nit inline. Approving.
hckrman101 (Migrated from github.com) commented 2026-11-10 18:14:36 +00:00

Nit: this runs on the window thread (display.c → rx_close_asked), and play_cache_save spins until the generation's users drops 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 holds mu. It's probably fine for a close, but if it ever shows up as a hang on exit, it could skip the swap when users is nonzero and let the exit-time forced save handle it.

Nit: this runs on the window thread (`display.c` → `rx_close_asked`), and `play_cache_save` spins until the generation's `users` drops 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 holds `mu`. It's probably fine for a close, but if it ever shows up as a hang on exit, it could skip the swap when `users` is nonzero and let the exit-time forced save handle it.
Commenting is not possible because the repository is archived.
No description provided.