Repository navigation
feat(vk): a state list record with no variant prepares the plain one; 11,985 pipelines from Fritchii's recipe - #2134
Conversation
…e; 11985 pipelines from Fritchii's recipe
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.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in-variant # Conflicts: # gpu/vk/vk_draw.cpp
hckrman101
left a comment
There was a problem hiding this comment.
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 coveringcontrib/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_workerthread-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.
| const uint64_t key = want[rec_program[i]].key; | ||
| if (key && (r.nanswers || r.nquestions) && !program_find_portable(key, r.variant) && seen.insert(mix(key, r.variant)).second) | ||
| variant_of.push_back(i); | ||
| if (key && (r.nanswers || r.nquestions || plain(r)) && !program_find_portable(key, r.variant) && |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Fixed in 865f832. 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.
There was a problem hiding this comment.
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.
…aved again at each prepare From review: the variant nothing bound makes has the hash of its answers as its variant, not 0, so program_find_portable(key, 0) missed it even when the program cache had just loaded it. Every prepare and Builder refresh translated the list's ~4.8k plain programs again, and appended a record for each to hookshot_programs.bin. - program_find_plain(key): the loaded, ready variant with an answer for every slot record_answers counts, each the declared type after canonical_answer (a variant that asked nothing stands for any textures, and is not it). - declared_answer: what an unbound slot answers, now one helper that record_answers uses too, so the lookup and the recording agree. - state_list_prepare queues a plain record only when program_find_plain finds nothing, and notes the loaded variant's pipeline from it too. - test_state_list_plain.py checks the lookup and both of its uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's NotRexed#2323 added the check, which copies seed_variant_of out of vk_shader.cpp; here seed_variant_of hands its questions and answers to seed_variant_made (shared with seed_plain_of), so the check copies both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. The fix is in 865f832 (reply inline), on top of a merge of main. On the other points:
|
Joey-Magnus
left a comment
There was a problem hiding this 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.
#2133) Fritchii ships a recipe of the pipelines Ocarina of Time draws, and 1,461 of them are pipelines no list here holds that convert to whole HKSTATE2 records. This adds them as `contrib/pmwork-personal-fritchii-recipe`, as discussed in #dev-work. **What the recipe is.** Fritchii (a Lichii fork) ships `OoT v131072 (cache v54).nx2recipe`: the pipelines its testers' sessions drew plus the states it predicts. Each shader is named by a fingerprint, and the recipe holds no code (its format is in `nx2_recipe.h` / `nx2_shader_extract.h` on Fritchii's `oot-gather-fix` branch). **How it maps.** - **Shaders.** The fingerprint is SHA-256 of the stage's control section (first 0x48 bytes, +0x30..0x33 zeroed), then its code area. `code_key` hashes the same control and code bytes with FNV, so a player's `hookshot_shaders.bin` names the recipe's shaders by code key: 41,165 of 41,216. - **Render state.** It maps by meaning: - target and depth formats (Maxwell render-target formats to NVN's, one to one for every format OoT uses) - samples and topology class - blend factors and equations, into `nvn_blend_st` - channel mask, polygon mode, depth clamp and alpha to coverage **What it can't hold.** The recipe was recorded with dynamic vertex input, so it has no vertex formats or divisors. It has no texture answers either. So a pipeline is in this list only when: - main's lists fix both for its program (one vertex input for the vertex stage and one variant), or - it needs neither: mesh draws take no vertex input, and a program none of whose stages asks about a texture or image has variant 0. All 40 such programs in main's lists have variant 0 there. Point-topology states that the recipe predicts for programs main's lists draw as triangles are left out: 728 of the 734 checkable ones disagreed with main. **Checked against main.** I converted every recipe pipeline main's lists already hold the same way. 13,323 of 19,738 match one of their records field for field, including blend, masks and clamp. The rest are: - other states of the same draws, e.g. additive vs alpha blend; or - blend enables past the last target, which Lichii clears and Hookshot keys only on a GPU without dynamic blend state. There was no systematic difference in any field. **Files.** - `assets/pipeline_states/contrib/pmwork-personal-fritchii-recipe.bin.xz` and `.md`, written by `nx2/pipeline_states.py add`. The note says how the list was made. - `assets/pipeline_states/README.md`: one paragraph saying the lists named `pmwork-personal-fritchii-recipe*` are converted, not recorded. They hold the same records and nothing more. The converter is a small script (recipe + `hookshot_shaders.bin` + main's merged list → HKSTATE2). It isn't in this PR; happy to add it under `scripts/` if you want the list reproducible. **Tested.** - `nx2/pipeline_states.py check`: merged list 26,712 pipelines (main 25,251 + 1,461). - Tests: `test_pipeline_states`, `test_pipeline_state_sources`, `test_pipeline_state_counts`, `test_pipeline_state_portable`, `test_ship_state_list`. - `python -I -B tests/run_all.py` on main with both PRs (Windows): 342 passed, 2 failed, 30 skipped (Linux-only). The two (`test_precompile_compute_guard.py`, `test_recorder_sync.py`) fail the same on main without these changes on this PC. - In the game's preparation: a build of main with this list and the follow-up's (#2134) ran the Builder's shader preparation (RTX 5090, driver 621.14): `vk: state list: 38697 pipelines (38697 shipped, version 2), 14866 programs (1200 translated now, 0 with stages not in the seed), 953 variants by their textures (953 made), 4798 by nothing bound (4798 made), 38584 noted (2438 native mesh, 0 mesh fallback, 11985 by nothing bound), 113 whose program variant was not made, in 10.2 s` Every record of this list was noted. The 113 not made are records main's lists already hold that were noted without answers (1,285 such records there, none here). Credit: the recipe is Fritchii's (Frizti), built from its testers' sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
NotRexed#2133) Fritchii ships a recipe of the pipelines Ocarina of Time draws, and 1,461 of them are pipelines no list here holds that convert to whole HKSTATE2 records. This adds them as `contrib/pmwork-personal-fritchii-recipe`, as discussed in #dev-work. **What the recipe is.** Fritchii (a Lichii fork) ships `OoT v131072 (cache v54).nx2recipe`: the pipelines its testers' sessions drew plus the states it predicts. Each shader is named by a fingerprint, and the recipe holds no code (its format is in `nx2_recipe.h` / `nx2_shader_extract.h` on Fritchii's `oot-gather-fix` branch). **How it maps.** - **Shaders.** The fingerprint is SHA-256 of the stage's control section (first 0x48 bytes, +0x30..0x33 zeroed), then its code area. `code_key` hashes the same control and code bytes with FNV, so a player's `hookshot_shaders.bin` names the recipe's shaders by code key: 41,165 of 41,216. - **Render state.** It maps by meaning: - target and depth formats (Maxwell render-target formats to NVN's, one to one for every format OoT uses) - samples and topology class - blend factors and equations, into `nvn_blend_st` - channel mask, polygon mode, depth clamp and alpha to coverage **What it can't hold.** The recipe was recorded with dynamic vertex input, so it has no vertex formats or divisors. It has no texture answers either. So a pipeline is in this list only when: - main's lists fix both for its program (one vertex input for the vertex stage and one variant), or - it needs neither: mesh draws take no vertex input, and a program none of whose stages asks about a texture or image has variant 0. All 40 such programs in main's lists have variant 0 there. Point-topology states that the recipe predicts for programs main's lists draw as triangles are left out: 728 of the 734 checkable ones disagreed with main. **Checked against main.** I converted every recipe pipeline main's lists already hold the same way. 13,323 of 19,738 match one of their records field for field, including blend, masks and clamp. The rest are: - other states of the same draws, e.g. additive vs alpha blend; or - blend enables past the last target, which Lichii clears and Hookshot keys only on a GPU without dynamic blend state. There was no systematic difference in any field. **Files.** - `assets/pipeline_states/contrib/pmwork-personal-fritchii-recipe.bin.xz` and `.md`, written by `nx2/pipeline_states.py add`. The note says how the list was made. - `assets/pipeline_states/README.md`: one paragraph saying the lists named `pmwork-personal-fritchii-recipe*` are converted, not recorded. They hold the same records and nothing more. The converter is a small script (recipe + `hookshot_shaders.bin` + main's merged list → HKSTATE2). It isn't in this PR; happy to add it under `scripts/` if you want the list reproducible. **Tested.** - `nx2/pipeline_states.py check`: merged list 26,712 pipelines (main 25,251 + 1,461). - Tests: `test_pipeline_states`, `test_pipeline_state_sources`, `test_pipeline_state_counts`, `test_pipeline_state_portable`, `test_ship_state_list`. - `python -I -B tests/run_all.py` on main with both PRs (Windows): 342 passed, 2 failed, 30 skipped (Linux-only). The two (`test_precompile_compute_guard.py`, `test_recorder_sync.py`) fail the same on main without these changes on this PC. - In the game's preparation: a build of main with this list and the follow-up's (NotRexed#2134) ran the Builder's shader preparation (RTX 5090, driver 621.14): `vk: state list: 38697 pipelines (38697 shipped, version 2), 14866 programs (1200 translated now, 0 with stages not in the seed), 953 variants by their textures (953 made), 4798 by nothing bound (4798 made), 38584 noted (2438 native mesh, 0 mesh fallback, 11985 by nothing bound), 113 whose program variant was not made, in 10.2 s` Every record of this list was noted. The 113 not made are records main's lists already hold that were noted without answers (1,285 such records there, none here). Credit: the recipe is Fritchii's (Frizti), built from its testers' sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b8fd5fd)
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. Re-checked at 8f3910d.
Fixed:
- The re-translation on every prepare.
program_find_plainfinds the loaded plain variant by its answers (one per counted slot, eachcanonical_answerofdeclared_answer), andstate_list_prepareonly 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. Becauserecord_answersnow uses the samedeclared_answer, the two can't drift apart. It takesshader_muonce 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 34f0578 and #2143 answers my question.
test_seed_variant_cleanup.pycopiesseed_variant_madetoo, andtest_state_list_plain.pypins 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 @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 What it adds is pipelines. This head merged with today's main (so #2133's list is in),
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. |
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):
program_find_portable(key, 0)never found it, and each prepare translated the list's ~4.8k plain programs again and appended them tohookshot_programs.bin. Nowprogram_find_plain(key)finds the loaded variant by its answers (an answer for every slot, each its declared type; onedeclared_answerhelper shared withrecord_answers), andstate_list_preparequeues 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).test_state_list_plain.pychecks the lookup and both its uses;test_program_answer_domains.pycopiesdeclared_answerwithrecord_answers; main's newtest_seed_variant_cleanup.py(perf(prepare): build fewer predicted pipelines, size the pools to the usable CPUs, and keep translated programs across cache-code changes #2323) copiesseed_variant_madewithseed_variant_of.tests/run_all.pyon Windows after merging main at 34f0578: 379 passed, 31 skipped, 2 failed, both failing on untouched main here too (test_recorder_sync,test_vram_budget_row). A Windows build compiles.