Skip to content

perf(vk): the retirement sweep's texture write-backs in batches, one submission and one wait each - #840

Merged
hckrman101 merged 3 commits into
NotRexed:mainfrom
SadTransGirl:perf/batched-eviction-writebacks
Nov 9, 2026
Merged

hckrman101 merged 3 commits into
NotRexed:mainfrom
SadTransGirl:perf/batched-eviction-writebacks

Conversation

@SadTransGirl

Copy link
Copy Markdown
Contributor

flush_texture is split into a GPU half (flush_record: the image's copies into a staging buffer, into rb_cmd) and a CPU half (flush_finish: the texels into the texture's memory), and flush_textures does a batch with one submission and one fence wait. The sweep chooses the textures to go first, writes back the flushable ones in batches of 64 or ~512 MB, then destroys them; its budget test counts the chosen ones as gone. Each write-back was a submit and a wait on its fence, a drain of the queue: a sweep of 500-600 textures at a scene load stalled for seconds (hookshot_log: 517 and 635 retired at once, 160 write-backs, submits of 4.7 and 7.7 s).

Tested: RTX 4070 SUPER, Windows 11, Kokiri Forest and the scene loads after it. Before, sweeps retiring 517 and 635 textures at once made single submits of 4.7 and 7.7 s (hookshot_log: 160 write-backs); after, the worst submit of a session with the same loads was 0.48 s, with no device loss.

…submission and one wait each

flush_texture is split into a GPU half (flush_record: the image's copies into a staging buffer, into rb_cmd) and a CPU half
(flush_finish: the texels into the texture's memory), and flush_textures does a batch with one submission and one fence
wait. The sweep chooses the textures to go first, writes back the flushable ones in batches of 64 or ~512 MB, then
destroys them; its budget test counts the chosen ones as gone. Each write-back was a submit and a wait on its fence, a
drain of the queue: a sweep of 500-600 textures at a scene load stalled for seconds (hookshot_log: 517 and 635 retired
at once, 160 write-backs, submits of 4.7 and 7.7 s).

Tested: RTX 4070 SUPER, Windows 11, Kokiri Forest and the scene loads after it. Before, sweeps retiring 517 and 635 textures
at once made single submits of 4.7 and 7.7 s (hookshot_log: 160 write-backs); after, the worst submit of a session with the
same loads was 0.48 s, with no device loss.

Assisted-by: Claude Mythos 5.1
Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
@lucasmazzoni45-del

Copy link
Copy Markdown
Contributor

Data point for this PR from an AMD card, in case it helps the rebase: RX 7800 XT + Ryzen 7 5800X3D, Windows 11, v2.0.10, 2560x1440, frame generation x2, about 1.5 hours of normal play.

The retirement sweep is very busy on this card. From hookshot_log.txt of that one session:

  • 2073 textures retired events, 108255 textures retired in all (about 50 a sweep, up to 130 in one: vk: 132 textures retired (1083 left, 3065 MB))
  • VRAM late in the session: 12.8 GB used of 15.4 GB, textures 7.1-8.1 GB against the 8192 MB budget, texture count going 4449 -> 3811 -> 3854

So the sweep runs constantly here, not just at a scene load. Each retired texture the GPU wrote goes through flush_texture, one submission and one fence wait each, so batching them as this PR does should matter on this card too. I have not measured it: I could not apply the PR as it is, because the eviction loop it changes was reworked in v2.0.8 (the dead-target handling: need_dead, drawn_over_shifted, the n_dead/b_dead counters). The merge conflicts are in exactly that loop.

Could the PR be rebased on v2.0.10 (or main)? I can test a rebased version on this machine (AMD, Windows) and report the effect on the frame times.

Related stutter data from the same card, for context: #995 (comment with the replay CPU times, v2.0.7 vs v2.0.10).

The conflict was in texture_frame's eviction loop, which main reworked (the dead transient targets: need_dead and drawn_over_shifted, the keep-unflushable case of issue 993, the evicted_from_memory record). Resolved as the review asked: main's selection logic stays as it is, with the chosen textures' bytes counted as gone on main's new break line; `shifted` is carried in Evict, and the per-eviction bookkeeping that sat between the write-back and destroy_later (n_dead/b_dead, evicted_from_memory, the remade log) runs in the destroy loop, after the batches. flush_texture had not changed on main, so the split into flush_record / flush_finish / flush_textures applies as it was.

Checked on this merge: vk_texture.cpp compiles; tests/run_all.py mem_trim, texture: 4 passed (test_mem_trim among them), test_keep_rendered and test_target_overlap skipped here for want of g++.

Assisted-by: Claude Code
Co-Authored-By: Claude Code <noreply@anthropic.com>
@SadTransGirl

Copy link
Copy Markdown
Contributor Author

@hckrman101 thanks for the careful read. Main is merged into the branch now (6ff5fc8), resolved the way you laid out:

  • main's selection logic is untouched; the chosen textures' bytes count as gone on the new break line (total_bytes - pending_bytes);
  • shifted is carried in Evict, and n_dead/b_dead, the evicted_from_memory record and the remade log run in the destroy loop, after the batches;
  • the keep-unflushable continue stays where main has it, before a texture is chosen;
  • flush_texture had not changed on main, so the split into flush_record / flush_finish / flush_textures applies as it was.

GitHub shows it mergeable, and test_mem_trim passes on the merge. It is a merge commit rather than a rebase because I could not force-push from my setup.

@lucasmazzoni45-del it applies to main now, so it builds on top of v2.0.11. If you can run a session with it, the lines to compare are the textures retired sweeps and the slow submit lines around them.

One honest note from my side: on current main I have not re-measured the effect. A ten-minute run here (RTX 4070 SUPER, Windows 11, main at 7dbdcdc with this branch) had 84 sweeps, the largest retiring 553 textures at once, and none of them had anything to write back, so the batches were not exercised; no device loss. The multi-second sweeps in the description were measured on the v2.0.5 base. A card where the sweep writes back often, like yours, is the better test.

@lucasmazzoni45-del

Copy link
Copy Markdown
Contributor

Thanks @SadTransGirl, and a correction to my own first comment here, because I ran this branch's idea on my machine and the data does not support what I wrote.

I wrote that each retired texture the GPU wrote goes through flush_texture one at a time, so batching "should matter on this card too". That was an assumption from the sweep counts, not something I had checked in the log. I now have four sessions on an RX 7800 XT / Windows 11 / v2.0.11 with a hand port of this PR in my build (flush_record / flush_finish / flush_textures, the Evict struct carrying shifted, the n_dead / b_dead bookkeeping in the destroy loop, the review's two points below), about 18 to 60 minutes each:

  • the sweeps are frequent: one session has 2539 textures retired lines, 163574 textures in all, the largest 1365 in one sweep (the others: 374 sweeps / 9607 textures / max 614; 158 / 4594 / max 709; 115 / 15863 / max 824);
  • none of them wrote anything back: the textures the GPU drew, written back into their memory before eviction: N so far line, which prints at the 1st write-back and every 50th, never appears in any of the four logs. So, like your 4070 SUPER run, the batches were never exercised here either, and I cannot say anything about their effect. My guess that this sweep was behind my stutter was wrong.
  • no did not complete in time batch timeout (the log line I added for the first review point), no device lost.

I also have no slow submit lines to compare, because I did not run with RX_VK_SLOW_LOG; I can do that if it helps.

On the two points @hckrman101 says are still open, in case my port is useful as a reference (small, builds and runs here, though as said the batch path itself has not been hit):

  • break line: if ((!sweep && (pending_bytes >= total_bytes || total_bytes - pending_bytes <= budget / 8 * 7)) || frame_no - last < need_dead) break;
  • timeout: in flush_textures, after const bool gpu_ok = any && readback_submit_wait();, if any && !gpu_ok I log once and set f.buf = VK_NULL_HANDLE, f.alloc = nullptr for every Flush in the batch before flush_finish, so the staging buffers are not freed while the copies may still write into them (they stay allocated, a rare small leak).

I would not spend more of your time on this one on my account; if a card that writes back often shows up, that is the test.

…the sweep's budget test cannot wrap

The two points left open in review, and its nit:

- When the batch's read-back does not complete (the 5 s fence wait, which also covers what queue_flush_locked just submitted, or a failed submit), flush_finish freed the staging buffers while the copies might still be writing into them. They now go on the deferred-destroy list, freed once a submission made after this one is done, as a destroyed image is. The batch's write-backs are lost, as a single one's was before; a log line says so.
- total_bytes can have drifted below the textures' sum (destroy_later clamps it), and total_bytes - pending_bytes then wrapped: the budget test never broke and everything old enough went. The chosen bytes reaching total_bytes now count as the budget met.
- The 512 MB cap of a batch counts guest_size, what the staging buffer holds, not the image's size.

Checked: vk_texture.cpp compiles; tests/run_all.py mem_trim, texture_handle, texture_init: 3 passed.

Assisted-by: Claude Mythos 5.2
Co-Authored-By: Claude Mythos 5.2 <noreply@anthropic.com>
@SadTransGirl

Copy link
Copy Markdown
Contributor Author

@hckrman101 the two open points and the nit are in 8a453c4:

  • Timeout: when the batch's read-back does not complete, the staging buffers are no longer freed in flush_finish. They go on the deferred-destroy list (dead, with rec_value + 1), so they are freed once a submission made after this one is done, the way a destroyed image is. A log line says the batch was not written back.
  • Wrap: the break line is now pending_bytes >= total_bytes || total_bytes - pending_bytes <= budget / 8 * 7.
  • Nit: the 512 MB cap counts guest_size.

vk_texture.cpp compiles and test_mem_trim, test_nvn_texture_handle and test_texture_init_barrier pass here.

@lucasmazzoni45-del thank you for running four sessions with a port and for writing up that the batches never ran. That matches my run, and I agree: the measured benefit is from the v2.0.5 base, and on current main neither of us has a sweep that writes back. Your two snippets are what went in, except that the buffers are handed to the deferred-destroy list rather than leaked.

@hckrman101
hckrman101 merged commit 581afaf into NotRexed:main Nov 9, 2026
10 checks passed
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