Skip to content

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

Open
SadTransGirl wants to merge 1 commit into
NotRexed:mainfrom
SadTransGirl:fix/retired-refused-imports
Open

SadTransGirl wants to merge 1 commit into
NotRexed:mainfrom
SadTransGirl:fix/retired-refused-imports

Conversation

@SadTransGirl

Copy link
Copy Markdown
Contributor

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 of them, the fuller VRAM was.

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 allocation over budget), 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.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants