Leave a stage library the driver has not compiled to a worker #1429

Merged
jarynclouatre merged 2 commits from fix/gpl-cold-libraries into main 2026-11-10 04:42:25 +00:00
jarynclouatre commented 2026-11-10 01:56:23 +00:00 (Migrated from github.com)

With RX_VK_GPL set, a draw links its pipeline from stage libraries, and a library the driver had not compiled was compiled right there, holding the draw. Where the device has pipelineCreationCacheControl, the draw now asks for the two libraries that hold shaders (pre-rasterization and fragment) with VK_PIPELINE_CREATE_FAIL_ON_PIPELINE_COMPILE_REQUIRED_BIT. If either comes back VK_PIPELINE_COMPILE_REQUIRED the pipeline is not linked: the draw is left out or waits for the worker's full build, exactly as without libraries, and the worker builds the cold libraries after it, for the program's next pipeline. A library that fails is not asked for again. With RX_VK_GPL set, the frame line also counts the libraries the driver had, the cold ones, and how many of those the workers built and in what time. RX_VK_GPL_SYNC (set to anything): compiled in the draw, as before.

Testing: RTX 4070 Ti Super, driver 621.14, Windows 11, Ryzen 7 9800X3D. A scripted 10-minute route (boot, Hyrule Field, Kakariko, Market, Lost Woods, Gerudo Valley), RX_VK_GPL=1, libraries not warmed, one run each on 7dbdcdc:

libraries off this compiled in the draw (RX_VK_GPL_SYNC)
frames presented in 595 s 33,000 33,000 25,800
time lost against 60 fps, s 45.5 45.2 165.7
replay thread's time in pipelines, s 3.5 4.3 153.3
draws left out 265,836 144,300 324

Alone it makes RX_VK_GPL as fast as having it off, with fewer draws left out. Warming the libraries into the pipeline cache (next PR) is what it is for. tests/test_gpl_cold_library.py is new.

With RX_VK_GPL set, a draw links its pipeline from stage libraries, and a library the driver had not compiled was compiled right there, holding the draw. Where the device has pipelineCreationCacheControl, the draw now asks for the two libraries that hold shaders (pre-rasterization and fragment) with VK_PIPELINE_CREATE_FAIL_ON_PIPELINE_COMPILE_REQUIRED_BIT. If either comes back VK_PIPELINE_COMPILE_REQUIRED the pipeline is not linked: the draw is left out or waits for the worker's full build, exactly as without libraries, and the worker builds the cold libraries after it, for the program's next pipeline. A library that fails is not asked for again. With RX_VK_GPL set, the frame line also counts the libraries the driver had, the cold ones, and how many of those the workers built and in what time. RX_VK_GPL_SYNC (set to anything): compiled in the draw, as before. Testing: RTX 4070 Ti Super, driver 621.14, Windows 11, Ryzen 7 9800X3D. A scripted 10-minute route (boot, Hyrule Field, Kakariko, Market, Lost Woods, Gerudo Valley), RX_VK_GPL=1, libraries not warmed, one run each on 7dbdcdc: | | libraries off | this | compiled in the draw (RX_VK_GPL_SYNC) | |---|---|---|---| | frames presented in 595 s | 33,000 | 33,000 | 25,800 | | time lost against 60 fps, s | 45.5 | 45.2 | 165.7 | | replay thread's time in pipelines, s | 3.5 | 4.3 | 153.3 | | draws left out | 265,836 | 144,300 | 324 | Alone it makes RX_VK_GPL as fast as having it off, with fewer draws left out. Warming the libraries into the pipeline cache (next PR) is what it is for. tests/test_gpl_cold_library.py is new.
NotRexed commented 2026-11-10 03:49:50 +00:00 (Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!
NotRexed commented 2026-11-10 03:55:30 +00:00 (Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.
jarynclouatre commented 2026-11-10 04:17:40 +00:00 (Migrated from github.com)

Rebased on main and resolved the conflict in gpu/vk/vk_replay.cpp: the frame line keeps main's descriptor saves and adds the library counts after them. Nothing else changed. The tests pass on the rebased branch; I have not run the game on it yet.

Rebased on main and resolved the conflict in gpu/vk/vk_replay.cpp: the frame line keeps main's descriptor saves and adds the library counts after them. Nothing else changed. The tests pass on the rebased branch; I have not run the game on it yet.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 04:19:31 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. I went through the handoff between the draw and the workers and couldn't find a hole:

  • The cold key only goes back to the thread whose emplace was fresh, so a library is queued at most once even when two draw threads race on it. A null entry stops anyone from asking the driver again, and gpl_build_cold replaces that null under gpl_mu. A failure just leaves it null.
  • The cold jobs are pushed after ready.store(true) and with no flag, so a draw that steals the full build in jobs_wait doesn't also end up running the library builds. jobs_wait drops mu before it runs the job, so calling jobs_push from inside the job can't deadlock.
  • Lifetime is fine. The job is held by a shared_ptr, Program already outlives its pipeline jobs, and gpl_libs is never torn down, so a worker can't write into a map that's been destroyed.
  • The device gets the queried feature chain, so pipelineCreationCacheControl is enabled whenever dev.vk13 reports it.

One thing about drivers. The libraries are created with VK_NULL_HANDLE as the pipeline cache, so "the driver had it" only works where the driver keeps its own cache (NVIDIA, and RADV/ANV through Mesa's disk cache). A driver without one will report every shader library as cold on first use. That's harmless here (you get the no-GPL behaviour, then the worker-built library for the program's later pipelines), but the next PR's cache warming will probably need these libraries to go through pcache to help on those drivers.

Two small nits (non-blocking):

  • tests/test_gpl_cold_library.py: Nit: tests/run_all.py only checks for or skips a compiler when the test has an ARGS entry. Without one, this runs g++ unconditionally, so it fails instead of skipping on a machine without g++. Adding "tests/test_gpl_cold_library.py": ["--cxx", "{cxx}"], next to the other fixture tests would make it use whatever compiler the runner finds.
  • gpu/vk/vk_draw.cpp: Nit: since this only checks presence, RX_VK_GPL_SYNC=0 turns it on too. That matches how RX_VK_GPL works, but the block comment above says RX_VK_GPL_SYNC=1. Maybe say "set" there, like the PR description does.

The conflict with #1330 in log_frame_stats is resolved in your merge of main and keeps both sides, so it merges cleanly now.

Looks good. I went through the handoff between the draw and the workers and couldn't find a hole: - The cold key only goes back to the thread whose `emplace` was fresh, so a library is queued at most once even when two draw threads race on it. A null entry stops anyone from asking the driver again, and `gpl_build_cold` replaces that null under `gpl_mu`. A failure just leaves it null. - The cold jobs are pushed after `ready.store(true)` and with no flag, so a draw that steals the full build in `jobs_wait` doesn't also end up running the library builds. `jobs_wait` drops `mu` before it runs the job, so calling `jobs_push` from inside the job can't deadlock. - Lifetime is fine. The job is held by a shared_ptr, `Program` already outlives its pipeline jobs, and `gpl_libs` is never torn down, so a worker can't write into a map that's been destroyed. - The device gets the queried feature chain, so `pipelineCreationCacheControl` is enabled whenever `dev.vk13` reports it. One thing about drivers. The libraries are created with `VK_NULL_HANDLE` as the pipeline cache, so "the driver had it" only works where the driver keeps its own cache (NVIDIA, and RADV/ANV through Mesa's disk cache). A driver without one will report every shader library as cold on first use. That's harmless here (you get the no-GPL behaviour, then the worker-built library for the program's later pipelines), but the next PR's cache warming will probably need these libraries to go through `pcache` to help on those drivers. Two small nits (non-blocking): - tests/test_gpl_cold_library.py: Nit: `tests/run_all.py` only checks for or skips a compiler when the test has an `ARGS` entry. Without one, this runs `g++` unconditionally, so it fails instead of skipping on a machine without g++. Adding `"tests/test_gpl_cold_library.py": ["--cxx", "{cxx}"],` next to the other fixture tests would make it use whatever compiler the runner finds. - gpu/vk/vk_draw.cpp: Nit: since this only checks presence, `RX_VK_GPL_SYNC=0` turns it on too. That matches how `RX_VK_GPL` works, but the block comment above says `RX_VK_GPL_SYNC=1`. Maybe say "set" there, like the PR description does. The conflict with #1330 in `log_frame_stats` is resolved in your merge of main and keeps both sides, so it merges cleanly now.
jarynclouatre commented 2026-11-10 04:23:11 +00:00 (Migrated from github.com)

Thanks, both fixed in the new commit: the test is in run_all.py's ARGS with {cxx}, and the comment says "set".

Thanks, both fixed in the new commit: the test is in run_all.py's ARGS with {cxx}, and the comment says "set".
Commenting is not possible because the repository is archived.
No description provided.