perf(vk): the all-zero upload check reads only the guest pages that hold data, not the holes #790
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
beanstalk: approved @ 1697603
beanstalk: approved @ 2c5a62e
beanstalk: approved @ 4fd82b7
beanstalk: approved @ 64d6d09
beanstalk: approved @ 6f91fae
beanstalk: approved @ 70a78f9
beanstalk: approved @ 8015c9a
beanstalk: approved @ 8d361dc
beanstalk: approved @ 9d8356e
beanstalk: approved @ abf2227
beanstalk: approved @ d82f202
beanstalk: approved @ e5f1e40
beanstalk: approved @ ee8ab5f
ci: approved
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/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!790
Loading…
Reference in a new issue
No description provided.
Delete branch "zeldaootr/Longshot:zero-check-reads-no-holes"
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?
Type: perf · Risk: low · Tested on: Steam Deck OLED (SteamOS 3.9.5, RADV), Linux game-free · Needs: none
Why
Every time Hyrule Field loads, the game's 288 MB shadow map, almost none of which the game ever writes, turns into real
memory: checking it for zeros takes 200-350 ms on a Steam Deck and pushes the rest of the load into swap. The check now
skips the parts of guest memory nothing has written, which are zero without being read. See #388 (memory outgrowing RAM
and swap filling on area changes on Linux; not confirmed to be this).
What changed
A big texture whose storage is all zero is cleared on the GPU instead of unswizzled (
upload_all_zero). To prove itzero the check read every byte, and on Linux reading a page of the guest window's memfd that nothing wrote (a hole)
makes the kernel give it a zeroed page. The check now asks the memfd which parts hold data (
SEEK_DATA/SEEK_HOLE) andreads only those.
gpu/vk/guest_holes.h(new):guest_holes::data_extents/fd_data_extents, the data extents of a range of thememfd, found without touching the holes; a page in swap counts as data; past 1024 extents the rest is read whole; a
failed seek returns false and the caller reads everything.
upload_all_zeroingpu/vk/vk_texture.cppuses them when the storage lies inside the guest window and thememfd is known (
hos_posix_guest_memfd); otherwise, or withRX_VK_ZERO_HOLES=0, it reads the whole storage asbefore. No helper threads are started when there is nothing to read. Its two log lines say what was read (
4 KB read, the rest holes,64 MB read,nothing read, all holes).tests/test_guest_holes.pycutsupload_all_zeroout of the source and runs it over a real memfd window withstand-ins for the renderer;
tests/test_guest_holes.cppchecksguest_holes.hagainst a model of lseek and against areal memfd (robust to shmem transparent huge pages).
RX_VK_ZERO_HOLESindocs/development.md.Measurements
Verdict: on the Steam Deck the shadow map's check drops from 300-322 ms to 6 ms (16 MB read of 288 MB, the rest
holes), and the game's guest memory after the Field's load is 274 MB lower at its peak (5,638 against 5,912 MiB),
with the same frame rate and the same picture. Over the 3 minutes after the load the gap closes (5,620-5,628 against
5,611-5,825 MiB), and swap-out over those minutes varies more between runs than between arms, so a lasting saving
across a play session is not shown by these runs.
Hardware: Steam Deck OLED, AMD Custom GPU 0932 (RADV VANGOGH, Mesa 26.2.0-devel), SteamOS 3.9.5, kernel 7.2.7-valve2,
16 GB (14,798 MB usable), 8 GB swap; stock GPU clock (1040 MHz median in every run)
Settings: output 1280x800 (
oe: output resolution 1280x800in every log), render resolution 100% (resolution=1),Quality preset Custom (
quality_preset=0), frame generation off, headless over ssh; code defaults exceptRX_VK_PRECOMPILE=0with warm program and pipeline caches copied fresh into a private game folder each runScene: a child Kokiri save settled 20 s, a teleport to Hyrule Field (the Lost Woods bridge, which loads the 12288 x
12288 D16 shadow map, 288 MB), 20 s to settle, 45 s standing, 3 camera-pan cycles (420 game frames each), then standing
until 180 s after the arrival
Method: the
vk: upload: ... is all zerolines in the log; the guest memfd'sst_blocks(guest_memfds.backed_bytes)every 2 s and
/proc/meminfoand/proc/vmstatevery 0.5 s; fps and frame times from the present trace. 2 runs anarm, interleaved (main, PR, PR with
RX_VK_ZERO_HOLES=0, then the reverse), the median of the two; both values wherethey spread
How to reproduce: load the same Hyrule Field save twice with this PR and twice with
RX_VK_ZERO_HOLES=0, alternating,and read the shadow map's
is all zeroline inlongshot_log.txtThe PR's
gpu/vkchange (gpu/vk/guest_holes.h,gpu/vk/vk_texture.cpp, as in7e7447ab) was measured applied on main9c9d69ed(it applies there without conflicts), built as a renderer-only pack beside an unchanged build of9c9d69ed: thegame binary is the same in every column, only
longshot_vk.sodiffers. TheRX_VK_ZERO_HOLES=0column is this PR'sbuild reading the whole storage, as main does.
9c9d69edRX_VK_ZERO_HOLES=0: this PR @7e7447ab7e7447abguest_memfds.backed_bytes(MiB)Shmem(/proc/meminfo, MiB)VK_ERROR, kernel log lines, OOM killsstorage is all data, so it is read whole in every arm, as before.
Shmemright after the load are 250-280 MB lower. Within 3 minutes the other arms' backing falls to the same level in 3 of
4 runs, so the saving is mostly at the load itself. Lowest MemAvailable was 1.19 GB in both PR runs against 0.73-0.88
GB on main. The swap figures (swap-out, PSI, lowest SwapFree) follow how much of the rest of the Deck was already in
swap and spread from 179 to 3,210 MB swapped out within one arm: no difference between arms can be read from them.
not yet moved) of main and the PR were looked at side by side: the same scene, the same light and shadow on the
ground, the tunnel walls and Link; the mean red/green/blue of all 12 same-spot shots are within 4 of each other in
every arm (main 38-40 / 43-46 / 43-45, PR 37-38 / 42-44 / 42-44,
RX_VK_ZERO_HOLES=037-40 / 42-45 / 43-45), thespread of the animated scene (the fairy, the fireflies, the fog). The shadow map
this check clears is drawn as on main. The shots after the pans fall on different camera angles in each run and
were not compared.
check 201.7 ms on main, 6.7 ms here (16 MB read); by the Market the guest backing is the same in both (6.63 / 6.62
GB, peak 6.78 GB in both), and fps (Field 14.11 / 14.16, Lost Woods 10.34 / 10.31, Market 8.36 / 8.63), lowest
available (660 / 758 MiB) and lowest swap free (1,048 / 849 MiB) are within one run's spread. This PR takes the
wasted read and the pages it made out of the Field's load; it does not change the swapping later areas bring.
storage buffer NOT bound, 4-8a capture's wait for write-back ... timed out, 23-33no copy of its texels.Flags and settings
One new switch,
RX_VK_ZERO_HOLES, on by default (Linux only; other platforms read the whole storage as before).RX_VK_ZERO_HOLES0,off,false,no: the whole storage read0only for A/BTesting
Tested: on the Steam Deck OLED in the game (above): 6 interleaved runs, the PR's
gpu/vkchange on9c9d69ed, everyrun exited normally, no device loss or kernel line. On Linux (x86-64), game-free:
tests/test_guest_holes.pywith g++and with clang at each commit (fails at the test commit for the bug's reason, passes at the change); the 32 checks
under
tests/that readgpu/vk/vk_texture.cpp(32 passed); a whole-file-fsyntax-onlyofgpu/vk/vk_texture.cppfor Linux (clang) and Windows (llvm-mingw), the same warnings as main and, on Linux, the same one narrowing error main
has (line 1453 on main, 1455 here, not in this change).
python -I -B tests/run_all.pyat7e7447ab: 495 passed, 14failed, 85 skipped; 13 of the 14 fail the same way on main
4b646738run one by one on the same PC(
test_lift_cleanup_error,test_make_game_cache,test_bake_portability,test_lift_cache,test_cpm_discovery,test_appimage_player_build,test_env_on,test_crash_dialog,test_pipeline_tour,test_readback_late,test_vk_pgo,test_vram_budget_row,test_walk_key), andtest_crash_recordpasses on its own on both.Not tested:
7e7447abitself in the game (itsgpu/vkchange was, on9c9d69ed); a desktop Linux PC in the game; ahost with shmem transparent huge pages on (the memfd checks allow for 2 MB pages, but none ran with them); Windows and
macOS in the game (the change is Linux-only there, the check reads the whole storage as before); a long session across
areas.
Must be tested: None.
Risks
What could break: a page of the memfd that holds non-zero data but that
SEEK_DATAreported as a hole would makea texture that is not zero be cleared to zero. The kernel counts every page in memory or in swap as data, and a failed
search reads everything, so this is not expected; the Deck runs' shadows and picture were the same as main's.
Turning it off:
RX_VK_ZERO_HOLES=0(the whole storage read, as before), orRX_VK_ZERO_UPLOAD=0(no all-zerocheck at all).
Compatibility: Nothing changes.
Checklist
Everyone
type(scope): description) of the status line's type.main. Onto main94abdd57(headddad8cf8) with no conflict, with the commit subjects made Conventional Commits, which CI's lint job now checks for every commit, on a base that has CI's config (#417); each commit's change is the same as before bygit patch-id. The commits named in this description are from before this rebase:ed5b9f25is now615d6725,7e7447abis nowe08cfa74,a14e9edfis nowddad8cf8. Re-run on94abdd57:test_guest_holesfails at the test-only commit615d6725and passes at the head, with g++ 13.3 and clang 23.1.5; of the 49 checks that are this PR's or read its files (g++ 13.3), 43 pass and the 6 that fail fail on main94abdd57too (test_bc_storage_views,test_texture_init_barrier,test_crash_dialog,test_vk_occlusion_queries,test_vk_pgo,test_walk_key).python -I -B tests/run_all.pypasses (any failure that also fails onmainis named).third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.docs/tested/for each new measured run (format indocs/tested-configurations.md)mainand passing here.tests/that reads changed source updated in this PR, not weakened.Code review only (nothing built or run). This PR duplicates 567 and the two cannot both land.
Both add the same new files,
gpu/vk/guest_holes.h,tests/test_guest_holes.cppandtests/test_guest_holes.py, both changeupload_all_zeroingpu/vk/vk_texture.cpp, and both addRX_VK_ZERO_HOLES(SEEK_DATA/SEEK_HOLE over the guest memfd, found throughhos_posix_guest_memfd). Whichever merges second hits an add/add conflict on those files. The two also measure the same thing on a Steam Deck (shadow map check about 300 ms down to about 6 ms here, 108 ms down to 2 ms in 567). 567 is the older PR. The authors should agree on one and close the other, porting what the closed one did better.How they differ, from the diffs:
RX_VK_ZERO_HOLESparsed withenv_value_off, sooff,falseandnowork as documented; richer log line (4 KB read, the rest holes); documentsRX_VK_ZERO_UPLOADtoo.per = total / nthreads), so when all the data is at one end one thread reads it all, and the existing rounding quirk stays: whentotal / nthreadsis a multiple of 4096 and the division has a remainder, up tonthreads - 1bytes at the end of the storage are never read. 567 fixes this (share is the ceiling) and balances the threads over the data, with a test for it.vk_memory.cpp(memory_guest_data_parts, declared invk_internal.h), which is renderer-only and does not touchvk_api.h; this PR does thedlsyminline invk_texture.cpp, which is fine too.Code in this PR itself: I found no bug. The extent offsets are relative to the storage start and
p + kindexing matches; the clamp to[a,b)per thread is right; a failed seek falls back to a whole read; the window bounds check cannot overflow.My suggestion: take 567 as the base (rounding fix, balanced threads, rebased, runs on the Deck), and fold in this PR's extent cap and
env_value_offparsing there. Leaving a comment, not an approval, for that reason.a14e9edf05ddad8cf8f8Rebased onto
main94abdd57(headddad8cf8), with no conflict; the commit subjects are now Conventional Commits, which CircleCI's lint job checks for every commit of a pull request (#417), and the base has the CI's config, which a fork's pull request needs at its own head. No other change: each commit's change is the same as before bygit patch-id.Re-run on
94abdd57:test_guest_holesfails at the test-only commit615d6725and passes at the head, with g++ 13.3 and clang 23.1.5; of the 49 checks that are this PR's or read its files (g++ 13.3), 43 pass and the 6 that fail fail on main94abdd57too (test_bc_storage_views,test_texture_init_barrier,test_crash_dialog,test_vk_occlusion_queries,test_vk_pgo,test_walk_key). The description names the new commits.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.