WIP: perf(vk): no pool import for one GPU unswizzle, and the CPU unswizzle threaded from 4 MB #335
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!335
Loading…
Reference in a new issue
No description provided.
Delete branch "DUDUYG/Longshot:perf/unswizzle-no-pool-import"
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
Turning the camera toward something new, or entering an area, could hitch for tens of milliseconds while textures uploaded. Two things on the replay thread caused it:
What changed
From the original PR (FIocker's commit,
17a3ef23here):gpu/vk/vk_texture.cppupload_unswizzle_gpuandgpu/vk/vk_memory.cppmemory_guest_contained: the GPU path is taken only where one already-imported pool holds the whole range. Everything else takes the CPU path, so an upload no longer imports a pool for itself.gpu/vk/vk_texture.cppupload_inner: the CPU unswizzle is threaded from 4 MB, not 16 MB. A 15 MB scene-size level takes a few ms on several threads instead of ~20 ms on one.gpu/vk/vk_internal.h: the declaration ofmemory_guest_contained.Added for the review (
46cbcc94,3344ade7):tests/test_unswizzle_upload.pyandtests/unswizzle_upload_test.cpp: the choice and the threaded rows cut out ofvk_texture.cppand run against a model of guest memory (see Testing).docs/development.md:RX_VK_UNSWIZZLE_IMPORTin the switch reference.docs/tested/2026-11-14-market-entry-duduyg-{main,pr335}-run0{1,2,3}.md: the six runs below. The three main files are the same files in PR 334, so they merge as one.Flags and settings
RX_VK_UNSWIZZLE_IMPORT=1Measurements
The original author's report (from Hookshot PR 2785): on Windows 11 with an NVIDIA RTX card, turning the camera toward the Deku Tree hitched for about 60 ms, three of those imports in one frame. The hitch was gone with both changes, measured as part of a combined build.
This PR alone, on Longshot's current main, as the review asked:
main:
5e072fc8(2026-11-14), plus three env-gated test-logging commits of mine, b2698fae (RX_VK_PRESENT_LOG), a8988ce3 (the sampler on the same clock) and 0e78b98f (the sampler's own stalls): branch meas/main on my forkthis PR: meas/335 @ ddbc5fa7, the same plus FIocker's commit (meas/335). Its code is this branch's
17a3ef23; the tests and docs came after.Baseline: main @
5e072fc8, with the same logging commits.Hardware: RTX 5070 (621.14), Ryzen 7 9800X3D, 32 GB, Windows 11 Pro 26300. The game copy on NVMe, the romfs on SATA.
Scene: a save in the Market, loaded from the title, then the camera turned left 4 s, right 8 s, left 4 s. 1920x1080 output, frame generation 4 and no cap in
longshot_options.ini; main's lazy heap on.Method: one warm-up run of each build, then main, PR 334 and this PR in turn, three times (09:24-09:43 local), with the disks idle before each. Input by script (
HOS_INPUT_EXCLUSIVE=1). Frame times are each present's time since the one before (RX_VK_PRESENT_LOG), from 120 presents before the first log window that drew the Market to the run's end; the rest islongshot_log.txtlines. No run excluded: all nine drew the Market and stopped normally.How to reproduce: build both branches, copy a Market save, then from the title press A every 4 s until the Market draws and turn the camera as above, with
RX_VK_FRAME_TRACE=1 HOS_INPUT_EXCLUSIVE=1.5e072fc8vk: slow uploadlines per runa frame's uploads took)slow submitWhat it shows: in every main run, two slow uploads came besides the 12288x12288 R8 one: a 128x128 array texture (28-36 ms) and an ASTC texture (16-33 ms). With this PR neither appears in any run. The one left is the 12288x12288 R8 texture (294 MB) on the CPU path, which this PR doesn't touch. At the Market entry nothing is unswizzled on the GPU any more: no imported pool held those textures, so they all took the CPU path. I haven't traced why the ASTC upload goes too; on main it comes in the frame after the GPU unswizzles. The worst frames are within the run-to-run spread: the load's own frame (445 ms on main) and others this PR doesn't change. So these runs show fewer and shorter upload stalls, but no frame-time gain I can claim.
Per run (worst frame / frames over 100 ms / slow uploads): main 445/4/3, 253/2/3, 458/4/3; this PR 456/3/1, 447/5/1, 214/1/1.
The earlier combined runs (this PR together with Hookshot PR 3043, on
514591a3before the lazy heap) are in the PR's history; they measured both changes, not this one.Testing
Tested:
tests/test_unswizzle_upload.py(new). It cutsupload_unswizzle_gpu's choice (withmemory_guest_contained) andupload_inner's threaded unswizzle out ofvk_texture.cppand runs them against a model of guest memory, with suyu's realUnswizzleTexture:gpu_buffer.RX_VK_UNSWIZZLE_IMPORT=1imports as before.gpu_read; storage ending inside the span; 2 and 3 threads; 1, 8 and 16-byte texels.--source) it fails as it should: six of the choice's cases reach for a pool (five import one), and 15 levels of 4-16 MB run on one thread.python -I -B tests/run_all.pyon this branch (Windows, llvm-mingw clang 23): 513 passed, 8 failed, 41 skipped. The 8 fail the same way on main5e072fc8:test_module_ids,test_pad_layout,test_bake_portability,test_framegen_resources,test_tested_configurations(its ASCII-pipe case gets CRLF on Windows),test_oot_state_tap,test_lift_cache,test_bug_report.Not tested:
hardware_concurrency()/2gives 2-3 threads is unmeasured.Needs other testers:
vk: slow uploadlines, no texture artifacts and no worse hitch on fewer cores (T-EEPM; it waits for a maintainer's "testing wanted" label).Screenshots
Nothing on screen.
Risk and rollback
Textures that used to take the GPU unswizzle now take the CPU path when no imported pool holds them. That costs CPU time on worker threads, but no import. On a CPU with few cores the threaded path has fewer threads to spread over (unmeasured, see Testing).
RX_VK_UNSWIZZLE_IMPORT=1restores the imports; reverting the commit restores the 16 MB threshold too.Checklist
For everyone
type(scope): description).main(5e072fc8).python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above). 8 checks fail here and on main alike, named under Testing.third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested/for each new measured run (format indocs/tested-configurations.md; no appended row or assembled view).mainand passing here.🤖 Generated with Claude Code
c9ead212a9c4dd2d01aeRead head
c4dd2d01against maincf605c85: the full diff and description, the empty comments and reviews, pool lookup, virtual-address translation, CPU fallback and upload publication; rantest_import_buffer_binding,test_image_view_retry,test_texture_staging_publication,test_import_span,test_mirror_trimandtest_env_onwith GCC and Clang: all passed. No game or renderer build was run.What's good
upload_unswizzle_gpucallsgpu_buffer, which can reachimport_poolsynchronously throughpool_ofandpool_lookup. Checking for an existing import before that call is a small change, not a second upload implementation.upload_innercontinues into the CPU path. The contiguity check also stays ahead of virtual-address translation. The threaded conversion still divides work at GOB-block rows and joins its workers before flushing staging memory and queuing the copy.Must change
No patch-introduced correctness defect found in the paths reviewed. This is a comment on the WIP, not confirmation of its performance claim.
Measurement and smaller points
gpu/vk/vk_texture.cpp, lines 1203-1208 and 1645-1646, andgpu/vk/vk_memory.cpp, lines 3859-3865, have no new behavioral test. The passing view check extracts only view creation; the staging check extracts publication and checks that a join precedes it. Neither executes the new containment decision or the threaded conversion. Cover an existing containing import, a missing or partial import, a noncontiguous virtual mapping, and the override. Compare threaded output with serial output around the new threshold, including a partial final block row and a partial upload. The no-import case should reject main for actually attempting an import, not for missing a source string.cf605c85, not theeca9b12anamed as the branch base in the description. Identify the three logging commits too, documentRX_VK_UNSWIZZLE_IMPORTin the switch reference, and put the measured runs in separate files underdocs/tested/. No frozen-table rows were added by this patch. The override restores import behavior, not the old CPU threading threshold, so it is not an off switch for the whole patch. Replace “request to open” with the hardware test requests before completing that checklist item.Not checked
No Windows execution, AMD, Intel, Steam Deck, visual comparison, full suite or independent reading of the raw game runs here. In particular, the new default's CPU cost on a lower-core-count machine remains unmeasured. The focused checks do not settle that or the changed output paths. The original discussion at https://github.com/NotRexed/Hookshot/pull/2785 was unavailable, so its earlier reviews could not be checked. The current source-check jobs are blocked, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3798.
c4dd2d01ae3344ade7a0@mooki thanks. Pushed
46cbcc94and3344ade7, rebased onto5e072fc8(FIocker's commit is17a3ef23now).tests/test_unswizzle_upload.pycuts the choice (withmemory_guest_contained) and the threaded rows out ofvk_texture.cpp, and runs them against a model of guest memory.gpu_buffer: no pool, half in one, split across two pools, a noncontiguous virtual mapping, and an unmapped texture.RX_VK_UNSWIZZLE_IMPORT=1, in its own process, imports as before.UnswizzleTextureover the whole level, with the threads counted. They cover just under and at 4 MB, a partial last block row, partial uploads from the middle and to the end, sparse storage, and storage that ends inside the span.--sourceon main's code it fails on behavior: six choice cases reachgpu_buffer(five import a pool), and the 4-16 MB levels run on one thread.5e072fc8.meas/mainandmeas/335.RX_VK_UNSWIZZLE_IMPORTis documented as restoring the imports only, not the 16 MB threshold.5e072fc8. The six runs are indocs/tested/.tests/run_all.pyon Windows has 8 failures, all of which fail the same way on main; they are named in the description. I didn't build for Linux here.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.