Repository navigation
fix(agc): let a kept descriptor set's table record hold its textures weakly - #7802
Merged
oneandonlydean merged 2 commits intoOct 19, 2026
Merged
Conversation
…weakly A freed dedicated set kept for reuse recorded every texture of the resources that freed it and kept them alive, so the kept handles could not be reused while the set referred to them. Up to eight kept sets held textures the caches had already evicted: their memory stayed in use outside the caches' budgets, and a title with large image tables ran out of device memory. The record now holds the textures weakly. A build compares the kept table's views only while every recorded texture is still alive, so a matching handle is still the same view; when one has died the table is written in full, as for any other change.
github-actions
Bot
added
the
unblocks title
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
Collaborator
|
Thanks, the cause and the fix both check out here. One ask: nothing in the PR fails on main yet. Both of these fit in tests/execution/ImageTableReuse.cpp:
|
… a kept table whose textures died is written again
Contributor
Author
|
Thanks. Both are in
On this card, step 2 also passes with the The runs are in the Tested section. |
12 tasks done
fogydes
approved these changes
Oct 19, 2026
fogydes
left a comment
fogydes
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the updated head addressing @oneandonlydean's feedback:
- Holding textures weakly in kept descriptor table records prevents stale descriptor sets from indefinitely locking evicted VRAM allocations during
ReleaseTextureCaches, directly resolving the device memory exhaustion observed in Marvel's Wolverine (Issue #7801). - The two requested regression steps are now present in
ImageTableReuse.cpp:- Confirming
SampledTextureMemory() == 0afterReleaseTextureCaches(~0ull)(which failed with 96,256 outliving bytes on main). - Verifying that expired weak texture references invalidate the kept table and force a table write.
- Confirming
- Zero comments added in code; Conventional Commits title; all CI checks pass.
Ready to merge.
AI-assisted: yes
Contributor
|
AMD and Intel Windows data: with this PR's test change, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
AI-assisted: yes (Claude Code) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Since #6454, a freed dedicated descriptor set kept for reuse (
APS5_KEPT_DESCRIPTOR_SETS, 8 by default) carries aTableDescriptorsrecord whosetexturesis every sampled texture of theShaderResourcesthat freed it:keptTablemovestexturesinto it.ReleaseTextureCachescounts an evicted sampled entry as freed only when it held the last reference, so the textures these records hold leave the cache but keep their memory. On Marvel's Wolverine, heap 0 overruns its budget within a minute, allocations move to system memory, andvkAllocateMemoryfails with -2 (bisect and logs in #7801).The record now holds
std::weak_ptr<Texture>. The build compares a kept table's views only while every recorded texture is still alive. Two live textures cannot share a view handle, so an equal handle is still the same view, which is what the strong references were there to guarantee. When a recorded texture has died, the table is written in full, as for any other change. The rest of the kept-set path is unchanged.ImageTableReuse.cppgains two steps after its existing ones, as asked in review. A fullReleaseTextureCaches(~std::uint64_t{0})must leaveSampledTextureMemory()at 0. Then a table is built twice, and the second build must keep its table write, so the set it leaves holds a record that matches while its textures live. A full release then destroys every texture (checked again throughSampledTextureMemory()), and the next build of the table must make one table write and keep none.Tested
NVIDIA GeForce RTX 4080 SUPER 16 GB (driver 617.42), Windows 11 Pro 26300, Ryzen 7 5800X3D, WinLibs GCC 15.2.0 r7, Release.
ctest -R '^agc_driver_(graphics|image_table_reuse)$'passes with this change onmainfb0dc11 and on 676d423 (both with feat(libs): restore libSceNpCppWebApi for dumps that ship no copy #6901 and fix(agc): run the unwatch hook when a watched import leaves the write watch #6308 merged, as for the game). These are the descriptor cache's kept-set unit test and the image table reuse execution test, run on the GPU.main6030b31 (this branch):agc_driver_image_table_reusepasses in 3 of 3 runs, and in 3 of 3 withAPS5_KEPT_DESCRIPTOR_SETS=0;agc_driver_graphicspasses. WithShaderResources.cppandShaderResources.hppreverted to 6030b31 and the new steps kept,agc_driver_image_table_reusefails in 3 of 3 runs: "96256 bytes of sampled textures outlived a full release of the texture caches after the kept-table steps".heldAlivecheck removed (the record still weak), the new steps pass in 3 of 3 runs on this card: the second build kept its write, and the build after the textures were destroyed made one table write and kept none, so it was written in full anyway. Without the check the step can fail only on a driver that gives every new view of the table the handle that the destroyed view in the same slot had; that did not happen here, so on this card the step does not catch a missing check.python tools/check_conventions.pypasses.vkAllocateMemory ... memoryType=0 ... initialResult=-2)APS5_KEPT_DESCRIPTOR_SETS=0Frame times on the card with this change are the same as with
APS5_KEPT_DESCRIPTOR_SETS=0and as before #6454: on 676d423, about 5.8 s per present once warm (31 presents in the 252 s after the first one, in the 300 s run).Not checked: Linux, lavapipe and other vendors; #6454's lavapipe timings with this change; how often the comparison now falls back to a full write (the kept and made counters are only exposed to tests).
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)