Repository navigation
Conversation
When transitioning between areas within < 60 frames, min_unused = 60 refused to evict any streamed textures even as total_bytes surged past 170% of budget, pushing total VRAM past 6-8 GB GPU allocations and triggering 60-70 ms PCIe paging stalls. Bypass the 10-frame eviction gate when cache exceeds 112.5% of budget, cap pool_cap to 128 MB and keep_finalized_max to 192 MB on <= 6.5 GB VRAM devices, and add a surge relief pass releasing non-GPU-written textures unused for >= 8 frames back down to budget. Co-Authored-By: erfboom <gabrielh.ltd@gmail.com>
Joey-Magnus
left a comment
There was a problem hiding this comment.
- The every-10-frames gate is bypassed whenever textures exceed 112.5% of
own(vk_texture.cpp:3875). If the excess can't be evicted (mostly GPU-written textures), the full budget pass runs every frame and never stops. - The relief pass (
:4262) evicts CPU-uploaded textures unused for only 8 frames and skips the compaction hold, so textures drawn every few frames can churn. - On 6.5 GB or smaller cards,
pool_capdrops from 256 to 128 MB (:290) andkeep_finalized_maxfrom 512 to 192 MB (:4131). The finalized keep exists for the rainbow/garbage-texture issues #107 and #386, so this risks regressing exactly those cards. test_target_image_pool.pywas loosened rather than updated.- The
RX_VK_LOG_PARTIALchange is unrelated. - This rewrites the same
frame_no % 10gate as #2484.
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for going after the scene-change hitches. I agree with all of Joey-Magnus's points above (the gate that can stay open every frame, the 8-frame relief pass, the smaller finalized keep on cards up to 6.5 GB and the risk to #107/#386, the loosened test, the unrelated logging change, and the clash with #2484). I won't go over them again. A few things to add:
- No numbers yet. The description talks about allocation at 170% of budget and 60-70 ms paging hitches, but gives no log, frame times or GPU. A before/after on a 6-8 GB card (the
vk: ... textures retiredlines plus frame times for the Kokiri -> Field -> Market run) would show whether the gate, the relief pass or the two caps actually fixes it. Then the risky parts could be dropped. This changes behaviour on cards people actually play on, so it needs an in-game report. - The relief pass works against the existing 60-frame rule. The comment above
min_unusedexplains why it is 60: with a shorter age, about 300 textures in a few minutes of Kokiri were evicted and made again, 200 of them within 10 frames. Evicting CPU-uploaded textures after 8 frames brings back that pattern of re-uploads and re-decodes, just limited to the surge. See inline. - An every-frame pass costs more than it looks. Each pass sorts every texture, calls
vram_usage, and does the write-back batches, each of which is a submission plus a wait. Running it every present during a surge can put those waits on consecutive frames, in the very window this PR is trying to smooth out. - Nit: the comment above
keep_finalized_maxstill says "512 by default". - CI: no checks have run on this head.
About your other open PRs: #2469 also edits vk_texture.cpp's staging and texture_frame code, but in a different hunk, so I expect no textual conflict. #2484 rewrites the same frame_no % 10 gate and the same line in test_target_image_pool.py, so one of the two will need a rebase. That one is easier to reason about, since it only stays open while dead targets pile up.
My suggestion: split this up. The first PR would be the gate and/or a relief pass that respects min_unused, with measurements. Keep the default caps out unless a log shows they matter. For now this is a comment rather than an approval.
| // last 8 presents used, nothing recorded work may still read. | ||
| static uint64_t own = 0; | ||
| if (frame_no % 10) return; | ||
| if (frame_no % 10 && (!own || total_bytes <= own + (own >> 3))) return; |
There was a problem hiding this comment.
This is measured against own, while the relief pass below is measured against budget (which a preset, the 90% rule or the compaction hold can make smaller). If the surge is over budget but not own, the relief only runs on every tenth present. And as Joey said, if the excess is textures this pass won't evict, it runs every present indefinitely. If you keep the bypass, a hysteresis or a cap (e.g. at most N extra passes after a gate trigger) would help.
| (total_bytes - pending_bytes > budget + (budget >> 3) || (pool_pressed && total_bytes - pending_bytes > budget))) { | ||
| const uint64_t relief_to = evict_to ? evict_to : budget; | ||
| for (auto& [last, tex] : order) { | ||
| if (pending_bytes >= total_bytes || total_bytes - pending_bytes <= relief_to || frame_no - last < 8) break; |
There was a problem hiding this comment.
8 frames is far below min_unused (60), and the comment on min_unused describes exactly the churn this causes: 200 textures made again within 10 frames. A texture the game samples every few frames can be evicted and uploaded/decoded again on every surge pass. Could this at least respect RX_VK_EVICT_MIN_UNUSED, or use a much larger floor than 8?
| @@ -4120,8 +4125,12 @@ void texture_frame(uint64_t completed) { | |||
| // after a while of play (issues 107 and 386). Kept, up to RX_VK_KEEP_FINALIZED_MB (512 by default; 0: | |||
There was a problem hiding this comment.
Nit: stale now. The default is 192 MB on cards of 6.5 GB or less.
|
I tested this on Windows 11, RTX 3080 10 GB, i7-11700, 32 GB RAM, 1920x1080. The base is my fork build (
The PR does lower the VRAM peak (and slightly lowers the retired count at 6.5 GB), which matches its goal. In my runs it did not improve the frame rate; at 6.5 GB the transition window had a long low stretch with this PR (the sample counts differ because the two runs took different times to reach the same present range). Caveats: the PR's own base is |
|
For reference, the exact machine and test setup behind the table above:
The result agrees with the review concerns above: this PR lowered the VRAM peak (9.55 -> 8.90 GB with the driver's 9 GB allowed; 6.85 -> 6.78 GB with |
hckrman101
left a comment
There was a problem hiding this comment.
Thanks @DFN-master for the runs. That is the before/after I asked for, though on a 10 GB card with a simulated 6.5 GB limit instead of a real 6-8 GB card.
What they show: the VRAM peak goes down (9.55 -> 8.90 GB; 6.85 -> 6.78 GB with RX_VK_TEST_ALLOWED_MB=6500), but the frame rate doesn't get better. With 6.5 GB simulated, the transition window drops from 41 to 15 fps. The memory sync slow-submit maximum stays the same on both sides (~253-263 ms), so the stall this PR is aimed at doesn't shrink either.
The simulated limit only changes what vram_usage reports. It doesn't page anything. So the 41 -> 15 fps drop at 6.5 GB can't come from PCIe paging: it comes from the new policy itself, on the host side. That means the budget pass now runs on every present while textures are over 112.5% of own (each pass sorts every texture, queries the budget and does write-back submissions with a CPU wait each), plus whatever the 8-frame relief pass and the smaller pool/finalized caps make again. The textures retired count is a little lower at 6.5 GB, so the extra cost seems to be the passes running more often rather than more evictions. A vk: slow submit phase breakdown from the two 6.5 GB runs would confirm which part it is.
The code is unchanged since my last review (still 956ec66), so all the earlier points are still open: the gate compares against own and the relief against budget, the gate can stay open every frame, the 8-frame age goes under min_unused, the caps are lower on cards of 6.5 GB or less (the risk to #107/#386), the test was loosened, the logging change is unrelated, there's the clash with #2484, and the stale "512 by default" comment. CI still hasn't run on this head.
Given these numbers, I'd still split it: a gate/relief change that respects min_unused and the 10-frame cadence, measured on a real 6-8 GB card, and no default cap changes unless a log shows they help. Still a comment, not an approval.
|
Thank you @Joey-Magnus, @hckrman101, and especially @DFN-master for taking the time to test and provide concrete measurements. The benchmark data across fast area transitions (Kakariko → Hyrule Field → Lake Hylia) with a 6.5 GB limit clearly demonstrates the issue: while peak VRAM was slightly reduced, running the budget pass on every frame past 112.5% caused heavy host-side submission stalls and the 8-frame relief pass induced re-upload churn, dropping transition performance from 41 FPS down to 15 FPS. On lower-spec hardware (6 GB VRAM laptops, Steam Deck), this is a noticeable regression rather than a performance win. Given these findings, along with the overlap with #2484 and the potential risk to #107/#386 from lowering the finalized keep caps, I agree with closing this PR. Thanks again for the thorough review and testing! |
When moving quickly between areas (e.g. Kokiri Forest <-> Hyrule Field <-> Market within < 60 frames),
min_unused = 60prevented eviction of newly idle textures from the previous scene while streaming in hundreds of megabytes of new textures. This caused texture allocations to surge past 170% of budget, pushing total GPU allocation past 6-8 GB VRAM capacity and causing severe 60-70 ms PCIe driver paging hitches.Changes
total_bytessurges overown + (own >> 3)(> 112.5% of texture budget), bypass the 10-frame gate intexture_frameto immediately initiate cache rebalancing.pool_cap(): Default to 128 MB (was 256 MB) on devices with <= 6.5 GB allowed VRAM.keep_finalized_max: Default to 192 MB (was 512 MB) on devices with <= 6.5 GB allowed VRAM.env_on("RX_VK_LOG_PARTIAL")to avoid spam during bursts of texture uploads.