perf(vk): the texture shadow's copies read pages staged in cached RAM, not the write-combined capture buffer #2314
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!2314
Loading…
Reference in a new issue
No description provided.
Delete branch "perf/shadow-staging"
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?
Long frames in Hyrule Field on an NVIDIA Linux PC came from take_capture's texture-shadow copies: the replay thread copied the submit's texture pages out of the capture buffer, and that buffer was write-combined (host-visible, not cached) memory, where the CPU reads at ~0.3 GB/s (~14 µs per 4 KB page with RX_VK_CAPTURE_THREADS=0; ~3 µs with the four helpers). Frames that streamed ~5000 texture pages spent up to 16 ms in those copies (RX_VK_FRAME_TRACE's memsync).
The capture buffers come from
create_buffer_cached, which asks VMA for cached host memory, but on this driver only the first ~30 MB of HOST_CACHED allocations succeed; later capture buffers (87 / 32 / 16 MB) get VK_ERROR_OUT_OF_DEVICE_MEMORY for the cached type and land in the uncached one. Requiring HOST_CACHED, or importing malloc'd RAM with VK_EXT_external_memory_host, hits the same limit, so the fix doesn't depend on the buffer's memory type:capture_make(game threads) copies the pages that are texture storage (the mergedtex_cap_ranges, already taken there) guest -> a staging block in ordinary heap RAM -> the capture buffer, so both hold the same bytes; a pageguarded_copycan't read is marked the same way as before.take_capturebuilds the shadow'sCopyPairs from the staging block; a page without a slot there (none staged, or a late page, which take_capture copies into the capture buffer after its GPU wait) is read from the capture buffer as before.RX_VK_SHADOW_STAGE=0turns it off. A line every 600 takes counts the shadow copies from staging and from the capture buffer.The game thread does one extra cached copy per texture page (~0.4 µs) to save the replay thread's uncached read (~3-14 µs).
Related: #1375 and #1619 (the capture copies this builds on), #995 (CPU-bound Hyrule Field).
A/B on this PC
RTX 3080 Ti (driver 615.78.08), Ryzen 7 5800X3D, CachyOS (kernel 7.3.1), headless gamescope 1920x1080 at 60 Hz, Hyrule Field (day), main
49f75dc8with this change, the same build with RX_VK_SHADOW_STAGE=0 and without, 2 rounds each, alternating, upstream's defaults otherwise:Deku Tree (the same build, one round): off 57.1 fps, p99 38.0 ms, 38.8 frames over 33 ms a minute; on 60.0 fps, p99 19.5 ms, 0.
The frames over 33 ms drop in both rounds; the average went up in the second round and not the first, so the fps gain is somewhere between none and ~13% here. On our fork, with the threaded recorder on top (RX_VK_RECORDER=thread), the same change was 47.9 / 48.8 -> 52.7 / 52.6 fps and 172 / 43 -> 9 / 10 frames over 33 ms a minute over two rounds. The log line (staging on) counted ~87,000 shadow copies from staging and none from the capture buffer per 600 takes; with RX_VK_SHADOW_STAGE=0, ~90,000-128,000 from the capture buffer.
On our fork the same change also kept the title screen, the same frame's picture and the audio unchanged, and a 10-minute Field run (threaded recorder on top) held 53.5 fps with 0 errors and flat memory.
Tests:
python -I -B tests/run_all.py323 passed, 3 failed: test_updater_asset_digest, test_wayland_libdecor_hint and test_builder_log_read_only fail the same way on main on this PC (environment: no Tk, the updater and libdecor checks).test_capture_copy_nt's source check now finds the fence after both of copy_part's page-by-page retries (the plain one and the staged one).Assisted-by: Claude Sonnet 5.5 (implementation), Claude Opus 5.5 (diagnosis, review, measurements).
Thanks, nice diagnosis, and the A/B table plus the 600-take counter make this easy to follow.
What I checked and found fine:
cap_buf_get/create_buffer_cachedare unchanged, the capture buffer still gets the same bytes (guest -> staging -> buffer, so both hold one snapshot), andflush_mapped(c->alloc, c->nbytes)still runs aftercopy_pagesand after the late re-copy, so the non-coherent case is as before. The staging block is plain heap, so it needs no flush/invalidate of its own.c->lateis pushed in index order, so the merge walk is right) and the shadow keeps reading them from the buffer aftertake_capture's GPU wait.stage/sidx:copy_split'spart()copieswhole, and then < 128path now passeswholethrough too. Consecutive staged pages get consecutive slots, so splitting runs onsidx[r] == sidx[r-1] + 1is right, and a failed page isok = 0, so it never reachescopied/the shadow.stage_putruns aftercopy_pairsreturns; a capture dropped another way just frees its block with theunique_ptr.RX_VK_CAPTURE_NT=1,copy_ntfences at its end and both retry loops callcopy_fence(), so the staging is visible to the replay thread as the buffer was. The test update checks both loops.The main point (inline): staging is on wherever the texture ranges aren't empty, including where the capture buffer did get HOST_CACHED memory. That's the usual case on RADV, Intel and integrated/UMA GPUs, and on NVIDIA for the first ~30 MB per your numbers. There it's one extra copy per texture page on the game thread (~0.4 us x ~5000 pages a frame, plus the up-to-96 MB kept heap) and nothing to gain, because the shadow could read the cached buffer directly. Could it check the buffer's real memory type (
vmaGetAllocationMemoryPropertiesonc->allocand testVK_MEMORY_PROPERTY_HOST_CACHED_BIT) and stage only when the bit is missing? That keeps the fix where it helps and leaves the other tiers as they are today. If you'd rather keep it unconditional, then one A/B on an AMD or Intel GPU (or one withRX_VK_LOW_SPEC) showing it's neutral there would cover it.On perf: the NVIDIA numbers are convincing for the over-33 ms frames (116/140 -> 73/16, and Deku Tree p99 38 -> 19.5 ms), less so for average fps, which you already note. Nothing from a second vendor yet, hence the ask above.
CI: the Windows builder, Linux AppImage and game-free regression workflows are waiting on maintainer approval (
action_required), so only the title check has run so far.Nit: up to 96 MB of heap stays resident here when it's idle. That's fine on a desktop, but maybe keep fewer blocks (or none) under
RX_VK_LOW_SPEC, where RAM is the tight resource. With the memory-type gate above this mostly goes away, because the cached-memory machines would never allocate a block.This stages on every GPU, including when
c->alloclanded in HOST_CACHED memory, where the shadow can already read the buffer at cached speed and the extra guest->staging->buffer copy is pure game-thread cost. Suggest adding a check likevmaGetAllocationMemoryProperties(allocator, c->alloc, &f); (f & VK_MEMORY_PROPERTY_HOST_CACHED_BIT) == 0here (or caching that flag alongside the buffer incap_free), so only the write-combined buffers get staged.NVIDIA data from a second card, measured over a longer session. Short version: the gain shows up once a session has been running a while, and there it is large in average fps too.
Setup: RTX 4070 12 GB, driver 615.71.09, Ryzen 7 5800X3D, 64 GB, Ubuntu 26.04.1, GNOME 50 Wayland (not headless), 2560x1440 output, DLSS DLAA, frame generation off,
RX_VK_PARALLEL=4. Build: nightlyv2.1.0-rc.3-46-gddafc1fwith only this PR'sgpu/vk/vk_memory.cpp(97c1a202) swapped in (source c872b22bin the log), not rebased on main.Method: two sessions, one with the default and one with
RX_VK_SHADOW_STAGE=0, full quit between. Each session did the same three steps at one fixed Kokiri Forest view: stand still 60 s on a fresh launch, explore about 10 minutes, then go back to the same view and stand still 60 s again. Over the whole session, the log line counted 99.96% of shadow copies from staging with it on (the rest from the capture buffer, as expected for late pages), and 0 from staging with it off.What we see:
bytes copiedstill reads 0.000. The extra time instead shows up as about 0.5 ms of capture time outside the GPU wait, plus about 1.1 ms more GPU wait per take.One session per arm, so treat the exact numbers as approximate. The gap after exploring is well beyond the variation I've seen between sessions on this machine.
The main point from the earlier review is still open: staging runs wherever texture ranges exist, including where the capture buffer already got
HOST_CACHEDmemory (RADV, Intel, UMA, and NVIDIA's first ~30 MB). Gating onvmaGetAllocationMemoryPropertieswould limit it to where it helps. jabbaxipil's RTX 4070 numbers cut both ways: 33.1 to 49.5 fps after 10 minutes, but 4 fps lower on a fresh launch, from a single pair.stage_on()usesatoi, so=onturns it off; #2042'senv_value_offis the convention.No new commits since my last review (still
97c1a202), so this is about the discussion since.jabbaxipil's RTX 4070 run makes the NVIDIA case stronger: once the shadow is copying ~100k pages per 600 takes, staging takes memory sync from 15.9 to 6.3 ms and the view from 33.1 to 49.5 fps. That's well past the session-to-session noise they describe, so I'm convinced it helps where the buffer is write-combined.
Still open, as Joey-Magnus also said: staging isn't gated on the buffer's memory type, so it also runs where
c->allocgot HOST_CACHED memory (inline thread on line 1638). The fresh-launch pair (59.0 staging on vs 63.2 off) is one sample and may be noise, but it points the same way, and nobody has measured a non-NVIDIA GPU yet. ThevmaGetAllocationMemoryPropertiescheck would settle it without needing more A/Bs. It would also help to put the memory type in the 600-take log line (e.g. how many takes hit a cached vs uncached buffer). jabbaxipil couldn't tell which case their sessions were in.I agree with Joey-Magnus's nit too:
stage_on()usesatoi, soRX_VK_SHADOW_STAGE=on,=trueor=yesturn staging off. Usingenv_value_offfromgpu/vk/compat/hookshot_env.hmatches the other switches.CI: only the title check has run; the build and regression workflows are still waiting on maintainer approval. GitHub reports it as mergeable.
Thanks both (@hckrman101, @Joey-Magnus) for the careful reads, and @jabbaxipil for the RTX 4070 session data. Merged main
a740d67ainto the branch; three changes:capture_makechecksvmaGetAllocationMemoryPropertiesonc->alloc(host_cached()in vk_vma.cpp) and stages only whenVK_MEMORY_PROPERTY_HOST_CACHED_BITis missing. On cached buffers the shadow reads the buffer directly, as before.RX_VK_LOW_SPECno idle staging block is kept.RX_VK_SHADOW_STAGEandRX_VK_LOW_SPECnow follow the switches' rule (env_value_off/env_on, #2042), so=onno longer turns staging off.Same binary,
RX_VK_SHADOW_STAGE=0vs default, main49f75dc8+ this branch, RTX 3080 Ti / 5800X3D, Hyrule Field, 2 rounds alternating:Deku Tree: 60.0 / 60.0 fps, p99 19.5 / 19.4 ms. The 600-take line now shows the gate at work on this driver: ~63-99k shadow copies from staging (the write-combined buffers) and ~25-29k read straight from the buffers that did get cached memory. The average gain is consistent this time (+9% in both rounds).
I don't have an AMD or Intel GPU here; with the gate those tiers take the old path whenever their buffers are cached, so they should be unaffected.
One more finding that changes the picture: rebased on today's main (
a740d67a), the capture buffers in a short Field run all got cached memory, and the 600-take line read 0 copies from staging, ~60k straight from the buffer. On the older base the driver refused HOST_CACHED after the first ~30 MB (VK_ERROR_OUT_OF_DEVICE_MEMORY) and the rest fell back to write-combined memory. My guess is that #2358 (releasing discarded Vulkan resources) stopped whatever was holding that budget, so main no longer hits the limit in a short session. I haven't checked a long exploring session like @jabbaxipil's, which is where the limit showed up before.So with the gate this PR is now only a fallback: it does nothing while the buffers are cached, and stages when a driver or a long session runs out of cached memory again. If you'd rather not carry it for that, I'm fine closing it; if you'd keep it as insurance, it's ready.
@hckrman101 your review crossed with my push; the three changes are in (see the comment above). Added your last suggestion too: the 600-take line now counts the takes whose capture buffer was HOST_CACHED and the ones that weren't (the staged case), e.g. on today's main here:
0 shadow copies from staging, 68035 from the capture buffer; capture buffers: 600 host-cached, 0 not (write-combined: staged).github-sync referenced this pull request2026-11-12 06:22:54 +00:00