perf(vk): the all-zero upload check reads only the guest pages that hold data, not the holes #790

Open
zeldaootr wants to merge 3 commits from zeldaootr/Longshot:zero-check-reads-no-holes into main
Contributor

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 it
zero 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) and
reads only those.

  • vk: gpu/vk/guest_holes.h (new): guest_holes::data_extents / fd_data_extents, the data extents of a range of the
    memfd, 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.
  • vk: upload_all_zero in gpu/vk/vk_texture.cpp uses them when the storage lies inside the guest window and the
    memfd is known (hos_posix_guest_memfd); otherwise, or with RX_VK_ZERO_HOLES=0, it reads the whole storage as
    before. 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: tests/test_guest_holes.py cuts upload_all_zero out of the source and runs it over a real memfd window with
    stand-ins for the renderer; tests/test_guest_holes.cpp checks guest_holes.h against a model of lseek and against a
    real memfd (robust to shmem transparent huge pages).
  • docs: RX_VK_ZERO_HOLES in docs/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 1280x800 in every log), render resolution 100% (resolution=1),
Quality preset Custom (quality_preset=0), frame generation off, headless over ssh; code defaults except
RX_VK_PRECOMPILE=0 with warm program and pipeline caches copied fresh into a private game folder each run
Scene: 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 zero lines in the log; the guest memfd's st_blocks (guest_memfds.backed_bytes)
every 2 s and /proc/meminfo and /proc/vmstat every 0.5 s; fps and frame times from the present trace. 2 runs an
arm, interleaved (main, PR, PR with RX_VK_ZERO_HOLES=0, then the reverse), the median of the two; both values where
they 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 zero line in longshot_log.txt

The PR's gpu/vk change (gpu/vk/guest_holes.h, gpu/vk/vk_texture.cpp, as in 7e7447ab) was measured applied on main
9c9d69ed (it applies there without conflicts), built as a renderer-only pack beside an unchanged build of 9c9d69ed: the
game binary is the same in every column, only longshot_vk.so differs. The RX_VK_ZERO_HOLES=0 column is this PR's
build reading the whole storage, as main does.

Before: main @ 9c9d69ed After, RX_VK_ZERO_HOLES=0: this PR @ 7e7447ab After: this PR @ 7e7447ab
Runs zh-main-1, -2 zh-off-1, -2 zh-def-1, -2
The 288 MB shadow map's check (ms), what it read 310.8 (299.9, 321.6), all 305.1 (300.1, 310.1), 288 MB 6.1 (6.2, 6.0), 16 MB read, the rest holes
The 32 MB storage's check (ms) 11.3 (8.0, 14.5) 8.6 (8.1, 9.0), 32 MB 12.7 (16.1, 9.2), 32 MB
Peak guest_memfds.backed_bytes (MiB) 5,912 (5,911, 5,914) 5,886 (5,880, 5,892) 5,638 (5,650, 5,626)
Guest backing 20 s after the arrival (MiB) 5,903 5,870 5,627
Guest backing 180 s after the arrival (MiB) 5,825, 5,636 5,611, 5,624 5,628, 5,620
Peak Shmem (/proc/meminfo, MiB) 5,917 5,774 5,644
Lowest MemAvailable (MiB) 732, 882 1,150, 810 1,192, 1,189
Lowest SwapFree (MiB) 4,492, 3,503 3,451, 3,187 2,694, 5,947
Peak pressure stall, PSI full avg10 14.4%, 13.5% 14.2%, 16.5% 20.7%, 4.1%
Swapped out, teleport to 180 s after the arrival (MiB) 1,650, 2,496 2,350, 2,895 3,210, 179
fps standing (45 s) 14.51 (14.49, 14.53) 14.44 (14.46, 14.42) 14.50 (14.50, 14.49)
Frame time p50 / p95 / p99 standing (ms) 68.9 / 72.7 / 75.0 69.2 / 73.2 / 75.3 68.9 / 73.1 / 76.0
Frames over 100 ms standing 0, 0 0, 1 0, 0
fps over the 3 pan cycles 13.36 (13.38, 13.33) 13.22 (13.45, 12.99) 13.58 (13.44, 13.72)
Frames over 100 ms in the pans 100, 97 75, 118 88, 68
Teleport to settled (s) 9.9, 9.8 12.0, 13.1 10.0, 12.0
Device loss, VK_ERROR, kernel log lines, OOM kills 0 0 0
  • The check: 16 MB of the 288 MB storage holds data on this save; the rest are holes and are not read. The 32 MB
    storage is all data, so it is read whole in every arm, as before.
  • Memory: the pages the old check gave the holes (about 270 MB) are not made, so the guest backing and Shmem
    right 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.
  • Picture: the same-spot shots (20 s and 65 s after the arrival, Link standing at the Lost Woods bridge, the camera
    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=0 37-40 / 42-45 / 43-45), the
    spread 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.
  • Across areas (one run each, Field → Lost Woods → Market, 30 camera-pan cycles, 21 minutes, same settings): the
    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.
  • Log lines common to every arm (main too): 40 storage buffer NOT bound, 4-8 a capture's wait for write-back ... timed out, 23-33 no 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).

Flag What it is Before this PR After this PR Effect Helps / doesn't help / untested Recommendation
RX_VK_ZERO_HOLES The all-zero upload check reads only the guest memfd's pages that hold data none (the whole storage read) on; 0, off, false, no: the whole storage read Holes are taken as zero without being read, so the check backs no new pages Helps: on the Steam Deck the shadow map's check 305 → 6 ms and 248-274 MB less guest memory after the Field's load; same fps and picture On for everyone; 0 only for A/B

Testing

Tested: on the Steam Deck OLED in the game (above): 6 interleaved runs, the PR's gpu/vk change on 9c9d69ed, every
run exited normally, no device loss or kernel line. On Linux (x86-64), game-free: tests/test_guest_holes.py with 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 read gpu/vk/vk_texture.cpp (32 passed); a whole-file -fsyntax-only of gpu/vk/vk_texture.cpp
for 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.py at 7e7447ab: 495 passed, 14
failed, 85 skipped; 13 of the 14 fail the same way on main 4b646738 run 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), and test_crash_record passes on its own on both.

Not tested: 7e7447ab itself in the game (its gpu/vk change was, on 9c9d69ed); a desktop Linux PC in the game; a
host 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_DATA reported as a hole would make
a 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), or RX_VK_ZERO_UPLOAD=0 (no all-zero
check at all).

Compatibility: Nothing changes.

Checklist

Everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description) of the status line's type.
  • Why gives the reason plainly and links the issues that fit (the tracker searched).
  • Measurements has a verdict, or one line saying why there is nothing to measure.
  • Testing names what was tested, what wasn't, and what must still be tested, where and how.
  • Rebased on current main. Onto main 94abdd57 (head ddad8cf8) 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 by git patch-id. The commits named in this description are from before this rebase: ed5b9f25 is now 615d6725, 7e7447ab is now e08cfa74, a14e9edf is now ddad8cf8. Re-run on 94abdd57: test_guest_holes fails at the test-only commit 615d6725 and 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 main 94abdd57 too (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.py passes (any failure that also fails on main is named).
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value.
  • A file in docs/tested/ for each new measured run (format in docs/tested-configurations.md)
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths.
  • Docs updated.
**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 it zero 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`) and reads only those. - vk: `gpu/vk/guest_holes.h` (new): `guest_holes::data_extents` / `fd_data_extents`, the data extents of a range of the memfd, 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. - vk: `upload_all_zero` in `gpu/vk/vk_texture.cpp` uses them when the storage lies inside the guest window and the memfd is known (`hos_posix_guest_memfd`); otherwise, or with `RX_VK_ZERO_HOLES=0`, it reads the whole storage as before. 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: `tests/test_guest_holes.py` cuts `upload_all_zero` out of the source and runs it over a real memfd window with stand-ins for the renderer; `tests/test_guest_holes.cpp` checks `guest_holes.h` against a model of lseek and against a real memfd (robust to shmem transparent huge pages). - docs: `RX_VK_ZERO_HOLES` in `docs/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 1280x800` in every log), render resolution 100% (`resolution=1`), Quality preset Custom (`quality_preset=0`), frame generation off, headless over ssh; code defaults except `RX_VK_PRECOMPILE=0` with warm program and pipeline caches copied fresh into a private game folder each run Scene: 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 zero` lines in the log; the guest memfd's `st_blocks` (`guest_memfds.backed_bytes`) every 2 s and `/proc/meminfo` and `/proc/vmstat` every 0.5 s; fps and frame times from the present trace. 2 runs an arm, interleaved (main, PR, PR with `RX_VK_ZERO_HOLES=0`, then the reverse), the median of the two; both values where they 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 zero` line in `longshot_log.txt` The PR's `gpu/vk` change (`gpu/vk/guest_holes.h`, `gpu/vk/vk_texture.cpp`, as in 7e7447ab) was measured applied on main 9c9d69ed (it applies there without conflicts), built as a renderer-only pack beside an unchanged build of 9c9d69ed: the game binary is the same in every column, only `longshot_vk.so` differs. The `RX_VK_ZERO_HOLES=0` column is this PR's build reading the whole storage, as main does. | | Before: main @ 9c9d69ed | After, `RX_VK_ZERO_HOLES=0`: this PR @ 7e7447ab | After: this PR @ 7e7447ab | |---|---|---|---| | Runs | zh-main-1, -2 | zh-off-1, -2 | zh-def-1, -2 | | The 288 MB shadow map's check (ms), what it read | 310.8 (299.9, 321.6), all | 305.1 (300.1, 310.1), 288 MB | **6.1** (6.2, 6.0), 16 MB read, the rest holes | | The 32 MB storage's check (ms) | 11.3 (8.0, 14.5) | 8.6 (8.1, 9.0), 32 MB | 12.7 (16.1, 9.2), 32 MB | | Peak `guest_memfds.backed_bytes` (MiB) | 5,912 (5,911, 5,914) | 5,886 (5,880, 5,892) | **5,638** (5,650, 5,626) | | Guest backing 20 s after the arrival (MiB) | 5,903 | 5,870 | **5,627** | | Guest backing 180 s after the arrival (MiB) | 5,825, 5,636 | 5,611, 5,624 | 5,628, 5,620 | | Peak `Shmem` (`/proc/meminfo`, MiB) | 5,917 | 5,774 | **5,644** | | Lowest MemAvailable (MiB) | 732, 882 | 1,150, 810 | 1,192, 1,189 | | Lowest SwapFree (MiB) | 4,492, 3,503 | 3,451, 3,187 | 2,694, 5,947 | | Peak pressure stall, PSI full avg10 | 14.4%, 13.5% | 14.2%, 16.5% | 20.7%, 4.1% | | Swapped out, teleport to 180 s after the arrival (MiB) | 1,650, 2,496 | 2,350, 2,895 | 3,210, 179 | | fps standing (45 s) | 14.51 (14.49, 14.53) | 14.44 (14.46, 14.42) | 14.50 (14.50, 14.49) | | Frame time p50 / p95 / p99 standing (ms) | 68.9 / 72.7 / 75.0 | 69.2 / 73.2 / 75.3 | 68.9 / 73.1 / 76.0 | | Frames over 100 ms standing | 0, 0 | 0, 1 | 0, 0 | | fps over the 3 pan cycles | 13.36 (13.38, 13.33) | 13.22 (13.45, 12.99) | 13.58 (13.44, 13.72) | | Frames over 100 ms in the pans | 100, 97 | 75, 118 | 88, 68 | | Teleport to settled (s) | 9.9, 9.8 | 12.0, 13.1 | 10.0, 12.0 | | Device loss, `VK_ERROR`, kernel log lines, OOM kills | 0 | 0 | 0 | - **The check:** 16 MB of the 288 MB storage holds data on this save; the rest are holes and are not read. The 32 MB storage is all data, so it is read whole in every arm, as before. - **Memory:** the pages the old check gave the holes (about 270 MB) are not made, so the guest backing and `Shmem` right 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. - **Picture:** the same-spot shots (20 s and 65 s after the arrival, Link standing at the Lost Woods bridge, the camera 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=0` 37-40 / 42-45 / 43-45), the spread 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. - **Across areas** (one run each, Field → Lost Woods → Market, 30 camera-pan cycles, 21 minutes, same settings): the 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. - Log lines common to every arm (main too): 40 `storage buffer NOT bound`, 4-8 `a capture's wait for write-back ... timed out`, 23-33 `no 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). | Flag | What it is | Before this PR | After this PR | Effect | Helps / doesn't help / untested | Recommendation | |---|---|---|---|---|---|---| | `RX_VK_ZERO_HOLES` | The all-zero upload check reads only the guest memfd's pages that hold data | none (the whole storage read) | on; `0`, `off`, `false`, `no`: the whole storage read | Holes are taken as zero without being read, so the check backs no new pages | Helps: on the Steam Deck the shadow map's check 305 → 6 ms and 248-274 MB less guest memory after the Field's load; same fps and picture | On for everyone; `0` only for A/B | ## Testing **Tested:** on the Steam Deck OLED in the game (above): 6 interleaved runs, the PR's `gpu/vk` change on 9c9d69ed, every run exited normally, no device loss or kernel line. On Linux (x86-64), game-free: `tests/test_guest_holes.py` with 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 read `gpu/vk/vk_texture.cpp` (32 passed); a whole-file `-fsyntax-only` of `gpu/vk/vk_texture.cpp` for 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.py` at 7e7447ab: 495 passed, 14 failed, 85 skipped; 13 of the 14 fail the same way on main 4b646738 run 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`), and `test_crash_record` passes on its own on both. **Not tested:** 7e7447ab itself in the game (its `gpu/vk` change was, on 9c9d69ed); a desktop Linux PC in the game; a host 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_DATA` reported as a hole would make a 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), or `RX_VK_ZERO_UPLOAD=0` (no all-zero check at all). **Compatibility:** Nothing changes. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. Onto main 94abdd57 (head ddad8cf8) 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 (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/417); each commit's change is the same as before by `git patch-id`. The commits named in this description are from before this rebase: ed5b9f25 is now 615d6725, 7e7447ab is now e08cfa74, a14e9edf is now ddad8cf8. Re-run on 94abdd57: `test_guest_holes` fails at the test-only commit 615d6725 and 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 main 94abdd57 too (`test_bc_storage_views`, `test_texture_init_barrier`, `test_crash_dialog`, `test_vk_occlusion_queries`, `test_vk_pgo`, `test_walk_key`). - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] Switches documented (docs/development.md; the README for a menu row) and shipped at their recommended value. - [x] A file in `docs/tested/` for each new measured run (format in `docs/tested-configurations.md`) - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. - [x] Docs updated.
`tests/test_guest_holes.py` cuts `upload_all_zero` out of `gpu/vk/vk_texture.cpp` and runs it over a real memfd window.
Fails: checking a 64 MB storage of holes leaves the memfd 64 MB backed, and `gpu/vk/guest_holes.h` does not exist yet.
The cases where the check still reads the whole storage (a pointer outside the window, no memfd, the extent search failing,
`RX_VK_ZERO_HOLES=0`) pass as they are.

Assisted-by: Claude
Reading a hole of the guest window's memfd through its shared mapping makes the kernel give it a zeroed page.
`upload_all_zero` read every byte of a storage to prove it zero, so the Hyrule Field shadow map (12288 x 12288 D16,
288 MB, 284 MB of it outside any import and never written) became guest memory at each Field load: 203-353 ms of
checking on a Steam Deck against 5.5 ms for a 32 MB storage already in memory, then swapped out with the load.

`gpu/vk/guest_holes.h` finds the data extents with `SEEK_DATA`/`SEEK_HOLE` (a page in swap counts as data) and the
check reads only those. `RX_VK_ZERO_HOLES=0` reads the whole storage, as before.

Assisted-by: Claude
docs(tested): the all-zero check's holes on a Steam Deck OLED at 1280x800
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Guide screenshots / Guide images and local UI source review (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
a14e9edf05
Assisted-by: Claude
HumanBean left a comment

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.cpp and tests/test_guest_holes.py, both change upload_all_zero in gpu/vk/vk_texture.cpp, and both add RX_VK_ZERO_HOLES (SEEK_DATA/SEEK_HOLE over the guest memfd, found through hos_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:

  • This PR: extents capped at 1024 (the rest read whole), which is a good bound; RX_VK_ZERO_HOLES parsed with env_value_off, so off, false and no work as documented; richer log line (4 KB read, the rest holes); documents RX_VK_ZERO_UPLOAD too.
  • This PR keeps the thread split by absolute position (per = total / nthreads), so when all the data is at one end one thread reads it all, and the existing rounding quirk stays: when total / nthreads is a multiple of 4096 and the division has a remainder, up to nthreads - 1 bytes 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.
  • 567 puts the memfd lookup in vk_memory.cpp (memory_guest_data_parts, declared in vk_internal.h), which is renderer-only and does not touch vk_api.h; this PR does the dlsym inline in vk_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 + k indexing 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_off parsing there. Leaving a comment, not an approval, for that reason.

Code review only (nothing built or run). **This PR duplicates [567](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/567) and the two cannot both land.** Both add the same new files, `gpu/vk/guest_holes.h`, `tests/test_guest_holes.cpp` and `tests/test_guest_holes.py`, both change `upload_all_zero` in `gpu/vk/vk_texture.cpp`, and both add `RX_VK_ZERO_HOLES` (SEEK_DATA/SEEK_HOLE over the guest memfd, found through `hos_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: - This PR: extents capped at 1024 (the rest read whole), which is a good bound; `RX_VK_ZERO_HOLES` parsed with `env_value_off`, so `off`, `false` and `no` work as documented; richer log line (`4 KB read, the rest holes`); documents `RX_VK_ZERO_UPLOAD` too. - This PR keeps the thread split by absolute position (`per = total / nthreads`), so when all the data is at one end one thread reads it all, and the existing rounding quirk stays: when `total / nthreads` is a multiple of 4096 and the division has a remainder, up to `nthreads - 1` bytes 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. - 567 puts the memfd lookup in `vk_memory.cpp` (`memory_guest_data_parts`, declared in `vk_internal.h`), which is renderer-only and does not touch `vk_api.h`; this PR does the `dlsym` inline in `vk_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 + k` indexing 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_off` parsing there. Leaving a comment, not an approval, for that reason.
zeldaootr force-pushed zero-check-reads-no-holes from a14e9edf05
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Guide screenshots / Guide images and local UI source review (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to ddad8cf8f8
Some checks are pending
PR description / Description follows the template (pull_request_target) Waiting to run
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
2026-11-15 19:16:24 +00:00
Compare
Author
Contributor

Rebased onto main 94abdd57 (head ddad8cf8), 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 by git patch-id.
Re-run on 94abdd57: test_guest_holes fails at the test-only commit 615d6725 and 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 main 94abdd57 too (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.

Rebased onto `main` 94abdd57 (head ddad8cf8), with no conflict; the commit subjects are now Conventional Commits, which CircleCI's lint job checks for every commit of a pull request (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/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 by `git patch-id`. Re-run on 94abdd57: `test_guest_holes` fails at the test-only commit 615d6725 and 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 main 94abdd57 too (`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.
Some checks are pending
PR description / Description follows the template (pull_request_target) Waiting to run
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
Some workflows are waiting to be reviewed.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'zero-check-reads-no-holes:zeldaootr-zero-check-reads-no-holes'
git switch 'zeldaootr-zero-check-reads-no-holes'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!790
No description provided.