vk: a mesh fallback program whose compute pipeline failed is not compiled again #1317

Merged

CiLiBi18 merged 1 commit from fix/mesh-fallback-failed-pipeline-once into main

2026-11-09 23:32:55 +00:00

CiLiBi18 commented

2026-11-09 21:35:34 +00:00

(Migrated from github.com)

fallback_compute_pipelines (gpu/vk/vk_draw.cpp) builds the task and mesh compute pipelines on a program's first fallback draw. When one fails, nothing is kept, so every later draw of that program compiles it again. With a driver that faults on the module, each try is one more fault caught by guarded_compute:

RX 590 / RADV (Crono141 in #1001): 12,160 faults for a5da9f84b72d1471 in one Kokiri Forest run, and 2–3 fps, because the replay retried that pipeline on every draw.
GTX 1070 / NVIDIA 580.178 on Linux (mine): the replay stopped presenting after the first fault.

With this change, the failure is remembered by the program's key and variant, and its draws are left out without another try. That is what the first failure already did for that draw. A program the driver can't compile then costs one missing effect instead of the frame rate. The fix behind those faults is the separate #792 port. This change only stops the next bad module from costing the frame rate again.

The record is a std::set of (key, variant) under the function's existing mutex, so Program keeps its layout and the program cache's stamp doesn't change.
tests/test_mesh_fallback_failed_once.py checks that the record is kept, checked under the lock before guarded_compute, and written before the return. I validated the test by mutation: it fails on main, without the check and without the insert.

hookshot_vk.so builds on Linux with it, and python -I -B tests/run_all.py passes, except nx2/test_nup_read_name.py (zstandard is missing on my machine; it fails the same way on main). It has not been run in game: with #792's fixes, the GTX 1070 no longer reaches this path.

`fallback_compute_pipelines` (gpu/vk/vk_draw.cpp) builds the task and mesh compute pipelines on a program's first fallback draw. When one fails, nothing is kept, so every later draw of that program compiles it again. With a driver that faults on the module, each try is one more fault caught by `guarded_compute`: - RX 590 / RADV (Crono141 in #1001): 12,160 faults for `a5da9f84b72d1471` in one Kokiri Forest run, and 2–3 fps, because the replay retried that pipeline on every draw. - GTX 1070 / NVIDIA 580.178 on Linux (mine): the replay stopped presenting after the first fault. With this change, the failure is remembered by the program's key and variant, and its draws are left out without another try. That is what the first failure already did for that draw. A program the driver can't compile then costs one missing effect instead of the frame rate. The fix behind those faults is the separate #792 port. This change only stops the next bad module from costing the frame rate again. - The record is a `std::set` of (key, variant) under the function's existing mutex, so `Program` keeps its layout and the program cache's stamp doesn't change. - `tests/test_mesh_fallback_failed_once.py` checks that the record is kept, checked under the lock before `guarded_compute`, and written before the return. I validated the test by mutation: it fails on main, without the check and without the insert. `hookshot_vk.so` builds on Linux with it, and `python -I -B tests/run_all.py` passes, except `nx2/test_nup_read_name.py` (`zstandard` is missing on my machine; it fails the same way on main). It has not been run in game: with #792's fixes, the GTX 1070 no longer reaches this path.

hckrman101 (Migrated from github.com) approved these changes

2026-11-09 22:44:51 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good. The failed set is checked and written under the same mu as the rest of the function, key/variant are both on Program, and the early return false is the same thing the old failure path already did for that draw, so callers (fallback_record -> SKIP_HERE) behave the same, just without the retry. Test checks the right ordering (check after the lock, before guarded_compute; insert before the return).

Overlap with #1314: both touch gpu/vk/vk_draw.cpp, but in different places (this one is inside fallback_compute_pipelines, #1314 starts a few lines later in fallback_record and in draw()), so they should merge cleanly, and they are complementary: #1314 makes sure the descriptors are right, this stops a module that still fails from being retried every draw. Their tests check different blocks, no conflict there either.

Nits, nothing blocking:

The record is permanent for the process, so a transient failure (e.g. out of host memory) would drop that program's draws for the rest of the run. Probably the right trade-off here, maybe worth a word in the comment.
When a later stage fails, the pipelines already made for earlier stages in made are never destroyed (was already the case before, but now it happens once per program instead of per draw, so it's bounded).

No CI checks are reported for the branch, and I didn't build it.

Looks good. The failed set is checked and written under the same `mu` as the rest of the function, `key`/`variant` are both on `Program`, and the early `return false` is the same thing the old failure path already did for that draw, so callers (`fallback_record` -> SKIP_HERE) behave the same, just without the retry. Test checks the right ordering (check after the lock, before `guarded_compute`; insert before the return). Overlap with #1314: both touch `gpu/vk/vk_draw.cpp`, but in different places (this one is inside `fallback_compute_pipelines`, #1314 starts a few lines later in `fallback_record` and in `draw()`), so they should merge cleanly, and they are complementary: #1314 makes sure the descriptors are right, this stops a module that still fails from being retried every draw. Their tests check different blocks, no conflict there either. Nits, nothing blocking: - The record is permanent for the process, so a transient failure (e.g. out of host memory) would drop that program's draws for the rest of the run. Probably the right trade-off here, maybe worth a word in the comment. - When a later stage fails, the pipelines already made for earlier stages in `made` are never destroyed (was already the case before, but now it happens once per program instead of per draw, so it's bounded). No CI checks are reported for the branch, and I didn't build it.