Skip to content

perf(vk): no pool import for one GPU unswizzle, and the CPU unswizzle threaded from 4 MB - #2785

Open
FIocker wants to merge 1 commit into
NotRexed:mainfrom
FIocker:super-build/upload-hitches
Open

FIocker wants to merge 1 commit into
NotRexed:mainfrom
FIocker:super-build/upload-hitches

Conversation

@FIocker

@FIocker FIocker commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Turning the camera to the Deku Tree hitched for about 60 ms. Both causes are in texture uploads, and this PR fixes both. It was split out of #1834, as Joey-Magnus suggested, and builds on its own on main.

What it does

  • No pool import for one upload (vk_texture.cpp upload_unswizzle_gpu, vk_memory.cpp memory_guest_contained):
    • Before: when no imported pool held the guest memory under a texture, the GPU unswizzle imported it for that one upload. That took 22-25 ms on the replay thread each time, since the driver locks the pages, and the Deku Tree turn did three in one frame.
    • Now: the GPU path is taken only where an imported pool already holds the whole range, and the rest goes to the CPU path.
    • RX_VK_UNSWIZZLE_IMPORT=1 imports as before (read with env_on, as test_env_on.py wants).
  • The CPU unswizzle threaded from 4 MB (upload_inner), not 16 MB. The scene-size targets uploaded at that moment (2779 × 1318 RGBA8, 15 MB) took about 20 ms each on one thread; on several threads, a 15 MB level takes a few ms.

Testing

  • Builds: vk_texture.cpp and vk_memory.cpp compile with clang using nx2/build.py's renderer flags.
  • Tests: tests/run_all.py, the whole suite, locally (clang targeting MSVC, the CPM Vulkan headers): 433 passed. The failures are exactly the ones main at 51da046 has on this machine, all down to the local toolchain.
  • In game: the 60 ms hitch was measured on Windows 11 with an NVIDIA RTX card (OoT v131072) and gone with both changes, as part of the combined build. This head on its own hasn't had a separate run yet.

🤖 Generated with Claude Code

… threaded from 4 MB

Turning the camera to the Deku Tree hitched for about 60 ms. Two causes, both in texture uploads:
- upload_unswizzle_gpu imported the guest memory under a texture for that one upload when no imported pool held it already:
  22-25 ms on the replay thread each (the driver locks the pages), three of them there. It now takes the GPU path only where an
  imported pool already holds the whole range (memory_guest_contained, vk_memory.cpp) and leaves the rest to the CPU path.
  RX_VK_UNSWIZZLE_IMPORT=1 imports as before.
- the CPU path split a level over several threads only from 16 MB; the scene-size targets uploaded at that moment (2779 x 1318
  RGBA8, 15 MB) took about 20 ms each on one thread. From 4 MB now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged this into current main and ran the vk, texture, memory and image subset of tests/run_all.py with the Vulkan headers. All pass, and it merges cleanly with #2786 and #2745. The code looks right. memory_guest_contained only looks at live pools (retired ones are extracted from imported), which matches what gpu_buffer's pool_of would use without importing. The below-1 TB translation matches gpu_buffer_bind's. Pools don't overlap, so checking only the last pool at or below lo is enough. Before merging, I'd like a run of this head on its own in the Deku Tree turn, with the slow-upload or frame-time log lines. The description only says the hitch was gone in the combined #1834 build. Two reasons it matters:

  • vk_texture.cpp:1179 now sends every upload whose memory isn't imported yet to the CPU on every GPU. That is a default change, not just a fix for this scene.
  • vk_texture.cpp:1622 creates new std::threads per level from 4 MB instead of 16 MB. On a 4-core Steam Deck that is 4 threads per such level. That's likely still a win, but it hasn't been measured there.

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