perf(vk): cache mesh shader picks in thread-local program caches #2476

Open
Erfboom wants to merge 1 commit from Erfboom/perf/vk-mesh-pick-cache into main
Erfboom commented 2026-11-11 20:37:03 +00:00 (Migrated from github.com)

Problem

Mesh shader draws currently bypass the thread-local program pick caches (prog_fast and picks) and unconditionally query mesh_program_for(), acquiring shader_mu on every mesh draw call.

Solution

  1. Thread-Local Pick Caching: Allow ProgramKind::Mesh to query and populate thread-local prog_fast and picks caches before falling back to mesh_program_for().
  2. Prog Match Optimization: In prog_same(), skip vertex attribute comparisons for mesh shaders as mesh stages do not read vertex attribute arrays.
  3. Stage Input Hashing: In program_for(), compute mesh input hashes from color_types without vertex attributes.

Verification

  • python3 -I -B tests/test_program_answer_domains.py --headers /usr/include (all 7 domain configurations: 0 failures)
  • python3 -I -B tests/test_mesh_fallback.py (PASS)
  • python3 -I -B tests/test_state_list_plain.py (PASS)
  • python3 -I -B tests/test_seed_variant_cleanup.py (PASS)
  • python3 -I -B tests/test_pipeline_state_portable.py --headers /usr/include (PASS)
## Problem Mesh shader draws currently bypass the thread-local program pick caches (`prog_fast` and `picks`) and unconditionally query `mesh_program_for()`, acquiring `shader_mu` on every mesh draw call. ## Solution 1. **Thread-Local Pick Caching**: Allow `ProgramKind::Mesh` to query and populate thread-local `prog_fast` and `picks` caches before falling back to `mesh_program_for()`. 2. **Prog Match Optimization**: In `prog_same()`, skip vertex attribute comparisons for mesh shaders as mesh stages do not read vertex attribute arrays. 3. **Stage Input Hashing**: In `program_for()`, compute mesh input hashes from `color_types` without vertex attributes. ## Verification - `python3 -I -B tests/test_program_answer_domains.py --headers /usr/include` (all 7 domain configurations: 0 failures) - `python3 -I -B tests/test_mesh_fallback.py` (PASS) - `python3 -I -B tests/test_state_list_plain.py` (PASS) - `python3 -I -B tests/test_seed_variant_cleanup.py` (PASS) - `python3 -I -B tests/test_pipeline_state_portable.py --headers /usr/include` (PASS)
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 20:37:03 +00:00
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:24:44 +00:00
Joey-Magnus (Migrated from github.com) left a comment

The texture_answer table is already on main: #2439 replaced the hash map with a flat table indexed by slot, which is the conflict. At this PR's head, tests/test_program_answer_domains.py fails ("all UINT handle/version entries preserved") because of the new id | 0x80000000 encoding in answer_handles (vk_shader.cpp:1662). The mesh-pick cache is plausible (the pick only depends on the code and color_types). If you want to keep it, rebase down to that part, update the test, and add a measurement.

The `texture_answer` table is already on main: #2439 replaced the hash map with a flat table indexed by slot, which is the conflict. At this PR's head, `tests/test_program_answer_domains.py` fails ("all UINT handle/version entries preserved") because of the new `id | 0x80000000` encoding in `answer_handles` (`vk_shader.cpp:1662`). The mesh-pick cache is plausible (the pick only depends on the code and `color_types`). If you want to keep it, rebase down to that part, update the test, and add a measurement.
hckrman101 commented 2026-11-11 22:55:54 +00:00 (Migrated from github.com)

This now conflicts with main in gpu/vk/vk_shader.cpp. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.

This now conflicts with main in `gpu/vk/vk_shader.cpp`. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.
Erfboom commented 2026-11-12 04:11:02 +00:00 (Migrated from github.com)

Rebased onto latest main and addressed the review feedback:

  1. Dropped texture_answer conflicts: Since #2439 already landed the flat slot-indexed table on main, all changes to texture_answer have been dropped.
  2. Removed handle bit tagging: Dropped the id | 0x80000000 handle tagging in answer_handles. tests/test_program_answer_domains.py now passes with 0 failures across all 7 test configurations (default, handles-live-zero/one, register-at-once-zero/one, capture-zero/one).
  3. Focused strictly on thread-local mesh pick caching:
    • Skips vertex attribute array checks in prog_same() for ProgramKind::Mesh (mesh stages do not read vertex attribute buffers).
    • Computes mesh stage hash in program_for() from color_types and populates prog_fast and picks, avoiding unconditional fallback to mesh_program_for() and mutex acquisition on every mesh draw.
    • All tests pass (test_program_answer_domains.py, test_mesh_fallback.py, test_state_list_plain.py, test_seed_variant_cleanup.py, test_pipeline_state_portable.py).
Rebased onto latest `main` and addressed the review feedback: 1. **Dropped `texture_answer` conflicts**: Since #2439 already landed the flat slot-indexed table on `main`, all changes to `texture_answer` have been dropped. 2. **Removed handle bit tagging**: Dropped the `id | 0x80000000` handle tagging in `answer_handles`. `tests/test_program_answer_domains.py` now passes with 0 failures across all 7 test configurations (`default`, `handles-live-zero/one`, `register-at-once-zero/one`, `capture-zero/one`). 3. **Focused strictly on thread-local mesh pick caching**: - Skips vertex attribute array checks in `prog_same()` for `ProgramKind::Mesh` (mesh stages do not read vertex attribute buffers). - Computes mesh stage hash in `program_for()` from `color_types` and populates `prog_fast` and `picks`, avoiding unconditional fallback to `mesh_program_for()` and mutex acquisition on every mesh draw. - All tests pass (`test_program_answer_domains.py`, `test_mesh_fallback.py`, `test_state_list_plain.py`, `test_seed_variant_cleanup.py`, `test_pipeline_state_portable.py`).
Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 13:38 UTC · original

Thanks for cutting this down. I re-checked it merged into current main (51da0464):

  • The diff is now only the mesh pick cache, and the 0x80000000 tagging is gone. test_program_answer_domains.py passes, and so does the rest of tests/run_all.py, apart from a lift-cache check that fails on main here too.
  • The logic holds up. A mesh pick only depends on the code and color_types, the fast path skips the vertex attributes only for kind == 2, and the new key separates kinds and stage signatures, so a mesh pick can't answer for a graphics or compute one. The answers' handle check still guards texture-type variants.

Still open from my last review: a measurement. The point of the PR is to skip shader_mu on every mesh draw, so please show that it does. The vk: program picks: ... kept ... new log line before and after in the same scene would do, or a frame-time or lock-wait number from a mesh-heavy spot (Hyrule Field with mesh shaders on).

The four failing Python checks aren't from this PR. They fail in files it doesn't touch (menu translations, audio, Wayland, readback) and pass on current main, so a re-run after merging main should clear them.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 13:38 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2476#pullrequestreview-5875381608) Thanks for cutting this down. I re-checked it merged into current main (51da0464): - The diff is now only the mesh pick cache, and the `0x80000000` tagging is gone. `test_program_answer_domains.py` passes, and so does the rest of `tests/run_all.py`, apart from a lift-cache check that fails on main here too. - The logic holds up. A mesh pick only depends on the code and `color_types`, the fast path skips the vertex attributes only for `kind == 2`, and the new key separates kinds and stage signatures, so a mesh pick can't answer for a graphics or compute one. The answers' handle check still guards texture-type variants. Still open from my last review: **a measurement**. The point of the PR is to skip `shader_mu` on every mesh draw, so please show that it does. The `vk: program picks: ... kept ... new` log line before and after in the same scene would do, or a frame-time or lock-wait number from a mesh-heavy spot (Hyrule Field with mesh shaders on). The four failing Python checks aren't from this PR. They fail in files it doesn't touch (menu translations, audio, Wayland, readback) and pass on current main, so a re-run after merging main should clear them. <!-- github-sync:r:5875381608 -->
Commenting is not possible because the repository is archived.
No description provided.