MagicBeanDev/Hookshot

Archived

Watch

Fork 0

feat(vk): a state list record with no variant prepares the plain one; 11,985 pipelines from Fritchii's recipe #2134

Merged

pmwork-personal merged 5 commits from feat/state-list-plain-variant into main

2026-11-12 03:04:07 +00:00

pmwork-personal commented

2026-11-11 04:30:25 +00:00

(Migrated from github.com)

state_list_prepare: a record with variant 0 and no answers or questions stands for the variant the program's translation makes with nothing bound, each texture as its instruction declares it. When the record's own variant is not found (a program without textures has variant 0 already and is found as before), it is made with seed_plain_of and the pipeline noted with it. Draws whose textures differ make their own variant as before; the prepared pipeline is just not used. Not seed_variant_of with empty answers: that variant would hold no answers and stand for any textures. The log's state list line counts them ("by nothing bound").

seed_plain_of shares seed_variant_of's body (seed_variant_made), which hands the translation questions and answers only when given; without them it answers what nothing bound answers and notes the questions, as the seed's own translation does.

contrib/pmwork-personal-fritchii-recipe-plain: those 11,985 records (11,111 graphics, 874 mesh, 4,785 programs). A build without this rule leaves them out ("whose program variant was not made"). test_state_list_plain.py checks the rule in the source.

Since review (hckrman101):

The plain variant's own variant is the hash of its answers, not 0, so program_find_portable(key, 0) never found it, and each prepare translated the list's ~4.8k plain programs again and appended them to hookshot_programs.bin. Now program_find_plain(key) finds the loaded variant by its answers (an answer for every slot, each its declared type; one declared_answer helper shared with record_answers), and state_list_prepare queues a plain record only when that finds nothing. The prepare's log before this fix: a first prepare in a fresh folder made "4798 by nothing bound (4798 made)", and every later prepare made them again. With it, even a cold first prepare shows "0 by nothing bound (0 made)": a plain variant is the program's own first translation, so the 11,985 pipelines are noted "by nothing bound" with no extra translation (cost measured in the comments).
Disjoint lists: by pipeline (the record without its variant, answers and questions), none of the 11,985 is in #2133's list, any list on main as of 34f05785, or #2143's lists.
This lands after #2133 (the contrib note and README text come from there).
Tests: test_state_list_plain.py checks the lookup and both its uses; test_program_answer_domains.py copies declared_answer with record_answers; main's new test_seed_variant_cleanup.py (#2323) copies seed_variant_made with seed_variant_of. tests/run_all.py on Windows after merging main at 34f05785: 379 passed, 31 skipped, 2 failed, both failing on untouched main here too (test_recorder_sync, test_vram_budget_row). A Windows build compiles.

Lists converted from records that hold no texture answers, like Fritchii's shader recipe, cannot name a program variant. 11,985 of the recipe's pipelines that no list here holds have vertex input the lists here fix but no known variant; a build preparing them with the variant nothing bound makes builds most of them as the draws will ask. state_list_prepare: a record with variant 0 and no answers or questions stands for the variant the program's translation makes with nothing bound, each texture as its instruction declares it. When the record's own variant is not found (a program without textures has variant 0 already and is found as before), it is made with seed_plain_of and the pipeline noted with it. Draws whose textures differ make their own variant as before; the prepared pipeline is just not used. Not seed_variant_of with empty answers: that variant would hold no answers and stand for any textures. The log's state list line counts them ("by nothing bound"). seed_plain_of shares seed_variant_of's body (seed_variant_made), which hands the translation questions and answers only when given; without them it answers what nothing bound answers and notes the questions, as the seed's own translation does. contrib/pmwork-personal-fritchii-recipe-plain: those 11,985 records (11,111 graphics, 874 mesh, 4,785 programs). A build without this rule leaves them out ("whose program variant was not made"). test_state_list_plain.py checks the rule in the source. **Since review** (hckrman101): - The plain variant's own variant is the hash of its answers, not 0, so `program_find_portable(key, 0)` never found it, and each prepare translated the list's ~4.8k plain programs again and appended them to `hookshot_programs.bin`. Now `program_find_plain(key)` finds the loaded variant by its answers (an answer for every slot, each its declared type; one `declared_answer` helper shared with `record_answers`), and `state_list_prepare` queues a plain record only when that finds nothing. The prepare's log before this fix: a first prepare in a fresh folder made "4798 by nothing bound (4798 made)", and every later prepare made them again. With it, even a cold first prepare shows "0 by nothing bound (0 made)": a plain variant is the program's own first translation, so the 11,985 pipelines are noted "by nothing bound" with no extra translation (cost measured in the comments). - Disjoint lists: by pipeline (the record without its variant, answers and questions), none of the 11,985 is in #2133's list, any list on main as of 34f05785, or #2143's lists. - This lands after #2133 (the contrib note and README text come from there). - Tests: `test_state_list_plain.py` checks the lookup and both its uses; `test_program_answer_domains.py` copies `declared_answer` with `record_answers`; main's new `test_seed_variant_cleanup.py` (#2323) copies `seed_variant_made` with `seed_variant_of`. `tests/run_all.py` on Windows after merging main at 34f05785: 379 passed, 31 skipped, 2 failed, both failing on untouched main here too (`test_recorder_sync`, `test_vram_budget_row`). A Windows build compiles.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 18:26:06 +00:00

hckrman101 (Migrated from github.com) left a comment

One real problem, inline: the plain variant is never found by its own hash. So every state_list_prepare run (each prepare, and each Builder run with refresh_states) translates all of these programs again (about 4.8k with the contrib list) and appends a duplicate record for each to hookshot_programs.bin. program_cache_save doesn't dedupe.

Smaller points:

The contrib note refers to pmwork-personal-fritchii-recipe's note, and the README text covering contrib/pmwork-personal-fritchii-recipe* comes from #2133. Neither is on main yet, so this should land after #2133. Can you also confirm the 11,985 records are disjoint from #2133's list (and #2143's, if that lands first)? "Not held by main's lists at f915af9" doesn't say whether #2133 was counted.
#2414 rewrites the variant_worker thread-spawn line right after your hunk. It's a trivial conflict, whichever lands second.
"builds most of them as the draws will ask" can only be checked in play. A log line from a run with the list (the new "by nothing bound (N made)" count, plus how many of those pipelines were later used) would back it up. A mismatch only costs an unused pipeline, so it doesn't block.

CI: everything passes except Windows part 1/2, where test_native_astc_texture_count.py fails (scanf deprecated under -Werror). This PR doesn't touch that test.

Thanks, the idea holds up. A record with variant 0 and no answers or questions can't come from a real recording of a textured program, so old lists aren't misread. Programs without textures still hit `program_find_portable(key, 0)` first. Builds without this rule just count these records as "whose program variant was not made". The record format and both cache stamps are unchanged, which is right because nothing on disk changes shape. Going through the textures as their instructions declare them (`record_answers` with nothing bound, non-empty answers) also keeps the plain variant from passing as "any textures" in `still_valid`. Good catch, and the test pins it. One real problem, inline: the plain variant is never found by its own hash. So every `state_list_prepare` run (each prepare, and each Builder run with `refresh_states`) translates all of these programs again (about 4.8k with the contrib list) and appends a duplicate record for each to `hookshot_programs.bin`. `program_cache_save` doesn't dedupe. Smaller points: - The contrib note refers to `pmwork-personal-fritchii-recipe`'s note, and the README text covering `contrib/pmwork-personal-fritchii-recipe*` comes from #2133. Neither is on main yet, so this should land after #2133. Can you also confirm the 11,985 records are disjoint from #2133's list (and #2143's, if that lands first)? "Not held by main's lists at f915af9" doesn't say whether #2133 was counted. - #2414 rewrites the `variant_worker` thread-spawn line right after your hunk. It's a trivial conflict, whichever lands second. - "builds most of them as the draws will ask" can only be checked in play. A log line from a run with the list (the new "by nothing bound (N made)" count, plus how many of those pipelines were later used) would back it up. A mismatch only costs an unused pipeline, so it doesn't block. CI: everything passes except Windows part 1/2, where `test_native_astc_texture_count.py` fails (`scanf` deprecated under -Werror). This PR doesn't touch that test.

gpu/vk/vk_draw.cpp

The plain variant's portable_variant is the hash of its declared-type answers, not 0. So program_find_portable(key, 0) misses it on every run, even when it's already loaded: seed_program_of translated it with nothing bound on the first run, and program_cache_load_some has just read it back in. Every plain record's key then goes back into variant_of. seed_variant_made does dedupe in memory, but only after a full translation, and translate_into has already called program_cache_save. The result: ~4.8k translations on every prepare/refresh, and hookshot_programs.bin grows by that many records each time.

Could you look up the loaded plain variant before queueing? For example, a program_find_plain(key) that returns the ready variant whose answers each equal what record_answers gives an unbound slot (the declared type / is_integer bit, after canonical_answer). Then only queue seed_plain_of when that returns null.

The plain variant's `portable_variant` is the hash of its declared-type answers, not 0. So `program_find_portable(key, 0)` misses it on every run, even when it's already loaded: `seed_program_of` translated it with nothing bound on the first run, and `program_cache_load_some` has just read it back in. Every plain record's key then goes back into `variant_of`. `seed_variant_made` does dedupe in memory, but only after a full translation, and `translate_into` has already called `program_cache_save`. The result: ~4.8k translations on every prepare/refresh, and `hookshot_programs.bin` grows by that many records each time. Could you look up the loaded plain variant before queueing? For example, a `program_find_plain(key)` that returns the ready variant whose answers each equal what `record_answers` gives an unbound slot (the declared type / `is_integer` bit, after `canonical_answer`). Then only queue `seed_plain_of` when that returns null.

pmwork-personal (Migrated from github.com) reviewed

2026-11-11 20:12:56 +00:00

gpu/vk/vk_draw.cpp

Fixed in 865f8327. program_find_plain(key) returns the loaded, ready variant whose answers cover every slot record_answers counts, each the declared type after canonical_answer. The declared type now comes from one helper, declared_answer, which record_answers also uses for an unbound slot, so the two can't drift apart. state_list_prepare queues a plain record only when program_find_plain returns null, and notes the loaded variant's pipeline from it too. test_state_list_plain.py checks both places.

Fixed in 865f8327. `program_find_plain(key)` returns the loaded, ready variant whose answers cover every slot `record_answers` counts, each the declared type after `canonical_answer`. The declared type now comes from one helper, `declared_answer`, which `record_answers` also uses for an unbound slot, so the two can't drift apart. `state_list_prepare` queues a plain record only when `program_find_plain` returns null, and notes the loaded variant's pipeline from it too. `test_state_list_plain.py` checks both places.

pmwork-personal commented

2026-11-11 20:12:58 +00:00

(Migrated from github.com)

Thanks for the review. The fix is in 865f8327 (reply inline), on top of a merge of main. On the other points:

Disjoint from #2133: yes. I compared by pipeline, meaning the record without its variant, answers and questions, because #2133's records carry answers and these have none. 0 of the 11,985 are in #2133's 1,461, in any list on main as of 34f05785 (#2429's and #2445's included), or in #2143's lists.
Order: agreed, this lands after #2133 (its note and the README text come from there).
#2414: noted, a trivial conflict whichever lands second.
A run with the list: The prepare's own log line backs it up. A first prepare in a fresh game folder (11-10): "4798 by nothing bound (4798 made), ... 11985 by nothing bound" noted. A Builder prepare with this fix, in a folder that had prepared the list before: "0 by nothing bound (0 made), 40702 noted (2576 native mesh, 0 mesh fallback, 11985 by nothing bound)". So the loaded plain variants are found and their 11,985 pipelines prepared, and nothing is translated or saved again. How many of those pipelines the draws later use I haven't measured yet; that needs a play run with a use count.

Thanks for the review. The fix is in 865f8327 (reply inline), on top of a merge of main. On the other points: - **Disjoint from #2133:** yes. I compared by pipeline, meaning the record without its variant, answers and questions, because #2133's records carry answers and these have none. 0 of the 11,985 are in #2133's 1,461, in any list on main as of 34f05785 (#2429's and #2445's included), or in #2143's lists. - **Order:** agreed, this lands after #2133 (its note and the README text come from there). - **#2414:** noted, a trivial conflict whichever lands second. - **A run with the list:** The prepare's own log line backs it up. A first prepare in a fresh game folder (11-10): "4798 by nothing bound (4798 made), ... 11985 by nothing bound" noted. A Builder prepare with this fix, in a folder that had prepared the list before: "0 by nothing bound (0 made), 40702 noted (2576 native mesh, 0 mesh fallback, 11985 by nothing bound)". So the loaded plain variants are found and their 11,985 pipelines prepared, and nothing is translated or saved again. How many of those pipelines the draws later use I haven't measured yet; that needs a play run with a use count.

Joey-Magnus (Migrated from github.com) reviewed

2026-11-11 21:25:55 +00:00

Joey-Magnus (Migrated from github.com) left a comment

The code is sound and the earlier bug is fixed: program_find_plain finds the loaded plain variant by its answers before anything is queued, so nothing is retranslated and saved on each prepare, and made_plain[i] is one index per worker.

Whether to ship it depends on #2133. This adds 11,985 converted Fritchii pipelines, including predicted states. With #2133 the shipped list grows about 53% (25.2k to 38.7k), plus 4,798 program translations on first start (10.2 s on an RTX 5090 machine), with nothing measured on low-end hardware. The title is cut off with "…" and needs fixing before squash.

The code is sound and the earlier bug is fixed: `program_find_plain` finds the loaded plain variant by its answers before anything is queued, so nothing is retranslated and saved on each prepare, and `made_plain[i]` is one index per worker. Whether to ship it depends on #2133. This adds 11,985 converted Fritchii pipelines, including predicted states. With #2133 the shipped list grows about 53% (25.2k to 38.7k), plus 4,798 program translations on first start (10.2 s on an RTX 5090 machine), with nothing measured on low-end hardware. The title is cut off with "…" and needs fixing before squash.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 22:21:03 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for the quick turnaround. Re-checked at 8f3910d.

Fixed:

The re-translation on every prepare. program_find_plain finds the loaded plain variant by its answers (one per counted slot, each canonical_answer of declared_answer), and state_list_prepare only queues a plain record when that returns null. It's also used when noting the pipeline, so a variant loaded from the cache still gets its pipeline prepared. Because record_answers now uses the same declared_answer, the two can't drift apart. It takes shader_mu once and calls nothing that locks it again. Your log numbers ("0 by nothing bound (0 made)" on a second prepare) match the code.
Order: #2133 is merged now, so the contrib note and README reference resolve. The disjointness check against #2133, main at 34f05785 and #2143 answers my question.
test_seed_variant_cleanup.py copies seed_variant_made too, and test_state_list_plain.py pins the lookup and both places that use it. CI is all green and the PR merges cleanly. #2414 is still a trivial conflict for whichever lands second.

Still open: the cost-versus-benefit point @Joey-Magnus raised. With #2133 in, this grows the shipped list by about half and adds ~4.8k program translations to the first start (10 s on a 5090), and nobody has measured it on low-end hardware or counted how many of the 11,985 pipelines draws actually use. My earlier "doesn't block" only covered the case where a prepared pipeline goes unused. It didn't account for the first-start cost everyone pays. A first-prepare time on a low-end machine (or a use count from a play run) would settle that, and whether it's worth shipping is NotRexed's call. The code is ready from my side.

Nit: the title is still cut off at "…", so please fix it before squash (Joey's point).

Thanks for the quick turnaround. Re-checked at 8f3910d. Fixed: - The re-translation on every prepare. `program_find_plain` finds the loaded plain variant by its answers (one per counted slot, each `canonical_answer` of `declared_answer`), and `state_list_prepare` only queues a plain record when that returns null. It's also used when noting the pipeline, so a variant loaded from the cache still gets its pipeline prepared. Because `record_answers` now uses the same `declared_answer`, the two can't drift apart. It takes `shader_mu` once and calls nothing that locks it again. Your log numbers ("0 by nothing bound (0 made)" on a second prepare) match the code. - Order: #2133 is merged now, so the contrib note and README reference resolve. The disjointness check against #2133, main at 34f05785 and #2143 answers my question. - `test_seed_variant_cleanup.py` copies `seed_variant_made` too, and `test_state_list_plain.py` pins the lookup and both places that use it. CI is all green and the PR merges cleanly. #2414 is still a trivial conflict for whichever lands second. Still open: the cost-versus-benefit point @Joey-Magnus raised. With #2133 in, this grows the shipped list by about half and adds ~4.8k program translations to the first start (10 s on a 5090), and nobody has measured it on low-end hardware or counted how many of the 11,985 pipelines draws actually use. My earlier "doesn't block" only covered the case where a prepared pipeline goes unused. It didn't account for the first-start cost everyone pays. A first-prepare time on a low-end machine (or a use count from a play run) would settle that, and whether it's worth shipping is NotRexed's call. The code is ready from my side. Nit: the title is still cut off at "…", so please fix it before squash (Joey's point).

hckrman101 (Migrated from github.com) reviewed

2026-11-11 22:21:12 +00:00

gpu/vk/vk_draw.cpp

Confirmed at 8f3910d: the plain record is only queued when program_find_plain finds nothing, and the noting path falls back to it as well. Resolved, thanks.

Confirmed at 8f3910d: the plain record is only queued when `program_find_plain` finds nothing, and the noting path falls back to it as well. Resolved, thanks.

pmwork-personal commented

2026-11-12 00:09:23 +00:00

(Migrated from github.com)

Thanks @Joey-Magnus. I measured the cost, and one number in my earlier comment was wrong.

No extra translations. The "4798 by nothing bound (4798 made)" I quoted came from a first prepare before the program_find_plain fix, which re-made the plain variants it couldn't find. A plain variant is the program's own first translation (nothing bound), so a fresh prepare already has it. With the fix, a cold first start shows 0 by nothing bound (0 made), ... 11985 by nothing bound noted.

What it adds is pipelines. This head merged with today's main (so #2133's list is in), hookshot --prepare-shaders from cold caches each time (Hookshot's program and pipeline caches deleted, NVIDIA's shader disk cache pointed at an empty folder), Ryzen 7 9800X3D (16 threads) + RTX 5090:

	Without this list	With it
Shipped list	29,688 pipelines	41,673 (+40% on today's main)
Plain variants translated	—	0
List pipelines compiled in the precompile	27,880	35,253 (+7,373)
Precompile	158.6 s	170.8 s (+12.3 s)

The whole cold prepare took 896 s and 687 s, but those totals aren't comparable: the first run was slower in every phase, including the shader translation the list doesn't touch (328 vs 232 s), so it paid a one-time cost (the files not yet in the OS cache). The precompile line is the like-for-like part. I haven't measured a smaller CPU yet (an 8-CPU affinity run is the next step), or how many of these pipelines play actually draws.

Title fixed too.

Thanks @Joey-Magnus. I measured the cost, and one number in my earlier comment was wrong. **No extra translations.** The "4798 by nothing bound (4798 made)" I quoted came from a first prepare *before* the `program_find_plain` fix, which re-made the plain variants it couldn't find. A plain variant is the program's own first translation (nothing bound), so a fresh prepare already has it. With the fix, a cold first start shows `0 by nothing bound (0 made), ... 11985 by nothing bound` noted. **What it adds is pipelines.** This head merged with today's main (so #2133's list is in), `hookshot --prepare-shaders` from cold caches each time (Hookshot's program and pipeline caches deleted, NVIDIA's shader disk cache pointed at an empty folder), Ryzen 7 9800X3D (16 threads) + RTX 5090: | | Without this list | With it | |---|---|---| | Shipped list | 29,688 pipelines | 41,673 (+40% on today's main) | | Plain variants translated | — | 0 | | List pipelines compiled in the precompile | 27,880 | 35,253 (+7,373) | | Precompile | 158.6 s | 170.8 s (+12.3 s) | The whole cold prepare took 896 s and 687 s, but those totals aren't comparable: the first run was slower in every phase, including the shader translation the list doesn't touch (328 vs 232 s), so it paid a one-time cost (the files not yet in the OS cache). The precompile line is the like-for-like part. I haven't measured a smaller CPU yet (an 8-CPU affinity run is the next step), or how many of these pipelines play actually draws. Title fixed too.