perf(vk): texture_answer asks a flat table by slot, not a hash map #2439

Merged

lucasmazzoni45-del merged 1 commit from perf-texture-answer-flat into main

2026-11-11 20:16:37 +00:00

lucasmazzoni45-del commented

2026-11-11 19:16:00 +00:00

(Migrated from github.com)

Every draw's variant check calls texture_answer() for each texture it samples. The thread-local unordered_map behind it was ~16% of the back end's time on the replay thread in a profile of an RX 7800 XT run. Slots are the texture pool's indices (~33k in OoT), so a vector indexed by slot (8 bytes a slot, capped at 2^20 entries) replaces it. The version check is unchanged, so the answers are the same; only the lookup changes. No behaviour switch.

Tested: the Vulkan back end builds (llvm-mingw, RX_VK_ONLY) on top of v2.1.0, and a full build of that tree boots to the title at 60 fps on an RX 7800 XT / Ryzen 7 5800X3D (Windows 11, driver 0x8001a0) with the new log lines present. The repository test suite was not run, and I have not yet played a long session on this build, so the effect in real play is not shown. Related data: #995.

🤖 Generated with Claude Code

Every draw's variant check calls `texture_answer()` for each texture it samples. The thread-local `unordered_map` behind it was ~16% of the back end's time on the replay thread in a profile of an RX 7800 XT run. Slots are the texture pool's indices (~33k in OoT), so a vector indexed by slot (8 bytes a slot, capped at 2^20 entries) replaces it. The version check is unchanged, so the answers are the same; only the lookup changes. No behaviour switch. **Tested:** the Vulkan back end builds (llvm-mingw, `RX_VK_ONLY`) on top of v2.1.0, and a full build of that tree boots to the title at 60 fps on an RX 7800 XT / Ryzen 7 5800X3D (Windows 11, driver 0x8001a0) with the new log lines present. The repository test suite was **not** run, and I have not yet played a long session on this build, so the effect in real play is not shown. Related data: #995. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Joey-Magnus (Migrated from github.com) approved these changes

2026-11-11 20:08:50 +00:00

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

Looks good to me. The table is indexed by handle & 0xFFFFF, so id + 1 never exceeds the 1u << 20 cap, and the set flag keeps a zero-initialised entry from passing for a cached answer when a slot's version is 0 (an unregistered slot, or an id outside kSlotVersions, where texture_slot_version returns 0). The version check is the same as before, so a re-registered slot still gets a fresh answer. <vector> and <algorithm> are already included, and no check in tests/ cuts texture_answer out of vk_shader.cpp (test_program_answer_domains.py only stubs it), so nothing there needs updating.

A few small things, none blocking:

minor gpu/vk/vk_shader.cpp:731: the handle comes from a constant-bank word, so a garbage handle with a high slot grows that thread's table straight to the cap: 2^20 × 8 bytes = 8 MB, zero-filled and committed, on every thread that calls this (the replay thread, plus whatever runs record_answers). The old map's cost grew with the number of distinct ids, not with the largest one. In practice slots stay around 33k (about 264 KB), so this is fine. If you want to rule out the worst case, you could size to the texture pool's real slot count instead of 2^20, or skip caching ids above it.
question The description says the test run on the RX 7800 XT showed "the new log lines", but this diff adds no logging. Was that from a different branch, maybe the one behind #995? It would also help to say how you measured the 16% (the profiler and the area of the game), so the number can be checked again later.
CI's test workflows haven't run on this PR yet (only the title check has), probably because the contributor's workflows are waiting for approval. It's worth letting them run before merging, since the description says the suite wasn't run locally.

Looks good to me. The table is indexed by `handle & 0xFFFFF`, so `id + 1` never exceeds the `1u << 20` cap, and the `set` flag keeps a zero-initialised entry from passing for a cached answer when a slot's version is 0 (an unregistered slot, or an id outside `kSlotVersions`, where `texture_slot_version` returns 0). The version check is the same as before, so a re-registered slot still gets a fresh answer. `<vector>` and `<algorithm>` are already included, and no check in `tests/` cuts `texture_answer` out of `vk_shader.cpp` (`test_program_answer_domains.py` only stubs it), so nothing there needs updating. A few small things, none blocking: - **minor** `gpu/vk/vk_shader.cpp:731`: the handle comes from a constant-bank word, so a garbage handle with a high slot grows that thread's table straight to the cap: 2^20 × 8 bytes = 8 MB, zero-filled and committed, on every thread that calls this (the replay thread, plus whatever runs `record_answers`). The old map's cost grew with the number of distinct ids, not with the largest one. In practice slots stay around 33k (about 264 KB), so this is fine. If you want to rule out the worst case, you could size to the texture pool's real slot count instead of 2^20, or skip caching ids above it. - **question** The description says the test run on the RX 7800 XT showed "the new log lines", but this diff adds no logging. Was that from a different branch, maybe the one behind #995? It would also help to say how you measured the 16% (the profiler and the area of the game), so the number can be checked again later. - CI's test workflows haven't run on this PR yet (only the title check has), probably because the contributor's workflows are waiting for approval. It's worth letting them run before merging, since the description says the suite wasn't run locally.