fix(vk): give back the import of a retired pool whose mirror was refused

github-sync commented

2026-11-12 14:41:08 +00:00

Owner

@SadTransGirl opened this pull request on GitHub · 2026-11-12 13:38 UTC · original
Merged on GitHub in c2e9ed5820

Players report shared GPU memory that only grows over a session. A retired pool's import is given back only when its mirror state is MIRROR_NONE, so a pool whose mirror was refused kept its import, pinned system memory the driver counts as the GPU's shared memory, for the rest of the session. On current main that is a pool over 1 GB, or one whose mirror's VRAM allocation failed (mirror_make); a pool turned away by the mirror budget becomes MIRROR_NONE and was already given back. So in normal play this frees fewer pools than the A/B below, which needed a test patch to make them.

Change. gpu/vk/vk_memory.cpp, retired_imports_give_back: a MIRROR_REFUSED pool's import goes as a MIRROR_NONE pool's does, after the same waits. imports_give_back already treats the two alike ((p.mstate == Pool::MIRROR_NONE || p.mstate == Pool::MIRROR_REFUSED) a few functions below). A refused pool has no mirror and is never inherit_pending, so nothing else reaches its import once the GPU is past retired_value and last_value, as for a MIRROR_NONE one.

Tested in the game. RTX 4070 SUPER 12 GB (617.42), Windows 11, DLAA 2560x1440, unattended. This card never has a mirror refused (no pool over 1 GB, no failed allocation), so both builds carried a test-only patch that makes every budget refusal MIRROR_REFUSED, run with RX_VK_MIRROR_MB=256 so that most pools were refused. The last "vk: imports:" line of each run:

	Main	This change
Castle Town to Hyrule Field	20 retired still imported (1,488 MB), 2 given back	0 still imported, 21 given back (1,472 MB)
Hyrule Field walk, 5 min	13 retired still imported (992 MB), 2 given back	0 still imported, 15 given back (1,040 MB)
Kokiri Forest	(not run)	0 still imported, 11 given back (512 MB)

No device lost and no error in any run. Row added to docs/tested-configurations.md.

Tests. tests/test_retired_give_back.py checks the condition; it fails on main and passes here.

With #2618 (open). #2618 turns a mirror that found no VRAM into MIRROR_NONE and asks again, so after it lands MIRROR_REFUSED is left for pools over 1 GB (and RX_VK_MIRROR_NO_RETRY=1): this still gives their imports back. The two touch different functions; they conflict only in docs/ rows, if at all.

Not tested. AMD and Intel on Windows, where retired_mode waits before giving retired imports back and where refused mirrors happen without a patch; a real card with VRAM full.

Part of #993.

🤖 Generated with Claude Code

> [@SadTransGirl](https://github.com/SadTransGirl) opened this pull request on GitHub · 2026-11-12 13:38 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2795) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/c2e9ed58205a0b18d823c9b47ec2115c838a7b61 Players report shared GPU memory that only grows over a session. A retired pool's import is given back only when its mirror state is `MIRROR_NONE`, so a pool whose mirror was refused kept its import, pinned system memory the driver counts as the GPU's shared memory, for the rest of the session. On current main that is a pool over 1 GB, or one whose mirror's VRAM allocation failed (`mirror_make`); a pool turned away by the mirror budget becomes `MIRROR_NONE` and was already given back. So in normal play this frees fewer pools than the A/B below, which needed a test patch to make them. **Change.** `gpu/vk/vk_memory.cpp`, `retired_imports_give_back`: a `MIRROR_REFUSED` pool's import goes as a `MIRROR_NONE` pool's does, after the same waits. `imports_give_back` already treats the two alike (`(p.mstate == Pool::MIRROR_NONE || p.mstate == Pool::MIRROR_REFUSED)` a few functions below). A refused pool has no mirror and is never `inherit_pending`, so nothing else reaches its import once the GPU is past `retired_value` and `last_value`, as for a `MIRROR_NONE` one. **Tested in the game.** RTX 4070 SUPER 12 GB (617.42), Windows 11, DLAA 2560x1440, unattended. This card never has a mirror refused (no pool over 1 GB, no failed allocation), so both builds carried a test-only patch that makes every budget refusal `MIRROR_REFUSED`, run with `RX_VK_MIRROR_MB=256` so that most pools were refused. The last "vk: imports:" line of each run: | | Main | This change | |---|---|---| | Castle Town to Hyrule Field | 20 retired still imported (1,488 MB), 2 given back | 0 still imported, 21 given back (1,472 MB) | | Hyrule Field walk, 5 min | 13 retired still imported (992 MB), 2 given back | 0 still imported, 15 given back (1,040 MB) | | Kokiri Forest | (not run) | 0 still imported, 11 given back (512 MB) | No device lost and no error in any run. Row added to `docs/tested-configurations.md`. **Tests.** `tests/test_retired_give_back.py` checks the condition; it fails on main and passes here. **With #2618 (open).** #2618 turns a mirror that found no VRAM into `MIRROR_NONE` and asks again, so after it lands `MIRROR_REFUSED` is left for pools over 1 GB (and `RX_VK_MIRROR_NO_RETRY=1`): this still gives their imports back. The two touch different functions; they conflict only in `docs/` rows, if at all. **Not tested.** AMD and Intel on Windows, where `retired_mode` waits before giving retired imports back and where refused mirrors happen without a patch; a real card with VRAM full. Part of #993. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-sync added 1 commit

2026-11-12 14:41:08 +00:00

fix(vk): give back the import of a retired pool whose mirror was refused 10721fd9a8

Players report shared GPU memory that only grows over a session. A retired
pool's import is given back only when its mirror state is MIRROR_NONE, so
a pool whose mirror was refused (over 1 GB, or no room in VRAM when it was
asked for: mirror_make) kept its import, pinned system memory the driver
counts as the GPU's shared memory, for the rest of the session. The more
such pools, the fuller VRAM was when they were refused.

A refused pool has no mirror either, so its import goes as a MIRROR_NONE
pool's does, after the same waits; imports_give_back already treats the
two alike. test_retired_give_back checks the condition.

Run in the game (RTX 4070 SUPER, Windows 11, DLAA 1440p). This card never
has a mirror refused, so both builds had a test patch making every budget
refusal MIRROR_REFUSED, with RX_VK_MIRROR_MB=256. The "vk: imports:" line:
main kept 20 retired pools imported (1,488 MB) after Castle Town to the
field and 13 (992 MB) in a field walk, giving back 2; with this change 0
were left in those and in Kokiri, 21 (1,472 MB), 15 (1,040 MB) and 11
(512 MB) given back. No device lost. docs/tested-configurations.md has
the row.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

github-sync referenced this pull request

2026-11-12 14:48:31 +00:00

perf(vk): sparse VRAM mirrors, memory only where a game pool is (RX_VK_MIRROR_SPARSE=1) #2794

github-sync commented

2026-11-12 14:48:33 +00:00

Author

Owner

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

I merged this into current main and ran test_retired_give_back, test_import_buffer_binding and test_import_write_watch. All pass, and CI is green. The change at vk_memory.cpp:3187 matches what imports_give_back already does for live pools: it treats MIRROR_NONE and MIRROR_REFUSED alike. A refused pool never has a mirror, so it can never be inherit_pending, and the existing retired_value and last_value checks against mem_completed still guard the GPU side. I don't see a path that touches a refused retired pool's import after those values pass.

One clarification for the description: on current main, a budget refusal in mirror_make sets MIRROR_NONE. Only a pool over 1 GB, or a failed create_device_buffer, becomes MIRROR_REFUSED. So in normal play this frees fewer pools than the patched A/B suggests, and that's why you needed the test patch. The fix is still right, and it's cheap. With #2618 it only conflicts in docs/development.md, and with #2794 only in docs/tested-configurations.md.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2795#pullrequestreview-5876258830) I merged this into current main and ran test_retired_give_back, test_import_buffer_binding and test_import_write_watch. All pass, and CI is green. The change at vk_memory.cpp:3187 matches what imports_give_back already does for live pools: it treats MIRROR_NONE and MIRROR_REFUSED alike. A refused pool never has a mirror, so it can never be inherit_pending, and the existing retired_value and last_value checks against mem_completed still guard the GPU side. I don't see a path that touches a refused retired pool's import after those values pass. One clarification for the description: on current main, a budget refusal in mirror_make sets MIRROR_NONE. Only a pool over 1 GB, or a failed create_device_buffer, becomes MIRROR_REFUSED. So in normal play this frees fewer pools than the patched A/B suggests, and that's why you needed the test patch. The fix is still right, and it's cheap. With #2618 it only conflicts in docs/development.md, and with #2794 only in docs/tested-configurations.md.

github-sync referenced this pull request from a commit

2026-11-12 16:54:32 +00:00

fix(vk): give back the import of a retired pool whose mirror was refused (#2795)

github-sync added 1 commit

2026-11-12 16:54:34 +00:00

Merge branch 'main' into fix/retired-refused-imports cc8b76b5cd

~~github-sync referenced this pull request 2026-11-12 16:54:48 +00:00~~

chore(main): release 2.2.0-rc.1 #2071

github-sync closed this pull request

2026-11-12 16:56:14 +00:00

github-sync commented

2026-11-12 16:56:15 +00:00

Author

Owner

@SadTransGirl commented on GitHub · 2026-11-12 16:07 UTC · original

Thanks, that's right: on current main a budget refusal is MIRROR_NONE, and only a pool over 1 GB or a failed create_device_buffer is MIRROR_REFUSED. I've corrected the description to say so, and that in normal play it frees fewer pools than the patched A/B.

> [@SadTransGirl](https://github.com/SadTransGirl) commented on GitHub · 2026-11-12 16:07 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2795#issuecomment-6828875600) Thanks, that's right: on current main a budget refusal is `MIRROR_NONE`, and only a pool over 1 GB or a failed `create_device_buffer` is `MIRROR_REFUSED`. I've corrected the description to say so, and that in normal play it frees fewer pools than the patched A/B.

github-sync commented

2026-11-12 16:56:15 +00:00

Author

Owner

@hckrman101 approved on GitHub · 2026-11-12 16:32 UTC · original

Thanks, this looks right to me, and I agree with Joey-Magnus's review. Your corrected description (budget refusals are MIRROR_NONE on main, so this only reaches pools over 1 GB or a failed mirror allocation) answers his one point.

What I checked:

GPU still using it: a refused pool goes through the same gates as a MIRROR_NONE one: retired_value and last_value both at or below mem_completed, then the free itself waits in dead_imports for max(mem_rec_value, last_value). retired_value is set at retirement to at least the recording being made then, so a binding made in that recording is covered. That matters a bit more for refused pools than for MIRROR_NONE ones, since with mirrors on they are the ones gpu_buffer_bind hands out from the per-thread cache without the lock. But retire_pool bumps pools_gen, and the retired node is no longer in imported, so no new binding can reach it after that.
Freed twice: the loop skips nodes whose memory is already null, and the give-back nulls buffer/memory and moves pieces out. A retired node lives only in retired_pools, never in evicted_pools, so imports_give_back can't free it as well. A refused pool never gets a mirror again (gpu_buffer_bind doesn't queue it, and retire_pool takes pools out of mirror_wanted), and it is never inherit_pending, so mirrors_give_back and the inherit copy in mirror_make don't touch it either.
Evidence: the A/B is the same scenes, same card, before and after, with the counts from the "imports:" line. Using a test patch to force refusals is fine here, since an RTX 4070 SUPER doesn't hit the natural case. Real play will free fewer pools than this, as you say.

Not blocking: on Windows AMD and Intel, retired imports go back only after the wait, and those are the vendors where give-backs once came right before device losses (309, 571). This adds the same kind of give-back with the same wait, nothing new, but a log from an AMD or Intel card with full VRAM would be good to have once one turns up.

Ordering: this is independent of #2618 and of #2759. #2759 retires more pools through retire_pool, so the two go well together, and this one is small enough to go first. Others only clash on the new row at the top of docs/tested-configurations.md.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 16:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2795#pullrequestreview-5878203852) Thanks, this looks right to me, and I agree with Joey-Magnus's review. Your corrected description (budget refusals are `MIRROR_NONE` on main, so this only reaches pools over 1 GB or a failed mirror allocation) answers his one point. What I checked: - **GPU still using it:** a refused pool goes through the same gates as a `MIRROR_NONE` one: `retired_value` and `last_value` both at or below `mem_completed`, then the free itself waits in `dead_imports` for `max(mem_rec_value, last_value)`. `retired_value` is set at retirement to at least the recording being made then, so a binding made in that recording is covered. That matters a bit more for refused pools than for `MIRROR_NONE` ones, since with mirrors on they are the ones `gpu_buffer_bind` hands out from the per-thread cache without the lock. But `retire_pool` bumps `pools_gen`, and the retired node is no longer in `imported`, so no new binding can reach it after that. - **Freed twice:** the loop skips nodes whose `memory` is already null, and the give-back nulls `buffer`/`memory` and moves `pieces` out. A retired node lives only in `retired_pools`, never in `evicted_pools`, so `imports_give_back` can't free it as well. A refused pool never gets a mirror again (`gpu_buffer_bind` doesn't queue it, and `retire_pool` takes pools out of `mirror_wanted`), and it is never `inherit_pending`, so `mirrors_give_back` and the inherit copy in `mirror_make` don't touch it either. - **Evidence:** the A/B is the same scenes, same card, before and after, with the counts from the "imports:" line. Using a test patch to force refusals is fine here, since an RTX 4070 SUPER doesn't hit the natural case. Real play will free fewer pools than this, as you say. Not blocking: on Windows AMD and Intel, retired imports go back only after the wait, and those are the vendors where give-backs once came right before device losses (309, 571). This adds the same kind of give-back with the same wait, nothing new, but a log from an AMD or Intel card with full VRAM would be good to have once one turns up. Ordering: this is independent of #2618 and of #2759. #2759 retires more pools through `retire_pool`, so the two go well together, and this one is small enough to go first. Others only clash on the new row at the top of `docs/tested-configurations.md`.

github-sync referenced this pull request

2026-11-12 16:56:29 +00:00

perf(vk): an import mostly outside every game pool is trimmed back to its live parts #2825

github-sync referenced this pull request

2026-11-12 17:59:36 +00:00

fix(host): keep address regions reserved until discard completes #2823

github-sync referenced this pull request

2026-11-12 17:59:48 +00:00

perf(vk): cap the system-heap budget for idle live imports on Linux #2839

github-sync referenced this pull request

2026-11-12 17:59:48 +00:00

perf(vk): cap the system-heap budget for idle live imports on Linux #2839

Rows
Columns

fix(vk): give back the import of a retired pool whose mirror was refused #2795