MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

vk: retire finalized pools without inheritance, remove 1.4 GB VMA prealloc, and trim stale render targets #2759

Open
github-sync wants to merge 1 commit from github/pr/2759 into main
pull from: github/pr/2759
merge into: MagicBeanDev:main
Conversation 6 Commits 1 Files changed 6 +153 -12
github-sync commented 2026-11-12 11:35:46 +00:00
Owner

@Hinkiii opened this pull request on GitHub · 2026-11-12 11:21 UTC · original

On 8 GB GPUs, Hyrule Field exceeds the Vulkan driver's VRAM budget (7,112 MB used of 6,614 MB allowed on RTX 4060 Ti 8 GB) while host memory imports and VMA system heap blocks accumulate across scene loads. This PR removes the eager 1,408 MB VMA pool pre-allocation at startup, immediately retires pools finalized via nvnMemoryPoolFinalize without inheriting stale textures into reused VA ranges, and reclaims idle render-target and finalized textures so 8 GB cards stay well below their VRAM budget (6,190–6,456 MB of 6,954 MB allowed) and release -5.9 GB of pinned host pool imports.

What changed

  1. Retire nvnMemoryPoolFinalized pools without texture inheritance (gpu/vk/vk_memory.cpp):

    • Previously, memory_pool_finalize() only marked FinalizeEntry::dead = true without retiring the matching PoolEntry, leaving finalized pools in pools[] until an overlapping pool was registered later and then calling retire_pool(..., inherit=true) to transfer dead textures into the new pool.
    • Added retire_finalized_pools() (called at the start of memory_begin_submission() and memory_pool_register()) which calls retire_pool(idx, false) (inherit = false) for every finalized pool. When the guest explicitly finalizes an NVNmemoryPool, its TextureEntry objects have pool_dead = true set and its host memory import is queued for retired_pools cleanup as soon as in-flight GPU work completes, rather than surviving indefinitely via VA-overlap inheritance.

  2. Avoid 1.4 GB upfront VMA block pre-allocation (gpu/vk/vk_vma.cpp):

    • vma_init() was setting minBlockCount = 9 (1,152 MB) on image_pool and minBlockCount = 2 (256 MB) on target_image_pool whenever split == true (!unified_memory), unconditionally pinning 1,408 MB of VRAM at boot even before any textures were created.
    • Changed minBlockCount to 0 on both pools while keeping 128 MB block sizes so VMA allocates blocks on demand and frees empty blocks back to the driver.

  3. Promptly evict finalized-pool textures & idle render targets (gpu/vk/vk_texture.cpp, gpu/vk/vk_texture.h):

    • In texture_TrimStale(), textures belonging to finalized pools (t->pool_dead) now release their VkImage as soon as their last submission timeline completes (last_val <= completed), even when overall VRAM is below the trim threshold.
    • Render targets (is_target) that have been unused for >= 120 completed submissions (~2 seconds) are also trimmed during pressure passes and set t->released = true, allowing target_image_pool blocks from prior scenes/passes to compact and return to VMA.

Measured before / after (RTX 4060 Ti 8 GB, Driver 617.42, Windows 11 Pro 26200, Ryzen 7 3700X, 32 GB RAM)

Measured in-game across Title Screen, Kokiri Forest, and Hyrule Field (45,600 presents):

Metric (hookshot_log.txt) Before (main) After (this PR) Delta
Title / Early VRAM used 2,681 MB (image_pool: 9 blocks / 1,152 MB for 350 MB used) 1,677 MB (image_pool: 1 block / 128 MB for 119 MB used) -1,004 MB VRAM
Hyrule Field Peak VRAM 7,112 MB used of 6,614 MB allowed (+498 MB over budget) 6,456 MB peak / 6,190–6,286 MB steady of 6,954 MB allowed -656 MB to -922 MB VRAM (stays 500–750 MB under budget)
target_image_pool VMA blocks 13 blocks 1,664 MB (653 MB used, 1,011 MB slack) 5 blocks 640 MB (446 MB used, 194 MB slack) -1,024 MB VMA blocks (-817 MB slack)
One-frame finalized textures kept 520 MB (285 textures) 21 MB (97 textures) -499 MB
VMA heap 1 (system) 32 blocks 1,232 MB (42 allocs 1,062 MB) 11 blocks 288 MB (38 allocs 224 MB) -944 MB blocks / -838 MB allocs
Driver system heap used 10,793 MB of 15,571 MB 3,252 MB of 15,571 MB -7,541 MB system GPU heap
Host pool imports (vk: imports:) 257 live (8,720 MB), 16 retired still imported (560 MB) 74 live (2,816 MB), 0 retired still imported (0 MB), 24 given back (1,136 MB) -5,904 MB live host imports
Process Private Commit Climbed to 33,860 MB Plateaued at 30,590–30,901 MB across 45,600 frames ~3.0–3.3 GB lower, flat plateau

Testing

  • Verified in-game across Title, Kokiri Forest, and Hyrule Field (45,600 frames, 57.0 fps at end of session, 0 device lost) and added row to docs/tested-configurations.md.
  • Added game-free check tests/test_finalized_pools_and_vma_prealloc.py and ran python -I -B tests/run_all.py target_image_pool retired_give_back pool_span vram_pressure finalized_pools utf8_paths (all 6 passed).
> [@Hinkiii](https://github.com/Hinkiii) opened this pull request on GitHub · 2026-11-12 11:21 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759) On 8 GB GPUs, Hyrule Field exceeds the Vulkan driver's VRAM budget (`7,112 MB` used of `6,614 MB` allowed on RTX 4060 Ti 8 GB) while host memory imports and VMA system heap blocks accumulate across scene loads. This PR removes the eager 1,408 MB VMA pool pre-allocation at startup, immediately retires pools finalized via `nvnMemoryPoolFinalize` without inheriting stale textures into reused VA ranges, and reclaims idle render-target and finalized textures so 8 GB cards stay well below their VRAM budget (`6,190–6,456 MB` of `6,954 MB` allowed) and release `-5.9 GB` of pinned host pool imports. ### What changed 1. **Retire `nvnMemoryPoolFinalize`d pools without texture inheritance (`gpu/vk/vk_memory.cpp`)**: - Previously, `memory_pool_finalize()` only marked `FinalizeEntry::dead = true` without retiring the matching `PoolEntry`, leaving finalized pools in `pools[]` until an overlapping pool was registered later and then calling `retire_pool(..., inherit=true)` to transfer dead textures into the new pool. - Added `retire_finalized_pools()` (called at the start of `memory_begin_submission()` and `memory_pool_register()`) which calls `retire_pool(idx, false)` (`inherit = false`) for every finalized pool. When the guest explicitly finalizes an `NVNmemoryPool`, its `TextureEntry` objects have `pool_dead = true` set and its host memory import is queued for `retired_pools` cleanup as soon as in-flight GPU work completes, rather than surviving indefinitely via VA-overlap inheritance. 2. **Avoid 1.4 GB upfront VMA block pre-allocation (`gpu/vk/vk_vma.cpp`)**: - `vma_init()` was setting `minBlockCount = 9` (`1,152 MB`) on `image_pool` and `minBlockCount = 2` (`256 MB`) on `target_image_pool` whenever `split == true` (`!unified_memory`), unconditionally pinning `1,408 MB` of VRAM at boot even before any textures were created. - Changed `minBlockCount` to `0` on both pools while keeping `128 MB` block sizes so VMA allocates blocks on demand and frees empty blocks back to the driver. 3. **Promptly evict finalized-pool textures & idle render targets (`gpu/vk/vk_texture.cpp`, `gpu/vk/vk_texture.h`)**: - In `texture_TrimStale()`, textures belonging to finalized pools (`t->pool_dead`) now release their `VkImage` as soon as their last submission timeline completes (`last_val <= completed`), even when overall VRAM is below the trim threshold. - Render targets (`is_target`) that have been unused for `>= 120` completed submissions (~2 seconds) are also trimmed during pressure passes and set `t->released = true`, allowing `target_image_pool` blocks from prior scenes/passes to compact and return to VMA. ### Measured before / after (RTX 4060 Ti 8 GB, Driver 617.42, Windows 11 Pro 26200, Ryzen 7 3700X, 32 GB RAM) Measured in-game across Title Screen, Kokiri Forest, and Hyrule Field (45,600 presents): | Metric (`hookshot_log.txt`) | Before (`main`) | After (this PR) | Delta | | :--- | :--- | :--- | :--- | | **Title / Early VRAM used** | `2,681 MB` (`image_pool`: 9 blocks / `1,152 MB` for `350 MB` used) | **`1,677 MB`** (`image_pool`: 1 block / `128 MB` for `119 MB` used) | **-1,004 MB VRAM** | | **Hyrule Field Peak VRAM** | `7,112 MB` used of `6,614 MB` allowed (**+498 MB over budget**) | **`6,456 MB`** peak / **`6,190–6,286 MB`** steady of `6,954 MB` allowed | **-656 MB to -922 MB VRAM** (stays 500–750 MB under budget) | | **`target_image_pool` VMA blocks** | `13 blocks 1,664 MB` (`653 MB` used, `1,011 MB` slack) | **`5 blocks 640 MB`** (`446 MB` used, `194 MB` slack) | **-1,024 MB** VMA blocks (**-817 MB** slack) | | **One-frame finalized textures kept** | `520 MB` (`285` textures) | **`21 MB`** (`97` textures) | **-499 MB** | | **VMA `heap 1 (system)`** | `32 blocks 1,232 MB` (`42` allocs `1,062 MB`) | **`11 blocks 288 MB`** (`38` allocs `224 MB`) | **-944 MB** blocks / **-838 MB** allocs | | **Driver system heap used** | `10,793 MB` of `15,571 MB` | **`3,252 MB`** of `15,571 MB` | **-7,541 MB** system GPU heap | | **Host pool imports (`vk: imports:`)** | `257 live (8,720 MB)`, `16 retired still imported (560 MB)` | **`74 live (2,816 MB)`**, `0 retired still imported (0 MB)`, `24 given back (1,136 MB)` | **-5,904 MB** live host imports | | **Process Private Commit** | Climbed to `33,860 MB` | Plateaued at **`30,590–30,901 MB`** across 45,600 frames | **~3.0–3.3 GB lower**, flat plateau | ### Testing - Verified in-game across Title, Kokiri Forest, and Hyrule Field (45,600 frames, `57.0 fps` at end of session, `0` device lost) and added row to `docs/tested-configurations.md`. - Added game-free check `tests/test_finalized_pools_and_vma_prealloc.py` and ran `python -I -B tests/run_all.py target_image_pool retired_give_back pool_span vram_pressure finalized_pools utf8_paths` (all 6 passed). <!-- github-sync:item:2759 -->
github-sync commented 2026-11-12 13:40:35 +00:00
Author
Owner

@Brett-D commented on GitHub · 2026-11-12 13:03 UTC · original

Tested on RTX 5090 (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) against main (c544828f), main + RX_VK_IMAGE_PREALLOC=0 (the zero-preallocation bound), and #2711.

Setup & method: origin/main (c544828f) built with RX_RELEASE=1 (17,888 programs, 25,303 graphics and 799 compute pipelines precompiled at startup), 2048x1152 uncapped (RX_VK_FRAMEGEN=0, HOS_FPS_MAX=240), Kokiri Forest autosave (80 s run: 40 s settle + 30 s measurement window).

Arm (RX_RELEASE=1, 25,303 pipelines) Peak Working Set Peak Private Bytes Δ Private vs main VMA Heap 0 Blocks VMA Heap 1 (System) image_pool / target / texture blocks Free in Heap 0 blocks Uncapped FPS (p95 / p99 ms) Frames > 50 ms
main control 17,458 MB 33,818 MB baseline 142 (9,357 MB) 13 (544 MB) 24 (3072 MB) / 6 (768 MB) / 18 (2304 MB) 3,484 MB 67.2 (17.07 / 19.61) 0
main + RX_VK_IMAGE_PREALLOC=0 (bound) 17,411 MB 29,053 MB (main2) -3,087 MB 118 (6,477 MB) 13 (512 MB) 1 (128 MB) / 7 (896 MB) / 18 (2304 MB) 680 MB 63.8 (18.06 / 19.40) 0
#2711 (hckrman101) 17,455 MB 31,601 MB -2,217 MB (-2.17 GB) 123 (7,021 MB) 12 (544 MB) 1 (128 MB) / 5 (640 MB) / 24 (3072 MB) 1,157 MB 68.2 (16.79 / 20.25) 0
#2759 (Hinkiii) 17,303 MB (-155 MB) 30,534 MB -3,284 MB (-3.21 GB) 115 (6,237 MB, -3,120 MB) 13 (352 MB, -192 MB) 1 (128 MB) / 6 (768 MB) / 18 (2304 MB) 544 MB 65.5 (17.38 / 19.98) 0

What #2759 recovers on a 24+ GB card:

  1. images_pool() pre-allocation (-2,944 MB / 100% of the stranded blocks): Setting minBlockCount = 0 when RX_VK_COMPACT or RX_VK_TARGET_BLOCKS is active drops image_pool from 24 blocks (3,072 MB) to 1 block (128 MB, 8 images 33 MB) while textures_pool allocates only the 18 blocks (2,304 MB) actually used in Kokiri Forest — recovering all 23 stranded blocks (2,944 MB / 2.88 GB) (100% of the RX_VK_IMAGE_PREALLOC=0 bound, compared to 17 blocks = 2,176 MB with #2711, which keeps minBlockCount = 24 on textures_pool).
  2. Capture buffer spares & finalized pools (-192 MB VMA system heap, -176 MB extra VMA device blocks): Capping idle capture buffer spares (25 kept (200 MB), 9 spare buffers) drops VMA system heap (heap 1) from 544 MB to 352 MB (-192 MB) and Peak Working Set by -155 MB, with 0 frames over 50 ms during the first 30 s in the field.
> [@Brett-D](https://github.com/Brett-D) commented on GitHub · 2026-11-12 13:03 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#issuecomment-6824372199) Tested on `RTX 5090` (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) against `main` (`c544828f`), `main + RX_VK_IMAGE_PREALLOC=0` (the zero-preallocation bound), and #2711. **Setup & method:** `origin/main` (`c544828f`) built with `RX_RELEASE=1` (17,888 programs, 25,303 graphics and 799 compute pipelines precompiled at startup), 2048x1152 uncapped (`RX_VK_FRAMEGEN=0`, `HOS_FPS_MAX=240`), Kokiri Forest autosave (80 s run: 40 s settle + 30 s measurement window). | Arm (`RX_RELEASE=1`, 25,303 pipelines) | Peak Working Set | Peak Private Bytes | Δ Private vs `main` | VMA Heap 0 Blocks | VMA Heap 1 (System) | `image_pool` / `target` / `texture` blocks | Free in Heap 0 blocks | Uncapped FPS (`p95` / `p99` ms) | Frames > 50 ms | |---|---|---|---|---|---|---|---|---|---| | `main` control | 17,458 MB | 33,818 MB | baseline | 142 (`9,357 MB`) | 13 (`544 MB`) | `24 (3072 MB)` / `6 (768 MB)` / `18 (2304 MB)` | 3,484 MB | 67.2 (`17.07` / `19.61`) | 0 | | `main` + `RX_VK_IMAGE_PREALLOC=0` (bound) | 17,411 MB | 29,053 MB (`main2`) | -3,087 MB | 118 (`6,477 MB`) | 13 (`512 MB`) | `1 (128 MB)` / `7 (896 MB)` / `18 (2304 MB)` | 680 MB | 63.8 (`18.06` / `19.40`) | 0 | | #2711 (`hckrman101`) | 17,455 MB | 31,601 MB | -2,217 MB (-2.17 GB) | 123 (`7,021 MB`) | 12 (`544 MB`) | `1 (128 MB)` / `5 (640 MB)` / `24 (3072 MB)` | 1,157 MB | 68.2 (`16.79` / `20.25`) | 0 | | **#2759 (`Hinkiii`)** | **17,303 MB (-155 MB)** | **30,534 MB** | **-3,284 MB (-3.21 GB)** | **115 (`6,237 MB`, -3,120 MB)** | **13 (`352 MB`, -192 MB)** | **`1 (128 MB)` / `6 (768 MB)` / `18 (2304 MB)`** | **544 MB** | **65.5 (`17.38` / `19.98`)** | **0** | **What #2759 recovers on a 24+ GB card:** 1. **`images_pool()` pre-allocation (`-2,944 MB` / `100%` of the stranded blocks):** Setting `minBlockCount = 0` when `RX_VK_COMPACT` or `RX_VK_TARGET_BLOCKS` is active drops `image_pool` from `24 blocks (3,072 MB)` to `1 block (128 MB, 8 images 33 MB)` while `textures_pool` allocates only the `18 blocks (2,304 MB)` actually used in Kokiri Forest — recovering all **23 stranded blocks (`2,944 MB` / `2.88 GB`)** (`100%` of the `RX_VK_IMAGE_PREALLOC=0` bound, compared to `17 blocks = 2,176 MB` with #2711, which keeps `minBlockCount = 24` on `textures_pool`). 2. **Capture buffer spares & finalized pools (`-192 MB` VMA system heap, `-176 MB` extra VMA device blocks):** Capping idle capture buffer spares (`25 kept (200 MB), 9 spare buffers`) drops VMA system heap (`heap 1`) from `544 MB` to `352 MB` (`-192 MB`) and Peak Working Set by `-155 MB`, with `0` frames over 50 ms during the first 30 s in the field. <!-- github-sync:c:6824372199 -->
github-sync commented 2026-11-12 14:45:30 +00:00
Author
Owner

@Joey-Magnus requested changes on GitHub · 2026-11-12 14:35 UTC · original

I merged this into current main. The related checks pass (39). The full run_all fails only test_lift_cache, which fails on main here too, plus a load timeout in test_recorder_sync. The memory numbers are useful, but:

  • The title fails the Conventional Commit check (fix(vk): ...).
  • gpu/vk/vk_vma.cpp:109 reads RX_VK_COMPACT and RX_VK_TARGET_BLOCKS with getenv/atoi instead of env_on. split is true unless both are 0, so this drops the prealloc for everyone, which RX_VK_IMAGE_PREALLOC=0 already does. It also overlaps #2711's images_pool() change.
  • The description doesn't match the diff. texture_TrimStale and target_image_pool are untouched, while the cap_spare_max default (unlimited to 128 MB), the smaller prewarm set and the idle cap_bufs retirement go undescribed.
  • retire_finalized_pools uses inherit=false. If a pool is finalized and remade larger at the same VA across a submission, the GPU-written mirror is lost, which is the foliage bug retire_pool's comment describes. retired_pools also never shrinks, so every finalized pool adds a node walked on each submission, and !retired_pools.empty() runs mirrors_give_back every submission.
> [@Joey-Magnus](https://github.com/Joey-Magnus) requested changes on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#pullrequestreview-5876248194) I merged this into current main. The related checks pass (39). The full run_all fails only test_lift_cache, which fails on main here too, plus a load timeout in test_recorder_sync. The memory numbers are useful, but: - The title fails the Conventional Commit check (`fix(vk): ...`). - gpu/vk/vk_vma.cpp:109 reads RX_VK_COMPACT and RX_VK_TARGET_BLOCKS with getenv/atoi instead of env_on. `split` is true unless both are 0, so this drops the prealloc for everyone, which RX_VK_IMAGE_PREALLOC=0 already does. It also overlaps #2711's images_pool() change. - The description doesn't match the diff. texture_TrimStale and target_image_pool are untouched, while the cap_spare_max default (unlimited to 128 MB), the smaller prewarm set and the idle cap_bufs retirement go undescribed. - retire_finalized_pools uses inherit=false. If a pool is finalized and remade larger at the same VA across a submission, the GPU-written mirror is lost, which is the foliage bug retire_pool's comment describes. retired_pools also never shrinks, so every finalized pool adds a node walked on each submission, and `!retired_pools.empty()` runs mirrors_give_back every submission. <!-- github-sync:r:5876248194 -->
github-sync commented 2026-11-12 16:56:00 +00:00
Author
Owner

@hckrman101 commented on gpu/vk/vk_memory.cpp line 2601 on GitHub · 2026-11-12 16:33 UTC · original

retire_pool(cur, true) would keep the foliage fix. The mirror waits for an overlapping new pool for at most kIdleEpochs submissions, and the import isn't given back while inherit_pending is set, so it stays bounded. Did you see a reason it has to be false?

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_memory.cpp` line 2601 on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#discussion_r4553281283) `retire_pool(cur, true)` would keep the foliage fix. The mirror waits for an overlapping new pool for at most `kIdleEpochs` submissions, and the import isn't given back while `inherit_pending` is set, so it stays bounded. Did you see a reason it has to be `false`? <!-- github-sync:rc:4553281283 -->
github-sync commented 2026-11-12 16:56:00 +00:00
Author
Owner

@hckrman101 commented on gpu/vk/vk_memory.cpp line 1035 on GitHub · 2026-11-12 16:33 UTC · original

The 128 MB class was prewarmed for the scene-load capture spikes on AMD (the comment above cap_prewarm_on). Without it, the first 128/192 MB capture of a load makes its buffer on the submitting thread again. The log line in cap_prewarm_on still says 32, 64, 128 MB.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_memory.cpp` line 1035 on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#discussion_r4553281295) The 128 MB class was prewarmed for the scene-load capture spikes on AMD (the comment above `cap_prewarm_on`). Without it, the first 128/192 MB capture of a load makes its buffer on the submitting thread again. The log line in `cap_prewarm_on` still says 32, 64, 128 MB. <!-- github-sync:rc:4553281295 -->
github-sync commented 2026-11-12 16:56:00 +00:00
Author
Owner

@hckrman101 commented on gpu/vk/vk_texture.cpp line 4724 on GitHub · 2026-11-12 16:33 UTC · original

This marks the textures released straight away, outside the hold the replay keeps for releases while a queue is stopped (held_tex_releases). If finalized-pool textures can still be read by stopped work, they'd need the same hold. Minor: env_on is read on every call here; a static const like the caller's would do.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_texture.cpp` line 4724 on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#discussion_r4553281305) This marks the textures released straight away, outside the hold the replay keeps for releases while a queue is stopped (`held_tex_releases`). If finalized-pool textures can still be read by stopped work, they'd need the same hold. Minor: `env_on` is read on every call here; a `static const` like the caller's would do. <!-- github-sync:rc:4553281305 -->
github-sync commented 2026-11-12 16:56:00 +00:00
Author
Owner

@hckrman101 reviewed on GitHub · 2026-11-12 16:33 UTC · original

Thanks for this, and for the numbers. Giving finalized pools' imports back is a real gap on main (they sit in imported until something overlaps them), and the host-import and system-heap drops in your table are the most interesting part of the PR. I agree with all four of @Joey-Magnus's points and won't repeat them; a few more below, then how I'd like to land it.

Scope: please split this. As maintainer, my position is:

  • The preallocation is settled in #2711, not here. #2711 moves the blocks made at the start into the pool the textures are made in. Testers there found it matches RX_VK_IMAGE_PREALLOC=0 on 8 GB setups (an RTX 3060 Ti 8 GB, and a 4070 SUPER under the 8 GB model). It also keeps what #2438 was for: the ~170 ms block allocations on AMD's Windows driver. Nobody has measured AMD on Windows either way since #2438, and that's the one case where removing the blocks outright could cost something. Brett-D's 5090 numbers show the price: #2711 holds about 1 GB more private memory than no preallocation on a 32 GB card, by design. If an AMD Windows run later shows no stalls without the blocks, we can drop them in a follow-up with that data. So please drop the vk_vma.cpp hunk. With it go the test's images_pool check and the prealloc part of the docs row.
  • Retiring finalized pools: its own PR, with the inheritance question below answered.
  • The capture-buffer changes: their own PR too, described as what they are. As Joey said, the description calls them render-target trimming, and texture_TrimStale isn't touched.

Retiring without inheritance. The retire is keyed on the guest pool table, which follows the game's current state, not the replay's. Three cases worry me:

  1. Finalized, then remade over the same range a submission or more later (a scene load). Here the old pool is retired without its mirror carried over, which is the foliage case in retire_pool's comment (Joey's point 4). Simply calling retire_pool(cur, true) here looks like it would keep that: inherit_pending already expires after kIdleEpochs (600 submissions), and retired_imports_give_back skips the import until it does. So the cost is a mirror held for ≤600 submissions, not a leak. Could you try that and see whether your memory numbers hold?
  2. Work still waiting at a queue's wait. The replay holds texture releases while a queue is stopped (held_tex_releases, from PR 430), because releasing them early let a pass read what wasn't there yet. texture_pool_finalized marks the pool's GPU-written textures released straight away, outside that hold. retire_pool also clears the range's page-table entries while that stopped work may still reach the pool when it runs. Can the game finalize a pool while work that uses it is still waiting in our replay? If it can, this needs the same hold.
  3. A pool missing from the table while it is still live. The host notes in hos_gpu_track (host/nvdrv.c) that a pool made over another's memory can lose its entry in the GPU mappings to the old pool's finalize. Those mappings are what hos_gpu_pools reads. On main the import just stays. With this PR, a live pool in that state is retired, its textures are marked released and its gpu_owned ranges are dropped.

Capture buffers. The prewarm set loses its 128 MB class (and 192 MB), and spares are capped at 128 MB. That prewarm is a stutter fix: per its comment, capture buffers made at scene loads cost 13-107 ms on AMD. Brett-D's 5090 run shows no spikes, but that's NVIDIA. The comments and the startup log (32, 64, 128 MB spares) no longer match either. This needs an AMD load run, or keep the 128 MB class.

Measurements (rule: before/after on the same scene, hardware named). The 4060 Ti numbers are useful, but:

  • Each arm is a single session, and the allowance differs between them (6,614 vs 6,954 MB). Something else was holding VRAM in one of them.
  • There's no fps "before" to set against the 57.0 after.
  • Nothing says whether pictures were checked across scene loads. For the retire, that matters most: Kokiri's foliage after entering and leaving, and textures after a few loads (Kokiri, Field, Castle Town and back).

Once the retire is its own PR, an alternated pair (main, then this, twice) on the same route, with a note that the foliage and the textures are right after loads, would be enough.

CI: the only failure is the Conventional Commit title check, which comes from this PR's title. Everything else passes.

I'm happy to see the finalized-pool retire land once it's split out and case 1 is handled (or shown not to happen).

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2759#pullrequestreview-5878205324) Thanks for this, and for the numbers. Giving finalized pools' imports back is a real gap on main (they sit in `imported` until something overlaps them), and the host-import and system-heap drops in your table are the most interesting part of the PR. I agree with all four of @Joey-Magnus's points and won't repeat them; a few more below, then how I'd like to land it. **Scope: please split this.** As maintainer, my position is: - **The preallocation is settled in #2711, not here.** #2711 moves the blocks made at the start into the pool the textures are made in. Testers there found it matches `RX_VK_IMAGE_PREALLOC=0` on 8 GB setups (an RTX 3060 Ti 8 GB, and a 4070 SUPER under the 8 GB model). It also keeps what #2438 was for: the ~170 ms block allocations on AMD's Windows driver. Nobody has measured AMD on Windows either way since #2438, and that's the one case where removing the blocks outright could cost something. Brett-D's 5090 numbers show the price: #2711 holds about 1 GB more private memory than no preallocation on a 32 GB card, by design. If an AMD Windows run later shows no stalls without the blocks, we can drop them in a follow-up with that data. So please drop the `vk_vma.cpp` hunk. With it go the test's `images_pool` check and the prealloc part of the docs row. - **Retiring finalized pools: its own PR**, with the inheritance question below answered. - **The capture-buffer changes: their own PR too**, described as what they are. As Joey said, the description calls them render-target trimming, and `texture_TrimStale` isn't touched. **Retiring without inheritance.** The retire is keyed on the guest pool table, which follows the game's current state, not the replay's. Three cases worry me: 1. *Finalized, then remade over the same range a submission or more later* (a scene load). Here the old pool is retired without its mirror carried over, which is the foliage case in `retire_pool`'s comment (Joey's point 4). Simply calling `retire_pool(cur, true)` here looks like it would keep that: `inherit_pending` already expires after `kIdleEpochs` (600 submissions), and `retired_imports_give_back` skips the import until it does. So the cost is a mirror held for ≤600 submissions, not a leak. Could you try that and see whether your memory numbers hold? 2. *Work still waiting at a queue's wait.* The replay holds texture releases while a queue is stopped (`held_tex_releases`, from PR 430), because releasing them early let a pass read what wasn't there yet. `texture_pool_finalized` marks the pool's GPU-written textures released straight away, outside that hold. `retire_pool` also clears the range's page-table entries while that stopped work may still reach the pool when it runs. Can the game finalize a pool while work that uses it is still waiting in our replay? If it can, this needs the same hold. 3. *A pool missing from the table while it is still live.* The host notes in `hos_gpu_track` (`host/nvdrv.c`) that a pool made over another's memory can lose its entry in the GPU mappings to the old pool's finalize. Those mappings are what `hos_gpu_pools` reads. On main the import just stays. With this PR, a live pool in that state is retired, its textures are marked released and its `gpu_owned` ranges are dropped. **Capture buffers.** The prewarm set loses its 128 MB class (and 192 MB), and spares are capped at 128 MB. That prewarm is a stutter fix: per its comment, capture buffers made at scene loads cost 13-107 ms on AMD. Brett-D's 5090 run shows no spikes, but that's NVIDIA. The comments and the startup log (`32, 64, 128 MB spares`) no longer match either. This needs an AMD load run, or keep the 128 MB class. **Measurements (rule: before/after on the same scene, hardware named).** The 4060 Ti numbers are useful, but: - Each arm is a single session, and the allowance differs between them (6,614 vs 6,954 MB). Something else was holding VRAM in one of them. - There's no fps "before" to set against the 57.0 after. - Nothing says whether pictures were checked across scene loads. For the retire, that matters most: Kokiri's foliage after entering and leaving, and textures after a few loads (Kokiri, Field, Castle Town and back). Once the retire is its own PR, an alternated pair (main, then this, twice) on the same route, with a note that the foliage and the textures are right after loads, would be enough. CI: the only failure is the Conventional Commit title check, which comes from this PR's title. Everything else passes. I'm happy to see the finalized-pool retire land once it's split out and case 1 is handled (or shown not to happen). <!-- github-sync:r:5878205324 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Hookshot!2759
No description provided.