Repository navigation
feat(vk): say why each texture is evicted, and which GPU-drawn ones go with no copy of their texels - #1879
Conversation
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a useful line to have for the garbling reports. I read the retire pass end to end and the logic holds up: classify matches how the loop picks textures (shifted implies overwritten, so DEAD first is right), and texels_lost is exactly the case the flush was skipped or failed for a GPU-drawn texture nothing drew over. Checking flushed_ok rather than trusting e.flush is the right call, since flush_textures can fail per texture. n and tally.total() should always agree too, since nothing else takes textures out of textures between building order and the gone loop.
Things to sort out:
- Conflicts with main. GitHub shows the PR as not mergeable. Main added
#include "vram_compact.h"on the same line you putevict_reason.h, andtexture_framehas picked up the RX_VK_COMPACT changes (evict_to,compact.in_pass) since your base. Those don't touch the lines you changed beyond the include, so the rebase should be mechanical, but it needs doing. - CI.
test_evict_reason.pyandtest_keep_rendered.pypass on both runners. The red in Python source checks istest_antilag,nx2/test_draw_parameters(Ctx::mesh_fallback),test_wayland_libdecor_hintandnx2/test_nup_out_of_space. None of those touch this code, so I don't think it's the PR, but a rebase onto current main should clear or at least change that set.
Optional: the summary lumps finalized and live textures into "GPU-drawn with no copy kept". Both matter (the keep_finalized path is there because finalized terrain normal maps came back empty), but for the question this is meant to answer, splitting it as ... no copy kept: 2 live, 5 finalized, 1 MB would save reading the per-texture lines, especially once the first 100 are used up.
Nit inline. It still needs an in-game run, as you say. Once it's rebased this looks good to me.
| logf("vk: texture #%u %#llx %ux%ux%u x%u NVN %#x the GPU drew goes with no copy of its texels (%s, unused for %llu " | ||
| "frames, %s): made again over this storage, it starts empty", tex->id, (unsigned long long)tex->gpu, tex->width, | ||
| tex->height, tex->depth, tex->layers, tex->nvn_format, tex->released ? "finalized" : "live", | ||
| (unsigned long long)(frame_no - tex->last_frame), sweep ? "the sweep" : "over the budget"); |
There was a problem hiding this comment.
Nit: sweep is always false here. In a sweep, if (sweep && !overwritten) continue; above means only overwritten textures get evicted, and texels_lost needs !overwritten, so this line only ever prints "over the budget". Harmless, but the ternary suggests the sweep can lose texels when it can't. Dropping it, or noting it can't happen, would be clearer.
There was a problem hiding this comment.
Dropped in ca70392: the line always says "over the budget", with a note that a sweep lets only drawn-over textures go, and those lose nothing.
|
Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks! |
…rawn ones left with no copy of their texels Ground and outdoor textures garble after a while of play on 16 GB cards until a restart. One 9070 XT log at 3.3 GB of its 6 GB budget retired 28,000 textures, another at the budget 18,500, and the log could not tell budget evictions from the render-target sweeps, nor whether a texture the GPU drew went with the only copy of its texels (one made again over that storage starts empty: issues 107 and 386). evict_reason.h counts each pass's textures by reason (budget, drawn over, dead targets) and the GPU-drawn ones with no write-back that nothing drew over; up to 100 of those get a line of their own. tests/test_evict_reason.py (no game, no GPU). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ith no copy kept - "GPU-drawn with no copy kept: 2 live, 5 finalized, 1 MB": the finalized ones are what keep_finalized is for, and the split saves reading the per-texture lines once the first 100 are used up. - The per-texture line always says "over the budget": a sweep lets only drawn-over textures go, and those lose nothing, so its "the sweep" could never be printed. Co-authored-by: Cursor <cursoragent@cursor.com>
9bc2232 to
ca70392
Compare
|
Rebased onto current The suggestion is in too (ca70392): the summary now reads
|
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for the rebase and the split. I compared the old and new patches file by file. The only changes are the ones you described, plus main's vram_compact.h include and env_on("RX_VK_EVICT_NO_FLUSH"), which are both kept.
- Rebase: fixed. GitHub shows it as mergeable.
vk_texture.cpphasn't changed on main since 2ed98c4 (the two commits after it are reactive-mask/DLSS work). - Live/finalized split: done.
lost_finalizedcountstex->released,describeprintsN live, M finalized, X MB, and the test checks the new string. 160 bytes is still plenty for the line, even with 10-digit counts. - The
sweep ?nit: fixed. The per-texture line now always says "over the budget" and has a comment saying why.
CI: test_evict_reason passes on both runners. The Windows reds aren't this PR's. test_native_astc_texture_count fails on main too (its fixture's scanf trips -Werror under the Windows CRT). test_recorder_sync ("thread oversize: 0 calls made directly") is a recorder-ring test that passed on main's run. This PR doesn't touch anything it reads, so it looks like a flake. A re-run should clear it.
Heads-up: #2198 edits the same struct Evict and evict.push_back lines, so whichever of the two lands second gets a small conflict there. #2368's kept shadow maps continue before the push, so they never reach the tally and the counts stay right.
It's log-only, so I'm fine with it going in before the in-game run. The run plus the tested-configurations.md row would still be good to have.
This adds diagnostics for the worklist row "ground and outdoor textures garble after a while of play, fixed by a restart" (16 GB AMD cards, plus an RTX 4080 report). It changes no behaviour, only log lines.
Why: the eviction pass logged only
N textures retired (left, MB), and the user logs we have can't answer the open question.Neither log says whether those were budget evictions or render-target sweeps. It also can't say whether a texture the GPU drew went without a write-back while nothing had drawn over it. A texture like that, made again over the same storage, starts empty, which was the ground streaks of issues 107 and 386.
What changes:
gpu/vk/evict_reason.h(new):classify(shifted, overwritten)sorts each evicted texture as budget, drawn over or a dead target.texels_lost(gpu_written, overwritten, flushed)flags the risky case. ATallycounts them anddescribe()formats the summary.vk_texture.cpp, the retire pass: it records which write-backs succeeded (flushed_ok), tallies each evicted texture, and appends the breakdown to the line:vk: 140 textures retired (1176 left, 5374 MB): budget 12, drawn over 3, dead targets 125; GPU-drawn with no copy kept: 1 live, 1 finalized, 1 MBThe first 100 risky evictions each get a line with size, NVN format, finalized or live, frames unused, and that the budget took them (a sweep lets only drawn-over textures go, which lose nothing).
Tested:
python3 tests/test_evict_reason.pypasses: classification, the lost-texels rule, the tally, the formatting, and a short buffer. It also checks that the call sites are still invk_texture.cpp.tests/test_keep_rendered.pystill passes.vk_texture.cppwith the back end's flags is clean.docs/tested-configurations.mdonce a run is in.🤖 Generated with Claude Code