Repository navigation
fix(agc): free retired depth surfaces nobody samples - #6465
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
81dddc8 to
0538d6a
Compare
|
Data from a second title, 007 First Light (PPSA11386 v01.018.000), on Linux 7.2.10, NVIDIA GeForce RTX 3080 10 GB, driver 615.78.08, GCC 16.2.1, Release with This title keeps creating depth surfaces in gameplay. A local counter in the run with this PR:
The list's length also costs time. Each sampled-texture lookup calls Per present, averaged over the same presents into gameplay:
AI-assisted: yes (Claude Code) |
|
Thanks @SleglJan, a second title, and one where the list cost shows up in frame time as well as memory, is very helpful. I added your results to the description. AI-assisted: yes |
0538d6a to
ab68352
Compare
A depth surface stayed in the list until its device went, even after another writer retired it. A title whose depth targets move or change extent (dynamic resolution, a transient heap) adds one surface per new target and never frees the old ones. Creating a surface now frees the retired single-sample ones whose sampled views no one else holds, through the recorder so recorded work that still uses them completes first. Framebuffers cached with a depth attachment are dropped after a release, since a freed view's handle can be reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ab68352 to
98600b4
Compare
|
@adriadam10 thanks for rebasing this onto The resolution: AI-assisted: yes (Claude Code) |
What
A depth surface stays in
DepthSurface.cpp's list until its device goes.retire()(another writer covered its memory, #454) only sets a flag, so a retired surface keeps its image and memory.DepthSurfaceViewmatches a target by address, stencil address, extent, format and sample count. A title whose depth targets move through a transient heap, or change extent under dynamic resolution, therefore adds a surface for every new target and never frees the old ones.Forspoken (PPSA03984) does this. With #5937, #5975 and #5982 (needed to get it in game), a throwaway print over a 300 s run showed 134 surfaces created: 67 at 1920x1080 and 37 at 1984x1116 (
D32_SFLOAT_S8_UINT), plus 2048², 1024² and 3072²D32_SFLOATshadow maps. 117 of them were retired by the end, still holding their memory. In another 300 s run, a throwawayvkAllocateMemorytally by call site put theDepthSurfaceconstructor's live allocations at 629 MiB at 70 s, 1,687 MiB at 153 s and 2,665 MiB at 293 s. Between 153 s and 293 s, every other site stayed within 30 MiB. They count against no cache budget, so the sampled and storage caches shrink to make room until an allocation fails:textures) is held outside the surface. The surface goes toRecorder::Keep, so it is destroyed only after the recorded work that may still use its image completes. With no recorder there is no recorded work, so it is destroyed right away.DepthSurfacesReleased).Pipeline::AcquireFramebufferchecks only the color attachments' owners, so a cached framebuffer with a depth attachment could match a recycled view handle. Such entries are now dropped once a release happens after they were made. This only matters on the render-pass path (devices without graphics pipeline library, orAPS5_NO_GPL=1).owned, the depth guards) and are not freed here.DepthSurfaceReuse.cppgainsTestRetiredSurfaceRelease. Four retired surfaces at moving addresses must free their images when the next surface is created. A retired surface whose sampled view is held must keep its image until the view goes. A surface that was not retired must keep its image. Onmain'sDepthSurface.cpp(with a stub accessor) the test stops at "a retired depth surface must free its image once a new surface is created".Tested
Linux (Ubuntu 24.04, GCC 13.3, Release), RTX 4070 Ti, NVIDIA 595.99:
agc_driver_graphicspasses 3 of 3 runs. AllagcCTests pass exceptagc_pending_key_storesandagc_driver_store_over_untracked, which fail onmainhere too (perf(agc): clear a register fast clear without waiting for a known key store #6247, fix(agc): save pending storage results in every 64 KiB block that leaves the write watch #5671).main0c4816f with fix(agc): accept a copy's sample selection on a draw that copies nothing #5937, fix(agc): copy an indirect draw's vertex buffer only up to its last element #5975 and fix(agc): treat a view past a depth surface's first level as memory #5982:APS5_NO_GPL=1(render passes, so the framebuffer cache is used)GPU memory is
nvidia-smi's total, including about 2.1 GB the desktop used before the title started. An earlier run onmain8856f18 with the same three PRs stopped at 180 s onvkAllocateMemory render target: Vulkan result -2.main82ddae7 with fix(agc): drain library prebuilds before device replacement #6814 (on thatmainForspoken segfaults 1 s in, AGC: since #5875, a registration library prebuild racing the window attach segfaults in LinkPipelineFromLibraries (Stray PPSA02100, warm shader cache) #6805), fix(agc): accept a copy's sample selection on a draw that copies nothing #5937, fix(agc): copy an indirect draw's vertex buffer only up to its last element #5975, fix(agc): treat a view past a depth surface's first level as memory #5982 and this PR: a 600 s pad-input run reaches the limit with 1.34 million draws.main0029e62 with this PR: Borderlands 4, Ghost of Tsushima, Super Meat Boy 3D and Steins;Gate Re:Boot run to the limit. Metal Gear Solid 4, Astro Bot, Crimson Desert, MXGP 26, Call of Duty: Black Ops 2 and Forspoken (without the three PRs) stop with the same messages as onmain.On the Intel VM (Arrow Lake iGPU, Mesa 26.1.6 ANV, Debian 13), the
agc_driver_graphicsbinary from 0538d6a, which holds the newDepthSurfaceReusecases, passes 3 of 3.@SleglJan ran a second title, 007 First Light (PPSA11386), on an RTX 3080 (615.78.08, Linux), with this head on their bring-up stack. It keeps creating depth surfaces in gameplay, about 13 per present (2,199 by present 1350). With the PR, 10 to 23 are alive at a time. Without it, the depth-surface list keeps growing. Each sampled-texture lookup scans that list in
DepthSurfaceTexture, which was 17.8% of the title's samples in aperfprofile. Time per present grew from 8.3 s to 28.6 s, and the driver ran out of device-local memory 15 presents into gameplay, placing about 19 GB of storage textures in host memory over the run. With the PR, time per present stays at 5.5 to 6.3 s for the whole run, nothing is placed in host memory, and device-local use is 6.8 to 7.2 GB against 7.4 to 8.2 GB.Not checked on Windows, on AMD, or on a console.
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)#6215 changes the same part of
AcquireFramebuffer(for multisampled color targets). Whichever lands second needs a small rebase.🤖 Generated with Claude Code