perf(vk): re-upload only the part of a texture a GPU write covered #282
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!282
Loading…
Reference in a new issue
No description provided.
Delete branch "bluetoe/Longshot:lb/part-upload"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
In Castle Town, one of the game's compute pipelines binds a 1.75 MB write buffer whose range lands inside a finalized 2560x1440 render target's storage - and every frame of that burst the renderer copied the whole 15 MB image back from guest memory, on a path where nothing at all had been written by the CPU (the log says so:
upload over a drawn image ... 0 KB dirty in 0 ranges, a GPU buffer wrote it). Players on cards already at their texture budget feel it as hitching in exactly the areas the writeback work found worst.What changed
gpu_wrote_intonow reports the ranges it matched, clipped to the texture's storage and merged, in image-relative offsets;texture_touchuploads those ranges instead of the whole image, plus the CPU's writes since the last upload in the same pass (required: the whole upload used to cover those pages and the upload movesupload_genpast them, so a partial that skipped them would lose them permanently). Sparse images keep the whole path; an event where the CPU also wrote still uploads the full amount.vk: gpu-write uploads: N (X MB copied, the whole image would have been Y MB).texture_gpu_wroteandgpu_wrote_intoout of the source and runs them against stand-ins: a write inside reports its part; a write spanning either end is clipped; adjacent writes merge; an unfinished submission reports nothing; the same write is not reported to the same texture twice; a write elsewhere reports nothing. Red on main (the source it cuts does not exist there), green here.Flags and settings
RX_VK_GPU_WRITE_WHOLE(set)Measurements
main:
7315957cthis PR: lb/part-upload @
53b99634Hardware: RTX 5070 Ti 16 GB, 595.99.02, i9-13900K (32 threads), 62 GB, Ubuntu 24.04, headless runs
Scene: five area changes in one process (Hyrule Field, Market, Kokiri Forest, Hyrule Field, Market), 2560x1440, default settings,
HOS_FPS_MAX=240Method: one save, one process, the tour run twice per arm, both arms this same build with the flag off and on (main needs no separate run: RX_VK_GPU_WRITE_WHOLE=1 reproduces its exact copy path, which is the Baseline column below); the numbers are the
vk: gpu-write uploadstally lines in the run logHow to reproduce: the test save, the scripted teleport tour; the whole-image arm with
RX_VK_GPU_WRITE_WHOLE=1, the parts arm unset. The patched binary's tally prints both sides of every event, so one run gives the comparison.Baseline: this PR @
53b99634, RX_VK_GPU_WRITE_WHOLE=17315957c53b9963456% less copied on the path, 802-814 MB not copied per tour. No fps effect is claimed: the same build in the same leg varied by up to 19 fps between samples, which is the noise floor these runs established. On 8-12 GB cards, where the same events compete with an already-over-budget texture set, the saved copying should matter more.
Testing
Tested: the full suite on this base and branch:
python -I -B tests/run_all.py501 passed, 0 failed, 28 skipped (Linux; the skips want spirv-val and Windows-only tools); the focused test green on this head; the in-game tour whose numbers are above, twice per arm.Not tested: the Windows build of this branch; AMD and Intel vendors in the game. Nothing on screen should differ: the change copies a strict subset of what the old path copied, so the known partial-upload hazards get narrower, not wider.
Needs other testers:
vk: gpu-write uploadstally to show the MB drop with no texture corruption (request to open)Screenshots
Nothing on screen.
Risk and rollback
Partial uploads have a known hazard class (an unrelated variable sharing an edge page can re-upload stale bytes over a drawn resource); this change narrows the whole-image case rather than widening partial upload. If anything goes wrong in the field:
RX_VK_GPU_WRITE_WHOLE=1restores the old behaviour per run, and reverting the two commits plus the doc line is self-contained.Reviewer notes
texture_touch's union of GPU-written parts and CPU-written pages is the part to read closely; the test covers its clipping and merging.Issues
None.
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.mainand passing here.tests/that reads changed source updated in this PR, not weakened.2ba35beacdaf43048fbfperf(vk): re-upload only the part of a texture a GPU write coveredto WIP: perf(vk): re-upload only the part of a texture a GPU write coveredWIP: perf(vk): re-upload only the part of a texture a GPU write coveredto perf(vk): re-upload only the part of a texture a GPU write coveredaf43048fbf53b9963430proci referenced this pull request2026-11-14 00:01:43 +00:00
Read head
53b99634, the complete diff and description, the empty comments/reviews, and the upload and write-back callers; rantest_gpu_write_part,test_compressed_over_rtandtest_texture_staging_publicationwith GCC and Clang, plustest_bc_upload_retry: all passed. The new test against maincf605c85stops at the missing switch, before compiling.What's good
upload_genis necessary. The whole-upload switches and the existing sparse-image path remain available.Must change
Include every overlapping earlier range. In
gpu/vk/vk_texture.cpp, lines 3658-3664, the new output collects only the entries reached by the old search, which starts at the immediate predecessor of the texture's start.texture_gpu_wrotecan retain nested ranges: write[0x100000, 0x500000)in submission 5, then[0x200000, 0x300000)in submission 6. For an uploaded texture at0x280000of size0x200000, with both submissions completed, neither seen and no CPU-dirty pages, the search starts at the second entry and returns only[0, 0x80000). The first write covers the entire texture. Main's boolean result still caused a whole upload, but the new result leaves the second of two 1 MiB array layers stale on the CPU upload path, and advancinggpu_wrote_seento 6 prevents the omitted write being recovered later. Search all potentially overlapping predecessors, or maintain an interval representation that makes this search complete. Add this nested-range case to the compiled test, including the resulting upload coverage.Count the bytes the upload actually processes. In
gpu/vk/vk_texture.cpp, lines 2694-2703, the new “MB copied” counter sums the input dirty ranges. Those are not the transfer sizes.upload_unswizzle_gpuuses the ranges only to select a mip, then dispatches its full width, height and all layers; the CPU path rounds to GOB-block rows, and the ASTC path stages whole selected layers. A small write in an eligible single-mip image can therefore report a large saving while the GPU still processes the whole image. Return or record the actual amount from the selected upload path, distinguish it from dirty bytes, and update the measurements using that count. The current tally does not establish “56% less copied”.Smaller points
test_gpu_write_part.pyexecutes the range recorder and lookup, but checks the CPU/GPU union and upload call only by looking for source strings. It does not execute that union, either whole-upload switch, or the final upload. Add behavioral coverage for a disjoint CPU write alongside the GPU write.main @ 7315957c, but both arms ran53b99634. Label the whole-upload arm with its actual commit and switch. The text gives two runs per arm but not their order, warm-up or raw tally lines. The displayed totals support the stated percentage arithmetically, not the meaning of “copied”.cf605c85; it is not rebased onto that main.Not checked
No game, full build, full suite or Windows check was run here. I have not inspected the author's raw runs or pictures, measured frame times, or tested remapping and eviction after a partial update. The source-check jobs are pending, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/2733. Corrected upload counts and a same-scene visual comparison would establish the claimed saving without relying on input-range sizes; retirement counts alone do not establish that texture contents survived.
53b9963430541418349fThis is at the rebased tip
54141834, on currentmainat5e072fc8, so the hashes you reviewed under53b99634have all changed: what was622b526a,ff9d817b,7bb71d32andbcaa9305is nowc2806167,fe587a3e,3630b0ccand54141834. Nothing ingpu/vk/vk_texture.cppcame from the two commits that landed on main since, so the code you reviewed is unchanged apart from that rebase.Both must-change points hold. I checked them in the code before touching anything, and each one now has a test that failed first.
One thing the checking turned up that you did not ask for, and it changes what this PR is worth: on the head you reviewed, the byte counts you asked about were wrong in a way that hid the fact that the branch was not yet doing the work it set out to save, on the route that the Castle Town case turns out to take. Details below.
1. Overlapping earlier ranges
Your case reproduces exactly. With
[0x100000,0x500000)recorded at submission 5 and[0x200000,0x300000)at 6, a texture at0x280000of size0x200000came back as the single range[0,0x80000), andgpu_wrote_seenmoved to 6, which hides 5 for the rest of the run. The insert only dropped stored ranges that began inside the new one, so the outer write survived under its own start key while the scan began at the last key at or before the texture.texture_gpu_wrotenow keeps the recorded ranges disjoint: every stored write the new one overlaps is cut, and each part outside the new write keeps the submission that made it. That is what makes starting the scan at the first range at or before the texture complete, and the reason is written next to both places. Two cases intests/test_gpu_write_part.py: the nested pair, asserted on the coverage that comes back ([0,0x200000)), and a write ending past the storage next to one inside it, which must come back as two clipped ranges.2. The byte count
You were right about all three routes, and the measurement you asked for said something worse than imprecise. The row choice now lives in
dirty_runs()with its geometry inrow_geom(), both report the bytes they take, andupload()passes that amount out of whichever path it chose (commitc2806167). The tally keeps the three apart:On the head before that change, the motivating event reported this:
The path moved the whole image. The unswizzle used the ranges only to pick levels and then dispatched each level entire, so the branch as you reviewed it copied the same 15,360 KB that
maincopies. The "56% less copied" was arithmetic on the request, not on the work, and it is gone from the description.That is why the branch now also narrows the unswizzle (commit
54141834). The shader already offsets both sides, reading atinvocation + originand writing atinvocation + destination, so the written GOB rows go in as those offsets: one dispatch per layer per run, no shader change. Same event after that change, two samples, same binary withRX_VK_GPU_WRITE_WHOLE=1as the other arm:2,560 KB rather than 15,360, and it matches what the CPU row path reports for the same event in a run with
RX_VK_GPU_UNSWIZZLE=0(2,560 KB), which is reassuring because the two routes get their rows from the samedirty_runs(). Over a tour: 3,460 KB and 3,580 KB taken against the whole-image rule's 16,260 KB.RX_VK_GPU_WRITE_LOG=1(commit3630b0cc) prints those lines one per event, since the running tally only speaks every 50 and a tour here produces a handful.3. Smaller points
join_ranges()is now a function the test runs instead of a string it looks for: a CPU page in a corner the GPU write never reached comes back in the part, and one that reaches into the part's tail merges (commitfe587a3e). The measurement table's baseline column names the commit and the switch it actually used rather thanmain @ 7315957c.The tester rows still say "request to open". No request exists for either, so I have unticked that checklist box with the reason beside it rather than leaving it ticked. The AMD and Windows requests should probably go out with this marked high priority; say whether you or I raise them.
One housekeeping confession: the disjoint-range fix rode inside
c2806167, whose message describes only the counting. I can split it into afix(vk)and aperf(vk)if that reads better, or leave it and let the squash absorb it.Verification
python -I -B tests/run_all.pyhere: 493 passed, 3 failed, 66 skipped (the skips want vulkan-headers and Windows tools). The part test runs green on g++ 13 and clang 18.gpu/vk/vk_texture.cppcompiles clean underclang++ -fsyntax-onlywith the renderer's own include set. The tour totals in section 2 come from a renderer-only build of the pre-rebase tipbcaa9305(RX_VK_ONLY,longshot_vk.sosha2566f1b6b0d…) running over a hard-linked copy of the dev game folder's host binary. After the rebase I built the same way at54141834(873c9035…) and re-ran both arms: the Castle Town event still reports 2,560 KB taken against 15,360 KB whole, and the tour totals came out 19,340 KB and 16,780 KB against the whole-image arm's 79,500 KB and 95,760 KB; this scene produces more of these events on currentmainthan the tour totals above suggest.MKWVK_API_VERSIONis 21 at both ends, so the pairing is fine for a counter the renderer prints itself, but it is not a full current-main game. RTX 5070 Ti, driver 615.71.09, Ubuntu 24.04, headless,HOS_OUTPUT_RES=2560x1440,HOS_FPS_MAX=240, eight area changes per tour.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.