perf(vk): persist pipeline caches without full-size heap copies

Arkex-ai commented

2026-11-12 04:15:33 +00:00

(Migrated from github.com)

Summary

Large Vulkan pipeline-cache reads and full saves allocate a cache-sized heap buffer. This replaces that temporary allocation with file-backed storage. The benefit targets cache I/O peaks; it does not promise lower steady gameplay memory or higher FPS. Fixes #2623.

Technical changes

Validate the device/driver header before mapping. Linux reads the opened source directly with read-only MAP_PRIVATE; atomic replacement preserves the inode. Regular symlink/cloud-file reads use normal filesystem resolution.
Reserve Linux output space before mapped driver writes. Publish successful actual-length output atomically; failed allocation, I/O or replacement preserves the old cache. Keep existing owned Linux modes; new files remain 0600.
Remove recognized temporaries older than 24 hours while protecting active writers with locks/handles and refreshed timestamps. Retain recent, unrelated, linked and published files.
Remove driver-rejected play data after releasing its mapping; retain I/O failures. Format, switches, retention and scheduling stay unchanged. #2625 owns session-file policy; #2684 owns translator invalidation.

Validation

Implementation tested: 84a006d5773bd7b20f3f94ffc3a73d7cf23150e8, based on 51da0464. Original documentation head b0cc8bdb6a4b97bd9431662468e94775f195e197 only updated tested configurations; no source/build input changed at that step. The accepted maintainer integration was 9f4ad43a, merged as fc5ab981; no fresh runtime result is claimed for that integration.

Four focused native checks pass per OS, including fault injection and cleanup. Causal controls reject unsafe allocation, premature deletion and active-writer deletion.
Full source suites: Linux 472 / 0 / 24 (passed/failed/skipped); Windows 458 / 3 / 35, with all three failures reproduced on exact clean main. Matching Windows native build passes.
All applicable CI passed at 84a006d5: source/SPIR-V/lint, Windows backend/Builder, AppImage. All applicable CI also passed on documentation head b0cc8bdb: source checks, Windows, AppImage; release-asset publication is skipped for the PR.
Eight fresh Windows game processes (A/A, AB, BA, AB) reached the same Kokiri checkpoint, observed it for 30 seconds and exited normally. Initial cache/program/shader/image hashes and options matched; checkpoint hashes and all final cache hashes matched.

Linux runtime evidence is now available for b0cc8bdb (below). This PR merged as fc5ab981 after maintainer integration; the observations below remain tied to their tested heads. OneDrive integration has not been exercised. External in-place truncation is unsupported; mapped-media faults are not recoverable C++ I/O exceptions.

Memory results and external reports

RTX 5070 Ti 16 GB, NVIDIA 621.01; Ryzen 7 9800X3D / 32 GB; Windows 11, 1280x720, 60 fps cap, FG off, precompile/prediction off, RX_VK_PLAY_CACHE=0. One fixed 1,124,480,456-byte base cache plus one play file and warmed translator inputs were restored before every process. Driver cache stayed warm. Sampling started within 0.5 s, every 100 ms, with cumulative OS working-set/commit peak counters.

Candidate minus control	Pair 1	Pair 2	Pair 3
Peak private bytes (MiB)	-924.9	-1,064.3	-900.3
Peak working set (MiB)	-8.7	-59.9	+74.3
Steady private median (MiB)	+13.2	-120.2	+330.8

The A/A private peak differed by 979.9 MiB, a material spread. These bounded observations support investigating cache-I/O commit peaks, not a precise universal saving. No consistent steady private/working-set reduction, physical-memory, startup-time or FPS gain is established. Page-fault counts do not distinguish hard faults.

dankman42's RTX 3080 report compares earlier 23c50087 against 51da0464: three interleaved pairs with a 1.6 GB cache and three full saves per run, about 1.4 GB lower median peak private bytes, level working set, and a successful saved-cache reread. This is attributed earlier-head evidence.

Brett-D's RTX 5090 report finds identical steady-state memory/FPS without a full save, consistent with the scoped cache-I/O benefit; the exact PR commit is not specified. Neither external report validates Linux or the current review changes.

Warm-up runs that replaced stale translator inputs are excluded. Coverage is early Kokiri only; no campaign, Linux/AMD/Intel or OneDrive claim. Earlier 7820853d also passed the separate checkpoint-load, forced-save and fresh-read sequence; it is historical correctness evidence.

Independent Linux validation

hckrman101 tested b0cc8bdb against base 51da0464 on RX580/RADV, Linux/btrfs. With an identical 307 MB cache restored for three alternating 150 s runs each, median peak anonymous memory was 1,302 MB on main versus 984 MB here; total resident high-water was roughly level. Startup and hitch results do not establish a speed improvement. Temporary-file lifetimes cover different portions of the save and are not comparable save-latency measurements.

The report also covers interrupted save/recovery, aged orphan cleanup, truncated input, read-only output, bidirectional cross-loading and full-disk tmpfs: no crash or SIGBUS was reported. These are independently supplied results for that exact head, not tests of later main or other hardware/filesystems.

## Summary Large Vulkan pipeline-cache reads and full saves allocate a cache-sized heap buffer. This replaces that temporary allocation with file-backed storage. The benefit targets cache I/O peaks; it does not promise lower steady gameplay memory or higher FPS. Fixes #2623. ## Technical changes - Validate the device/driver header before mapping. Linux reads the opened source directly with read-only `MAP_PRIVATE`; atomic replacement preserves the inode. Regular symlink/cloud-file reads use normal filesystem resolution. - Reserve Linux output space before mapped driver writes. Publish successful actual-length output atomically; failed allocation, I/O or replacement preserves the old cache. Keep existing owned Linux modes; new files remain 0600. - Remove recognized temporaries older than 24 hours while protecting active writers with locks/handles and refreshed timestamps. Retain recent, unrelated, linked and published files. - Remove driver-rejected play data after releasing its mapping; retain I/O failures. Format, switches, retention and scheduling stay unchanged. #2625 owns session-file policy; #2684 owns translator invalidation. ## Validation Implementation tested: **`84a006d5773bd7b20f3f94ffc3a73d7cf23150e8`**, based on **`51da0464`**. Original documentation head **`b0cc8bdb6a4b97bd9431662468e94775f195e197`** only updated tested configurations; no source/build input changed at that step. The accepted maintainer integration was `9f4ad43a`, merged as `fc5ab981`; no fresh runtime result is claimed for that integration. - Four focused native checks pass per OS, including fault injection and cleanup. Causal controls reject unsafe allocation, premature deletion and active-writer deletion. - Full source suites: Linux **472 / 0 / 24** (passed/failed/skipped); Windows **458 / 3 / 35**, with all three failures reproduced on exact clean main. Matching Windows native build passes. - All applicable CI passed at `84a006d5`: [source/SPIR-V/lint](https://github.com/NotRexed/Hookshot/actions/runs/43221315004), [Windows backend/Builder](https://github.com/NotRexed/Hookshot/actions/runs/43221315227), [AppImage](https://github.com/NotRexed/Hookshot/actions/runs/43221315310). All applicable CI also passed on documentation head `b0cc8bdb`: [source checks](https://github.com/NotRexed/Hookshot/actions/runs/43224834920), [Windows](https://github.com/NotRexed/Hookshot/actions/runs/43224834918), [AppImage](https://github.com/NotRexed/Hookshot/actions/runs/43224834904); release-asset publication is skipped for the PR. - Eight fresh Windows game processes (A/A, AB, BA, AB) reached the same Kokiri checkpoint, observed it for 30 seconds and exited normally. Initial cache/program/shader/image hashes and options matched; checkpoint hashes and all final cache hashes matched. **Linux runtime evidence is now available for `b0cc8bdb` (below).** This PR merged as `fc5ab981` after maintainer integration; the observations below remain tied to their tested heads. OneDrive integration has not been exercised. External in-place truncation is unsupported; mapped-media faults are not recoverable C++ I/O exceptions. <details> <summary>Memory results and external reports</summary> RTX 5070 Ti 16 GB, NVIDIA 621.01; Ryzen 7 9800X3D / 32 GB; Windows 11, 1280x720, 60 fps cap, FG off, precompile/prediction off, `RX_VK_PLAY_CACHE=0`. One fixed 1,124,480,456-byte base cache plus one play file and warmed translator inputs were restored before every process. Driver cache stayed warm. Sampling started within 0.5 s, every 100 ms, with cumulative OS working-set/commit peak counters. | Candidate minus control | Pair 1 | Pair 2 | Pair 3 | |---|---|---|---| | Peak private bytes (MiB) | -924.9 | -1,064.3 | -900.3 | | Peak working set (MiB) | -8.7 | -59.9 | +74.3 | | Steady private median (MiB) | +13.2 | -120.2 | +330.8 | The A/A private peak differed by **979.9 MiB**, a material spread. These bounded observations support investigating cache-I/O commit peaks, not a precise universal saving. No consistent steady private/working-set reduction, physical-memory, startup-time or FPS gain is established. Page-fault counts do not distinguish hard faults. [dankman42's RTX 3080 report](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2649) compares earlier `23c50087` against `51da0464`: three interleaved pairs with a 1.6 GB cache and three full saves per run, about 1.4 GB lower median peak private bytes, level working set, and a successful saved-cache reread. This is attributed earlier-head evidence. [Brett-D's RTX 5090 report](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2649) finds identical steady-state memory/FPS without a full save, consistent with the scoped cache-I/O benefit; the exact PR commit is not specified. Neither external report validates Linux or the current review changes. Warm-up runs that replaced stale translator inputs are excluded. Coverage is early Kokiri only; no campaign, Linux/AMD/Intel or OneDrive claim. Earlier `7820853d` also passed the separate checkpoint-load, forced-save and fresh-read sequence; it is historical correctness evidence. </details> ### Independent Linux validation [hckrman101 tested `b0cc8bdb` against base `51da0464`](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2649) on RX580/RADV, Linux/btrfs. With an identical 307 MB cache restored for three alternating 150 s runs each, median peak anonymous memory was 1,302 MB on main versus 984 MB here; total resident high-water was roughly level. Startup and hitch results do not establish a speed improvement. Temporary-file lifetimes cover different portions of the save and are not comparable save-latency measurements. The report also covers interrupted save/recovery, aged orphan cleanup, truncated input, read-only output, bidirectional cross-loading and full-disk tmpfs: no crash or SIGBUS was reported. These are independently supplied results for that exact head, not tests of later main or other hardware/filesystems.

github-sync commented

2026-11-12 06:23:47 +00:00

Owner

@Arkex-ai commented on GitHub · 2026-11-12 06:12 UTC · original

#2684 addresses a complementary cause of cache growth: translator lifetime. This PR changes how cache bytes are read and published, without deciding when a cache becomes stale.

The integration point is pcache_init: #2684's stale-file check needs to run before this PR opens the file-backed initial-data mapping and before play-file merge. Its preparation rewarm rule should stay with #2684. The current std::vector line in that patch has been replaced here, so expect a small textual conflict whichever merges first.

I’ll keep this PR scoped to byte storage; there is no reason to duplicate #2684’s invalidation policy here. This is a source-level overlap assessment, not a combined-branch runtime validation.

> [@Arkex-ai](https://github.com/Arkex-ai) commented on GitHub · 2026-11-12 06:12 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6816212849) #2684 addresses a complementary cause of cache growth: translator lifetime. This PR changes how cache bytes are read and published, without deciding when a cache becomes stale. The integration point is `pcache_init`: #2684's stale-file check needs to run before this PR opens the file-backed initial-data mapping and before play-file merge. Its preparation rewarm rule should stay with #2684. The current `std::vector` line in that patch has been replaced here, so expect a small textual conflict whichever merges first. I’ll keep this PR scoped to byte storage; there is no reason to duplicate #2684’s invalidation policy here. This is a source-level overlap assessment, not a combined-branch runtime validation.

github-sync referenced this pull request

2026-11-12 07:30:27 +00:00

perf(vk): drop the pipeline cache's files when another translator made them #2684

github-sync added the

testing wanted

label

2026-11-12 11:36:09 +00:00

github-sync commented

2026-11-12 11:36:10 +00:00

Owner

@hckrman101 commented on gpu/vk/pipeline_cache_file.h line 59 on GitHub · 2026-11-12 11:10 UTC · original

This leaves holes on Linux filesystems, so the mapped writes that follow can SIGBUS on ENOSPC (point 1). posix_fallocate(file_, 0, n) (it returns an error code, not -1/errno) before mapping avoids it.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/pipeline_cache_file.h` line 59 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614920) This leaves holes on Linux filesystems, so the mapped writes that follow can SIGBUS on ENOSPC (point 1). `posix_fallocate(file_, 0, n)` (it returns an error code, not -1/errno) before mapping avoids it.

github-sync commented

2026-11-12 11:36:10 +00:00

Owner

@hckrman101 commented on gpu/vk/pipeline_cache_file.h line 159 on GitHub · 2026-11-12 11:10 UTC · original

Each interrupted save now leaves a uniquely named, cache-sized file that nothing removes (point 2). Also a nit: mkstemp creates the file 0600 and rename keeps that mode, so the published cache goes from umask-based (0644) to 0600. Probably harmless, but an fchmod to 0644 would keep the old behaviour.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/pipeline_cache_file.h` line 159 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614925) Each interrupted save now leaves a uniquely named, cache-sized file that nothing removes (point 2). Also a nit: `mkstemp` creates the file 0600 and `rename` keeps that mode, so the published cache goes from umask-based (0644) to 0600. Probably harmless, but an `fchmod` to 0644 would keep the old behaviour.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 commented on gpu/vk/pipeline_cache_file.h line 195 on GitHub · 2026-11-12 11:10 UTC · original

Refusing reparse points (and O_NOFOLLOW on Linux) is new behaviour: the old fopen followed them. Files in a OneDrive-synced folder carry FILE_ATTRIBUTE_REPARSE_POINT (cloud-file tags), and some Linux users symlink caches to another drive. Either way, the cache would then silently never load. Is there a reason to reject them? If not, I'd drop the check, or limit it to directories.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/pipeline_cache_file.h` line 195 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614932) Refusing reparse points (and `O_NOFOLLOW` on Linux) is new behaviour: the old `fopen` followed them. Files in a OneDrive-synced folder carry `FILE_ATTRIBUTE_REPARSE_POINT` (cloud-file tags), and some Linux users symlink caches to another drive. Either way, the cache would then silently never load. Is there a reason to reject them? If not, I'd drop the check, or limit it to directories.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 commented on gpu/vk/pipeline_cache_file.h line 214 on GitHub · 2026-11-12 11:10 UTC · original

This is the full-size disk copy per read (point 3). Mapping source directly with MAP_PRIVATE survives our rename-based replacement, because the inode stays alive.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/pipeline_cache_file.h` line 214 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614943) This is the full-size disk copy per read (point 3). Mapping `source` directly with `MAP_PRIVATE` survives our rename-based replacement, because the inode stays alive.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 commented on gpu/vk/vk_draw.cpp line 792 on GitHub · 2026-11-12 11:10 UTC · original

Nit/behaviour change: a play file whose header matches but which the driver refuses used to be removed. Now it's kept and re-read at every start. It never counts toward merged, so compaction never removes it either. Removing it here seems fine. An I/O failure (part.empty() && !part.rejected()) is the case worth keeping, and you already do.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_draw.cpp` line 792 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614952) Nit/behaviour change: a play file whose header matches but which the driver refuses used to be removed. Now it's kept and re-read at every start. It never counts toward `merged`, so compaction never removes it either. Removing it here seems fine. An I/O failure (`part.empty() && !part.rejected()`) is the case worth keeping, and you already do.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 commented on docs/worklist.md line 55 on GitHub · 2026-11-12 11:10 UTC · original

The blank line above (54) ends the table, so this row renders as a stray line instead of a table row. It should go directly under the 0.04 row, in score order.

> [@hckrman101](https://github.com/hckrman101) commented on `docs/worklist.md` line 55 on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#discussion_r4549614961) The blank line above (54) ends the table, so this row renders as a stray line instead of a table row. It should go directly under the 0.04 row, in score order.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 reviewed on GitHub · 2026-11-12 11:10 UTC · original

Thanks, this is a careful piece of work: unique temporaries, header check before the payload, flush/fsync before the rename, the old file and pcache_saved_size left alone on any failure, and the base mapping dropped (data = {}) before play_files_merge can try to replace the same file on Windows. The fault-injection test is a nice touch. CI is green on 096de9a5 (including Windows part 2/2 and test_pcache_header_read, which you've adapted to count ReadFile/pread instead of fread). GitHub reports conflicts with main in docs/development.md and docs/tested-configurations.md, so it needs a rebase.

A few things before this can go in:

Linux save on a full disk crashes instead of failing. writable() sizes the temporary with ftruncate, which leaves a sparse file on ext4/btrfs/xfs, and then the driver writes into the mapping. If the disk fills up, that becomes a SIGBUS inside vkGetPipelineCacheData (often at exit, in the forced save), where the old fwrite just returned short. Doing posix_fallocate before map(true) turns it back into a clean failure. Windows is fine here, because SetEndOfFile on NTFS allocates up front. The new doc says "reserves the queried capacity", which is only true on Windows today.
Orphaned temporaries pile up. The old code always reused one .tmp name, so a save killed partway (closing the game during the exit save, a crash, power loss) left at most one stray file, and it got overwritten next time. With .tmp-<pid>-<n> / .tmp-XXXXXX, every interrupted save leaves a new file of up to the full cache size (GBs) in the game folder, and nothing ever deletes them. Could pcache_init sweep mkw_vk_pipelines*.bin.tmp-* siblings? An age check would cover a second instance that's still writing.
The Linux read path copies the whole cache to disk on every launch. For a 1-2.5 GB base cache, that's a full extra write of the file at every start (plus each play file), before the driver sees a byte. That costs startup time on slow disks and wears SSDs, and it fails outright when free space is below the cache size. Our own writers never truncate in place; they always rename a new inode over the old one. A MAP_PRIVATE mapping of the opened fd already keeps its inode alive across that rename, so the snapshot only protects against an outside process truncating the file. That's a narrow case for a GB of I/O per launch. I'd map the source directly on Linux, like Windows does. If you want to keep the snapshot, please measure it on Linux (startup time and bytes written, on a large cache) first.
Measurements (our rule for perf PRs: before/after on the same scene, with the hardware named). The Windows pair is there: 9,693,954,048 vs 9,056,952,320 peak private bytes, same 487 MB input cache, warm, RTX 5070 Ti / 9800X3D. Thanks. Two asks:
- "Private bytes" excludes mapped file pages by definition, so it drops whenever a buffer moves to a mapping. Peak working set (or commit) for the same pair would show whether physical memory actually went down.
- Linux has no run at all, and it's the platform with the different (snapshot) path. A Linux before/after on the same scene (peak RSS and startup time) is needed before this merges.
Docs. The worklist row is separated from the table by a blank line, so it won't render as part of the table (inline). Eight tested-configurations rows for one PR is a lot, and most read as a validation log ("bridge PNG", "protected checkpoint hashes"). The table is for what runs on which hardware. Please fold them into one or two rows: the A/B preparation pair, and the checkpoint load/save. The detail fits better in the PR description.

Smaller points are inline.

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#pullrequestreview-5873351177) Thanks, this is a careful piece of work: unique temporaries, header check before the payload, flush/fsync before the rename, the old file and `pcache_saved_size` left alone on any failure, and the base mapping dropped (`data = {}`) before `play_files_merge` can try to replace the same file on Windows. The fault-injection test is a nice touch. CI is green on `096de9a5` (including Windows part 2/2 and `test_pcache_header_read`, which you've adapted to count `ReadFile`/`pread` instead of `fread`). GitHub reports conflicts with main in `docs/development.md` and `docs/tested-configurations.md`, so it needs a rebase. A few things before this can go in: 1. **Linux save on a full disk crashes instead of failing.** `writable()` sizes the temporary with `ftruncate`, which leaves a sparse file on ext4/btrfs/xfs, and then the driver writes into the mapping. If the disk fills up, that becomes a SIGBUS inside `vkGetPipelineCacheData` (often at exit, in the forced save), where the old `fwrite` just returned short. Doing `posix_fallocate` before `map(true)` turns it back into a clean failure. Windows is fine here, because `SetEndOfFile` on NTFS allocates up front. The new doc says "reserves the queried capacity", which is only true on Windows today. 2. **Orphaned temporaries pile up.** The old code always reused one `.tmp` name, so a save killed partway (closing the game during the exit save, a crash, power loss) left at most one stray file, and it got overwritten next time. With `.tmp-<pid>-<n>` / `.tmp-XXXXXX`, every interrupted save leaves a new file of up to the full cache size (GBs) in the game folder, and nothing ever deletes them. Could `pcache_init` sweep `mkw_vk_pipelines*.bin.tmp-*` siblings? An age check would cover a second instance that's still writing. 3. **The Linux read path copies the whole cache to disk on every launch.** For a 1-2.5 GB base cache, that's a full extra write of the file at every start (plus each play file), before the driver sees a byte. That costs startup time on slow disks and wears SSDs, and it fails outright when free space is below the cache size. Our own writers never truncate in place; they always rename a new inode over the old one. A `MAP_PRIVATE` mapping of the opened fd already keeps its inode alive across that rename, so the snapshot only protects against an outside process truncating the file. That's a narrow case for a GB of I/O per launch. I'd map the source directly on Linux, like Windows does. If you want to keep the snapshot, please measure it on Linux (startup time and bytes written, on a large cache) first. 4. **Measurements (our rule for perf PRs: before/after on the same scene, with the hardware named).** The Windows pair is there: 9,693,954,048 vs 9,056,952,320 peak private bytes, same 487 MB input cache, warm, RTX 5070 Ti / 9800X3D. Thanks. Two asks: - "Private bytes" excludes mapped file pages by definition, so it drops whenever a buffer moves to a mapping. Peak working set (or commit) for the same pair would show whether physical memory actually went down. - Linux has no run at all, and it's the platform with the different (snapshot) path. A Linux before/after on the same scene (peak RSS and startup time) is needed before this merges. 5. **Docs.** The worklist row is separated from the table by a blank line, so it won't render as part of the table (inline). Eight tested-configurations rows for one PR is a lot, and most read as a validation log ("bridge PNG", "protected checkpoint hashes"). The table is for what runs on which hardware. Please fold them into one or two rows: the A/B preparation pair, and the checkpoint load/save. The detail fits better in the PR description. Smaller points are inline.

github-sync commented

2026-11-12 11:36:11 +00:00

Owner

@hckrman101 commented on GitHub · 2026-11-12 11:10 UTC · original

Your push to 23c50087 landed while I was reviewing. It rebases on main and adds one tested-configurations row, and the code in pipeline_cache_file.h and vk_draw.cpp hasn't changed, so the review above still applies. The conflicts are gone now, so you can ignore the rebase note.

> [@hckrman101](https://github.com/hckrman101) commented on GitHub · 2026-11-12 11:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6821984540) Your push to 23c50087 landed while I was reviewing. It rebases on main and adds one tested-configurations row, and the code in `pipeline_cache_file.h` and `vk_draw.cpp` hasn't changed, so the review above still applies. The conflicts are gone now, so you can ignore the rebase note.

github-sync referenced this pull request

2026-11-12 12:36:59 +00:00

feat(vk): load shader programs when the game creates them #2760

github-sync referenced this pull request

2026-11-12 12:37:16 +00:00

build(release): add RTX 5070 Ti Kokiri Forest pipeline coverage #2599

github-sync commented

2026-11-12 12:37:21 +00:00

Owner

@Arkex-ai commented on GitHub · 2026-11-12 11:50 UTC · original

I’m addressing the remaining storage review before rerunning the measurements. The Linux capacity-reservation guard is implemented locally: the ENOSPC fault test fails against 23c50087 and passes with the guard, without mapping after allocation refusal. The change is not pushed yet.

Next are interrupted-save cleanup, Linux source mapping, symlink/cloud-file compatibility, rejected play-file cleanup and the documentation layout. I’ll keep the PR draft while closing those points and collecting repeated Windows working-set/commit measurements plus the required Linux RSS/startup comparison.

One separate CI issue: AppImage fails before compilation in System packages (add-apt-repository: Launchpad HTTP504, then timeout124). Source checks and the Windows Builder pass. Could a maintainer rerun that failed job? The description now records its actual status.

> [@Arkex-ai](https://github.com/Arkex-ai) commented on GitHub · 2026-11-12 11:50 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6822806147) I’m addressing the remaining storage review before rerunning the measurements. The Linux capacity-reservation guard is implemented locally: the ENOSPC fault test fails against `23c50087` and passes with the guard, without mapping after allocation refusal. The change is not pushed yet. Next are interrupted-save cleanup, Linux source mapping, symlink/cloud-file compatibility, rejected play-file cleanup and the documentation layout. I’ll keep the PR draft while closing those points and collecting repeated Windows working-set/commit measurements plus the required Linux RSS/startup comparison. One separate CI issue: [AppImage](https://github.com/NotRexed/Hookshot/actions/runs/43206828619) fails before compilation in System packages (`add-apt-repository`: Launchpad HTTP504, then timeout124). Source checks and the Windows Builder pass. Could a maintainer rerun that failed job? The description now records its actual status.

github-sync commented

2026-11-12 13:40:14 +00:00

Owner

@dankman42 commented on GitHub · 2026-11-12 12:53 UTC · original

Test request T-U8LS: pass

Hardware: RTX 3080 10 GB, NVIDIA 621.14, i9-12900K, 64 GB, Windows 11. Built 23c50087 (the PR head, nightly-13-g23c50087) against main 51da0464 (nightly-11-g51da0464, its base), both standalone from source with llvm-mingw, default options.

What I checked: the repeated same-scene before/after memory pairs the description still asks for, with a large real cache. Before every run, the build's cache files were replaced by one fixed copy of a prepared install's cache: mkw_vk_pipelines.bin 1,615,665,731 bytes plus three play files (6.8 MB). Each run: start, load a save (adult, Kakariko), teleport to Hyrule Field, 30 s there, then hookshot_ctl.py quit. Every run read the base, merged the 3 play files and saved the base three times in play (1,580,092 / ~1,611,000 / ~1,628,500 KB), so each covers one full read, the start merge and three full saves. Private bytes (PagedMemorySize64, and the process's own PeakPagedMemorySize64) and the working set were sampled every 250 ms. One warm-up run per build first; then main, PR, PR, main, main, PR.

	main `51da0464`	PR 23c50087
peak private bytes	27,831 / 28,274 / 27,657 MB (median 27,831)	26,418 / 27,009 / 26,062 MB (median 26,418)
peak working set	16,437 / 16,660 / 16,319 MB	16,374 / 17,025 / 16,310 MB
cache saves in play, all published	3 / 3 / 3	3 / 3 / 3

Result:

Peak private bytes are about 1.4 GB lower (median -1,413 MB, -5.1 %). That is about the size of the 1.54 GB cache, and every PR run is below every main run (PR at most 27,009 MB, main at least 27,657 MB).
The working set is level, as the description says it would be: file-backed pages still count there.
The peak comes about 60 s into each run, in the teleport load, in both builds.

Round trip: after the last PR run, a further PR start without restoring the copy read the cache that run had published (vk: pipeline cache ... (1628351 KB), the size its last save logged). It reached play, saved again and quit cleanly. No .tmp file was left beside the cache after any run.

Log lines (pair 1, the same in all six runs apart from the save sizes):

vk: pipeline cache ...\mkw_vk_pipelines.bin (1577798 KB)
vk: pipeline cache: 3 play files merged (6963 KB)
vk: pipeline cache saved (1580092 KB)
vk: pipeline cache saved (1610345 KB)
vk: pipeline cache saved (1627960 KB)

Not tested: frame rate, start-up time, Linux, AMD and Intel, a cache-save failure, and the start compaction at kPlayFilesKept play files (3 were present, so no compaction ran).

Tested-configurations row, if you want to fold it into the PR:

| 2026-11-12 UTC | nightly-13-g23c50087 (PR head) against nightly-11-g51da0464 (main) | Windows 11, windowed | RTX 3080 10 GB, NVIDIA 621.14 | i9-12900K, 64 GB | 1280x720 window, default options | Kakariko save, teleport to Hyrule Field, 30 s, quit; same 1.6 GB base cache + 3 play files restored before each run; 3 interleaved pairs | Peak private bytes 26,062-27,009 MB against 27,657-28,274 MB (median -1,413 MB); working set level; 3 saves in play each run, all published; a fresh start read the PR's published cache |

> [@dankman42](https://github.com/dankman42) commented on GitHub · 2026-11-12 12:53 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6824141749) Test request T-U8LS: pass Hardware: RTX 3080 10 GB, NVIDIA 621.14, i9-12900K, 64 GB, Windows 11. Built 23c50087 (the PR head, `nightly-13-g23c50087`) against main 51da0464 (`nightly-11-g51da0464`, its base), both standalone from source with llvm-mingw, default options. **What I checked:** the repeated same-scene before/after memory pairs the description still asks for, with a large real cache. Before every run, the build's cache files were replaced by one fixed copy of a prepared install's cache: `mkw_vk_pipelines.bin` 1,615,665,731 bytes plus three play files (6.8 MB). Each run: start, load a save (adult, Kakariko), teleport to Hyrule Field, 30 s there, then `hookshot_ctl.py quit`. Every run read the base, merged the 3 play files and saved the base three times in play (1,580,092 / ~1,611,000 / ~1,628,500 KB), so each covers one full read, the start merge and three full saves. Private bytes (`PagedMemorySize64`, and the process's own `PeakPagedMemorySize64`) and the working set were sampled every 250 ms. One warm-up run per build first; then main, PR, PR, main, main, PR. | | main 51da0464 | PR 23c50087 | |---|---|---| | peak private bytes | 27,831 / 28,274 / 27,657 MB (median 27,831) | 26,418 / 27,009 / 26,062 MB (median 26,418) | | peak working set | 16,437 / 16,660 / 16,319 MB | 16,374 / 17,025 / 16,310 MB | | cache saves in play, all published | 3 / 3 / 3 | 3 / 3 / 3 | **Result:** - Peak private bytes are about 1.4 GB lower (median -1,413 MB, -5.1 %). That is about the size of the 1.54 GB cache, and every PR run is below every main run (PR at most 27,009 MB, main at least 27,657 MB). - The working set is level, as the description says it would be: file-backed pages still count there. - The peak comes about 60 s into each run, in the teleport load, in both builds. **Round trip:** after the last PR run, a further PR start without restoring the copy read the cache that run had published (`vk: pipeline cache ... (1628351 KB)`, the size its last save logged). It reached play, saved again and quit cleanly. No `.tmp` file was left beside the cache after any run. Log lines (pair 1, the same in all six runs apart from the save sizes): ``` vk: pipeline cache ...\mkw_vk_pipelines.bin (1577798 KB) vk: pipeline cache: 3 play files merged (6963 KB) vk: pipeline cache saved (1580092 KB) vk: pipeline cache saved (1610345 KB) vk: pipeline cache saved (1627960 KB) ``` **Not tested:** frame rate, start-up time, Linux, AMD and Intel, a cache-save failure, and the start compaction at `kPlayFilesKept` play files (3 were present, so no compaction ran). Tested-configurations row, if you want to fold it into the PR: `| 2026-11-12 UTC | nightly-13-g23c50087 (PR head) against nightly-11-g51da0464 (main) | Windows 11, windowed | RTX 3080 10 GB, NVIDIA 621.14 | i9-12900K, 64 GB | 1280x720 window, default options | Kakariko save, teleport to Hyrule Field, 30 s, quit; same 1.6 GB base cache + 3 play files restored before each run; 3 interleaved pairs | Peak private bytes 26,062-27,009 MB against 27,657-28,274 MB (median -1,413 MB); working set level; 3 saves in play each run, all published; a fresh start read the PR's published cache |`

github-sync commented

2026-11-12 13:40:14 +00:00

Owner

@Brett-D commented on GitHub · 2026-11-12 13:04 UTC · original

Tested on RTX 5090 (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) with RX_RELEASE=1 (17,888 programs, 25,303 graphics and 799 compute pipelines precompiled at startup) and a 1.20 GB (1,229,641 KB) mkw_vk_pipelines.bin in our Kokiri Forest regression harness (2048x1152 uncapped, RX_VK_FRAMEGEN=0, HOS_FPS_MAX=240, 80 s run: 40 s settle + 30 s measurement window, no hookshot_ctl.py quit).

Arm (`RX_RELEASE=1`, 25,303 pipelines, `1.20 GB` `mkw_vk_pipelines.bin`)	Peak Working Set (`t=60-80s`)	Peak Private Bytes (`t=60-80s`)	Precompile Time	Uncapped FPS (`p95` / `p99` ms)	Frames > 50 ms
`main` (`c544828f`)	17,458 MB	33,818 MB	2,563 ms	67.2 (`17.07` / `19.61`)	0
#2649 (`Arkex-ai`)	17,458 MB (0 MB)	33,818 MB (0 MB)	2,630 ms	67.2 (`17.07` / `19.61`)	0

Why steady-state field peak is unchanged while full-save peak drops by the cache size:

In pcache_init(), main reads mkw_vk_pipelines.bin into a std::vector<uint8_t> data, passes data.data() to vkCreatePipelineCache (which copies/parses it into the driver's internal heap), and then immediately drops data = {} before precompile_run() builds the 25,303 pipelines and before the scene loads.
During normal gameplay with RX_VK_PLAY_CACHE on (default), play_cache_save() only writes small per-session mkw_vk_pipelines_play_*.bin deltas (17 KB in this 80 s run) unless pcache grows by >= 64 MB (kPlayBaseSaveStep) or pipeline_cache_save(true) runs at clean exit / --prepare-shaders / kPlayFilesKept merge.
Consequently, when peak memory occurs later in the field (t=60-80s) without a forced full base-cache save, main's std::vector<uint8_t> is already gone and steady-state Peak Working Set / Peak Private Bytes are identical (0 MB delta).
Where #2649 saves ~1.2–2.5 GB of peak private commit is during full base-cache saves (pipeline_cache_save(true) at exit / --prepare-shaders / >= 64 MB growth, as @dankman42's hookshot_ctl.py quit test measured) and during the pcache_init() / play_files_merge() startup window, where main simultaneously holds both the live VkPipelineCache and a full-size std::vector<uint8_t> heap buffer.

> [@Brett-D](https://github.com/Brett-D) commented on GitHub · 2026-11-12 13:04 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6824388643) Tested on `RTX 5090` (32 GB VRAM, Windows 11, 98 GB RAM, NVIDIA 621.14) with `RX_RELEASE=1` (`17,888` programs, `25,303` graphics and `799` compute pipelines precompiled at startup) and a `1.20 GB` (`1,229,641 KB`) `mkw_vk_pipelines.bin` in our Kokiri Forest regression harness (`2048x1152` uncapped, `RX_VK_FRAMEGEN=0`, `HOS_FPS_MAX=240`, 80 s run: 40 s settle + 30 s measurement window, no `hookshot_ctl.py quit`). | Arm (`RX_RELEASE=1`, 25,303 pipelines, `1.20 GB` `mkw_vk_pipelines.bin`) | Peak Working Set (`t=60-80s`) | Peak Private Bytes (`t=60-80s`) | Precompile Time | Uncapped FPS (`p95` / `p99` ms) | Frames > 50 ms | |---|---|---|---|---|---| | `main` (`c544828f`) | 17,458 MB | 33,818 MB | 2,563 ms | 67.2 (`17.07` / `19.61`) | 0 | | **#2649 (`Arkex-ai`)** | **17,458 MB (0 MB)** | **33,818 MB (0 MB)** | **2,630 ms** | **67.2 (`17.07` / `19.61`)** | **0** | **Why steady-state field peak is unchanged while full-save peak drops by the cache size:** - In `pcache_init()`, `main` reads `mkw_vk_pipelines.bin` into a `std::vector<uint8_t> data`, passes `data.data()` to `vkCreatePipelineCache` (which copies/parses it into the driver's internal heap), and then immediately drops `data = {}` *before* `precompile_run()` builds the 25,303 pipelines and before the scene loads. - During normal gameplay with `RX_VK_PLAY_CACHE` on (default), `play_cache_save()` only writes small per-session `mkw_vk_pipelines_play_*.bin` deltas (`17 KB` in this 80 s run) unless `pcache` grows by `>= 64 MB` (`kPlayBaseSaveStep`) or `pipeline_cache_save(true)` runs at clean exit / `--prepare-shaders` / `kPlayFilesKept` merge. - Consequently, when peak memory occurs later in the field (`t=60-80s`) without a forced full base-cache save, `main`'s `std::vector<uint8_t>` is already gone and steady-state Peak Working Set / Peak Private Bytes are identical (`0 MB` delta). - Where #2649 saves **`~1.2–2.5 GB` of peak private commit** is during **full base-cache saves (`pipeline_cache_save(true)` at exit / `--prepare-shaders` / `>= 64 MB` growth, as @dankman42's `hookshot_ctl.py quit` test measured)** and during the `pcache_init()` / `play_files_merge()` startup window, where `main` simultaneously holds both the live `VkPipelineCache` and a full-size `std::vector<uint8_t>` heap buffer.

github-sync commented

2026-11-12 13:40:14 +00:00

Owner

@Arkex-ai commented on GitHub · 2026-11-12 13:10 UTC · original

Thanks both — the distinction between cache-I/O peaks and steady gameplay memory is useful. The review fixes are now in 84a006d5, including direct Linux reads, reserved output and protected stale-temp cleanup; all applicable CI passed.

I also finished eight fresh Windows game runs on that implementation (A/A, then AB/BA/AB), with identical restored inputs and the same Kokiri checkpoint. The three pairs showed 925/1,064/900 MiB lower peak private bytes, but A/A itself varied by 980 MiB. Working-set and steady-private results do not establish a consistent reduction, so the description now states that limit explicitly. All processes quit normally and preserved the checkpoint.

I've attributed the RTX 3080 result to 23c50087 in tested configurations. @Brett-D, which PR commit did your harness use? I'll keep that steady-state result separate from the full cache-I/O observations. Actual Linux game RSS/startup is still pending, so this remains draft.

> [@Arkex-ai](https://github.com/Arkex-ai) commented on GitHub · 2026-11-12 13:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6824534869) Thanks both — the distinction between cache-I/O peaks and steady gameplay memory is useful. The review fixes are now in `84a006d5`, including direct Linux reads, reserved output and protected stale-temp cleanup; all applicable CI passed. I also finished eight fresh Windows game runs on that implementation (A/A, then AB/BA/AB), with identical restored inputs and the same Kokiri checkpoint. The three pairs showed 925/1,064/900 MiB lower peak private bytes, but A/A itself varied by 980 MiB. Working-set and steady-private results do not establish a consistent reduction, so the description now states that limit explicitly. All processes quit normally and preserved the checkpoint. I've attributed the RTX 3080 result to `23c50087` in tested configurations. @Brett-D, which PR commit did your harness use? I'll keep that steady-state result separate from the full cache-I/O observations. Actual Linux game RSS/startup is still pending, so this remains draft.

github-sync referenced this pull request

2026-11-12 14:43:32 +00:00

fix(vk): one pipeline play file a session, and the next start puts them together #2625

github-sync referenced this pull request

2026-11-12 14:44:11 +00:00

perf(vk): drop the pipeline cache's files when another translator made them #2684

github-sync referenced this pull request

2026-11-12 15:49:05 +00:00

perf(vk): opt-in pipeline binary cache, lighter precompile and a wait budget for AMD Windows #2814

github-sync referenced this pull request

2026-11-12 15:49:19 +00:00

[Tracking] Linux and Steam Deck: GPU hangs, crashes, low FPS and build failures #1000

github-sync referenced this pull request

2026-11-12 15:51:47 +00:00

perf(vk): drop the pipeline cache's files when another translator made them #2684

github-sync referenced this pull request

2026-11-12 16:54:42 +00:00

[Tracking] Linux and Steam Deck: GPU hangs, crashes, low FPS and build failures #1000

github-sync referenced this pull request

2026-11-12 16:55:23 +00:00

fix(vk): one pipeline play file a session, and the next start puts them together #2625

github-sync commented

2026-11-12 16:55:30 +00:00

Owner

@hckrman101 reviewed on GitHub · 2026-11-12 16:47 UTC · original

Thanks for working through all of it. I re-read 84a006d5 and b0cc8bdb against my review at 096de9a5. Every code point is fixed, and the docs are cleaned up. What's left is the Linux measurement.

Fixed:

Full disk on Linux. posix_fallocate now runs before map(true), and a failure goes through reset(), which unlinks the temporary and leaves the old cache and pcache_saved_size alone.
- On filesystems that don't support fallocate, glibc writes one byte per block instead. That's slow on a GB cache over NFS, but it still reserves the space and fails cleanly. tmpfs, btrfs, xfs and extent-based ext4 support fallocate directly. An EINTR just fails that save, and the next one retries.
- The doc's "clean failure, not SIGBUS" holds everywhere except copy-on-write filesystems like ZFS, which can't truly reserve space. That's not worth more code.
Orphaned temporaries. The sweep looks safe to me:
- It only touches the two temp name patterns, owned by us, with one link, not a symlink, and 24 h old.
- A live writer is protected twice. On Linux the writer holds flock for as long as its fd is open. On Windows the writer's handle shares nothing, and the sweeper opens the file with FILE_SHARE_DELETE only.
- refresh_modified() before the close covers the gap between close and rename.
- The inode re-check before unlink closes the path-swap race.
- The one leftover race: if the sweeper takes the lock between another instance's mkstemp and flock, that save fails and the next one retries. It never deletes anything.
- Nit, optional: since the lock already protects writers, the 24 h age could come down to an hour or so. A crash during the exit save then wouldn't leave a cache-sized file for a whole day.
Linux read. It now maps the source directly with MAP_PRIVATE and writes no temporary.
- The only pread left is the 32-byte header. A short read there just makes that start's read empty, and the file is kept, which is the right outcome.
- Files over 4 GB are fine: the length is 64-bit throughout, and both mmap and MapViewOfFile take the full size.
Smaller points:
- Symlinks and OneDrive placeholders now open as fopen did.
- A play file the driver refuses is removed after reset() releases the view.
- Existing caches keep their mode. A new file stays 0600, which is fine by me.
Docs. The worklist row is back in the table, in score order. The tested-configurations rows are folded into three. Thanks.

Evidence so far:

dankman42's RTX 3080 run (Windows, at 23c50087) is the clean result: three interleaved pairs with a 1.6 GB cache and three full saves per run. Median peak private bytes drop by about 1.4 GB, every PR run is below every main run, the working set is level as expected, and a fresh start reads the saved cache. Since then only the I/O code has changed, and the save path is the same apart from the reservation.
Your own pairs agree in direction, but your A/A spread of 980 MiB is as large as the effect, so I'd lean on dankman42's numbers.
Brett-D's RTX 5090 run with no full save shows no change, which fits the scope the description now states.

What's needed to merge: a Linux before/after on the same scene, as the description already says.

Please take peak RssAnon (or VmHWM together with RssAnon from /proc/<pid>/status) and the time to the vk: pipeline cache ... KB line, with a large cache and at least one full save, and name the hardware.
Plain RSS counts the mapped file pages, so it will look level, the same way the working set does on Windows.
We have a Linux AMD run of this PR going for the T-U8LS request. If it reports those numbers, that covers it. Once the Linux run is in, take it out of draft and I'll approve.

CI and conflicts: all green on b0cc8bdb, including AppImage and Windows part 2/2. It merges cleanly with main.

Order with #2625: I said earlier that #2625 should land first. #2625 hasn't moved since its review, and its fold-never-fires problem is still open, so I wouldn't hold this PR for it any more. Whichever is ready first goes in.

The conflict is small: adjacent hunks in play_files_merge and the shared test file.
After the rebase, #2625's session file is written through PipelineCacheFile::writable. An interrupted rewrite leaves mkw_vk_pipelines_play_<n>.bin.tmp-XXXXXX, which matches temporary_name(). The running session's own temporary is locked, so the sweep covers it as is.
#2684 and #2456 also touch pcache_init and the save path, so expect small textual conflicts there too. Your note on where #2684's stale check goes still applies.

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 16:47 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#pullrequestreview-5878426010) Thanks for working through all of it. I re-read `84a006d5` and `b0cc8bdb` against my review at `096de9a5`. Every code point is fixed, and the docs are cleaned up. What's left is the Linux measurement. **Fixed:** 1. **Full disk on Linux.** `posix_fallocate` now runs before `map(true)`, and a failure goes through `reset()`, which unlinks the temporary and leaves the old cache and `pcache_saved_size` alone. - On filesystems that don't support fallocate, glibc writes one byte per block instead. That's slow on a GB cache over NFS, but it still reserves the space and fails cleanly. tmpfs, btrfs, xfs and extent-based ext4 support fallocate directly. An `EINTR` just fails that save, and the next one retries. - The doc's "clean failure, not SIGBUS" holds everywhere except copy-on-write filesystems like ZFS, which can't truly reserve space. That's not worth more code. 2. **Orphaned temporaries.** The sweep looks safe to me: - It only touches the two temp name patterns, owned by us, with one link, not a symlink, and 24 h old. - A live writer is protected twice. On Linux the writer holds `flock` for as long as its fd is open. On Windows the writer's handle shares nothing, and the sweeper opens the file with `FILE_SHARE_DELETE` only. - `refresh_modified()` before the close covers the gap between close and rename. - The inode re-check before `unlink` closes the path-swap race. - The one leftover race: if the sweeper takes the lock between another instance's `mkstemp` and `flock`, that save fails and the next one retries. It never deletes anything. - Nit, optional: since the lock already protects writers, the 24 h age could come down to an hour or so. A crash during the exit save then wouldn't leave a cache-sized file for a whole day. 3. **Linux read.** It now maps the source directly with `MAP_PRIVATE` and writes no temporary. - The only `pread` left is the 32-byte header. A short read there just makes that start's read empty, and the file is kept, which is the right outcome. - Files over 4 GB are fine: the length is 64-bit throughout, and both mmap and `MapViewOfFile` take the full size. 4. **Smaller points:** - Symlinks and OneDrive placeholders now open as `fopen` did. - A play file the driver refuses is removed after `reset()` releases the view. - Existing caches keep their mode. A new file stays 0600, which is fine by me. 5. **Docs.** The worklist row is back in the table, in score order. The tested-configurations rows are folded into three. Thanks. **Evidence so far:** - dankman42's RTX 3080 run (Windows, at `23c50087`) is the clean result: three interleaved pairs with a 1.6 GB cache and three full saves per run. Median peak private bytes drop by about 1.4 GB, every PR run is below every main run, the working set is level as expected, and a fresh start reads the saved cache. Since then only the I/O code has changed, and the save path is the same apart from the reservation. - Your own pairs agree in direction, but your A/A spread of 980 MiB is as large as the effect, so I'd lean on dankman42's numbers. - Brett-D's RTX 5090 run with no full save shows no change, which fits the scope the description now states. **What's needed to merge:** a Linux before/after on the same scene, as the description already says. - Please take peak `RssAnon` (or `VmHWM` together with `RssAnon` from `/proc/<pid>/status`) and the time to the `vk: pipeline cache ... KB` line, with a large cache and at least one full save, and name the hardware. - Plain RSS counts the mapped file pages, so it will look level, the same way the working set does on Windows. - We have a Linux AMD run of this PR going for the T-U8LS request. If it reports those numbers, that covers it. Once the Linux run is in, take it out of draft and I'll approve. **CI and conflicts:** all green on `b0cc8bdb`, including AppImage and Windows part 2/2. It merges cleanly with main. **Order with #2625:** I said earlier that #2625 should land first. #2625 hasn't moved since its review, and its fold-never-fires problem is still open, so I wouldn't hold this PR for it any more. Whichever is ready first goes in. - The conflict is small: adjacent hunks in `play_files_merge` and the shared test file. - After the rebase, #2625's session file is written through `PipelineCacheFile::writable`. An interrupted rewrite leaves `mkw_vk_pipelines_play_<n>.bin.tmp-XXXXXX`, which matches `temporary_name()`. The running session's own temporary is locked, so the sweep covers it as is. - #2684 and #2456 also touch `pcache_init` and the save path, so expect small textual conflicts there too. Your note on where #2684's stale check goes still applies.

github-sync referenced this pull request

2026-11-12 16:56:31 +00:00

perf(vk): the 90% rule and the driver's limit evict their excess, not an eighth of the texture cache #2826

github-sync referenced this pull request from a commit

2026-11-12 17:57:21 +00:00

feat(vk): load shader programs when the game creates them (#2760)

github-sync referenced this pull request

2026-11-12 17:58:33 +00:00

perf(vk): drop the pipeline cache's files when another translator made them #2684

github-sync referenced this pull request from a commit

2026-11-12 19:00:50 +00:00

perf(vk): persist pipeline caches without full-size heap copies (#2649)

github-sync closed this pull request

2026-11-12 19:01:47 +00:00

github-sync commented

2026-11-12 19:01:48 +00:00

Owner

@hckrman101 commented on GitHub · 2026-11-12 18:05 UTC · original

Test request T-U8LS: pass

Hardware: AMD Radeon RX 580 8 GB (RADV Polaris10), Mesa 26.2.6, CachyOS Linux (btrfs), 8 threads, 15 GB RAM. Built b0cc8bdb (the PR head) against main 51da0464, its base. Both are the same game build, with only hookshot_vk.so from the PR.

What I checked: every run started from the same 307 MB mkw_vk_pipelines.bin, a large cache from an earlier run on this card and driver, copied back in before each run. The run loads the test save and plays Kokiri for 150 s (headless, RX_VK_PRECOMPILE=0). PR and main alternated, 3 runs each. /proc/<pid>/status and /proc/<pid>/io were sampled every 250 ms, the folder's cache temporaries every 50 ms, and smaps every 2 s.

Exit save: a headless game ends on SIGTERM without one, so the saves measured are the full saves the game makes during play: two or three per run, each writing the whole ~300 MB file.

	main (3)	PR (3)
cache loaded	299,481 KB, every run	299,481 KB, every run
peak anonymous memory (smaps `Anonymous`)	1,407 / 1,302 / 1,241 → 1,302 MB	1,085 / 984 / 981 → 984 MB
peak `RssAnon` (status, 250 ms; added from the 4th run on)	1,226 MB (run 3)	984 / 981 MB (runs 2, 3)
peak `VmHWM`	8,085 / 8,105 / 8,095 → 8,095 MB	8,064 / 8,100 / 7,856 → 8,064 MB
start → first frame of play	117 / 107 / 104 → 107 s	109 / 96 / 110 → 109 s
disk written (`write_bytes`) by 2 s after the cache line	63 / 59 / 66 MB	118 / 111 / 114 MB
first save: temporary file's life	1.0 / 20.0 / 5.6 s	37.2 / 35.8 / 37.1 s
later saves: temporary file's life	≤ 0.1 s	17.2 / 11.5 / 19.6 s
frames > 100 ms (last 60% of each run)	17 / 12 / 14	12 / 11 / 13

Peak anonymous memory is ~320 MB lower with the PR, about the cache's size. VmHWM is level, as expected, since it counts the mapped file.
The cache read itself writes nothing: write_bytes shows no step when the cache loads. The PR's higher total by that point builds up steadily over the ~10 s before the load, and I haven't traced which files those writes go to.
Save time isn't like for like. On the PR the temporary exists while the driver writes straight into it (vkGetPipelineCacheData into the mapping, then msync). On main, .tmp only appears for the fwrite after the data is already in a heap buffer. So the PR's 12-37 s is the whole save, and main's ~0-20 s is only its last part. The saves run on the host thread; play hitches were the same in both.

Robustness (PR):

Killed during a save: SIGKILL 24 ms after mkw_vk_pipelines.bin.tmp-nzZran appeared.
- Left behind: the original mkw_vk_pipelines.bin (306,668,920 bytes, unchanged) and mkw_vk_pipelines.bin.tmp-nzZran (306,678,152 bytes, preallocated to full size).
- The next start was clean: the cache loaded at 299,481 KB and saved normally three times. The orphan was still there after that start and its exit.
- After I set its date back 2 days, the next start logged vk: pipeline cache: 1 old temporary files removed, and it was gone.
Truncated to half: loaded as 149,740 KB (the driver keeps what is intact), no crash, and the next saves wrote a whole file (158,119 KB). main does exactly the same.
Read-only game folder (where the cache lives): the cache loaded, play ran the full 150 s with no crash, no save was made and nothing was logged about it, the original file was unchanged, and no temporary was left.
Cross-loading: the PR's saved file loads on main (303,349 KB), and main's loads on the PR (303,349 KB).
Full disk: the game folder on a throwaway 400 MB tmpfs in a private mount namespace, holding the 307 MB cache with 110 MB free; ~4 min of play each.
- PR: no SIGBUS and no crash. Nothing was saved; usage stayed at 299,484 KB the whole time, and no temporary was ever seen in the 2 s samples, or left at the end.
- main: wrote mkw_vk_pipelines.bin.tmp until the filesystem was full (112,758,784 bytes, 0 bytes free), and left it there for the rest of the run. No crash either.
No crash, hang, device loss or error in 14 runs plus the two full-disk runs.

> [@hckrman101](https://github.com/hckrman101) commented on GitHub · 2026-11-12 18:05 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6831810988) Test request T-U8LS: pass Hardware: AMD Radeon RX 580 8 GB (RADV Polaris10), Mesa 26.2.6, CachyOS Linux (btrfs), 8 threads, 15 GB RAM. Built b0cc8bdb (the PR head) against main 51da0464, its base. Both are the same game build, with only `hookshot_vk.so` from the PR. What I checked: every run started from the same 307 MB `mkw_vk_pipelines.bin`, a large cache from an earlier run on this card and driver, copied back in before each run. The run loads the test save and plays Kokiri for 150 s (headless, `RX_VK_PRECOMPILE=0`). PR and main alternated, 3 runs each. `/proc/<pid>/status` and `/proc/<pid>/io` were sampled every 250 ms, the folder's cache temporaries every 50 ms, and smaps every 2 s. Exit save: a headless game ends on SIGTERM without one, so the saves measured are the full saves the game makes during play: two or three per run, each writing the whole ~300 MB file. | | main (3) | PR (3) | |---|---|---| | cache loaded | 299,481 KB, every run | 299,481 KB, every run | | peak anonymous memory (smaps `Anonymous`) | 1,407 / 1,302 / 1,241 → **1,302 MB** | 1,085 / 984 / 981 → **984 MB** | | peak `RssAnon` (status, 250 ms; added from the 4th run on) | 1,226 MB (run 3) | 984 / 981 MB (runs 2, 3) | | peak `VmHWM` | 8,085 / 8,105 / 8,095 → 8,095 MB | 8,064 / 8,100 / 7,856 → 8,064 MB | | start → first frame of play | 117 / 107 / 104 → 107 s | 109 / 96 / 110 → 109 s | | disk written (`write_bytes`) by 2 s after the cache line | 63 / 59 / 66 MB | 118 / 111 / 114 MB | | first save: temporary file's life | 1.0 / 20.0 / 5.6 s | 37.2 / 35.8 / 37.1 s | | later saves: temporary file's life | ≤ 0.1 s | 17.2 / 11.5 / 19.6 s | | frames > 100 ms (last 60% of each run) | 17 / 12 / 14 | 12 / 11 / 13 | - Peak anonymous memory is ~320 MB lower with the PR, about the cache's size. `VmHWM` is level, as expected, since it counts the mapped file. - The cache read itself writes nothing: `write_bytes` shows no step when the cache loads. The PR's higher total by that point builds up steadily over the ~10 s before the load, and I haven't traced which files those writes go to. - Save time isn't like for like. On the PR the temporary exists while the driver writes straight into it (`vkGetPipelineCacheData` into the mapping, then `msync`). On main, `.tmp` only appears for the `fwrite` after the data is already in a heap buffer. So the PR's 12-37 s is the whole save, and main's ~0-20 s is only its last part. The saves run on the host thread; play hitches were the same in both. Robustness (PR): - **Killed during a save**: SIGKILL 24 ms after `mkw_vk_pipelines.bin.tmp-nzZran` appeared. - Left behind: the original `mkw_vk_pipelines.bin` (306,668,920 bytes, unchanged) and `mkw_vk_pipelines.bin.tmp-nzZran` (306,678,152 bytes, preallocated to full size). - The next start was clean: the cache loaded at 299,481 KB and saved normally three times. The orphan was still there after that start and its exit. - After I set its date back 2 days, the next start logged `vk: pipeline cache: 1 old temporary files removed`, and it was gone. - **Truncated to half**: loaded as 149,740 KB (the driver keeps what is intact), no crash, and the next saves wrote a whole file (158,119 KB). main does exactly the same. - **Read-only game folder** (where the cache lives): the cache loaded, play ran the full 150 s with no crash, no save was made and nothing was logged about it, the original file was unchanged, and no temporary was left. - **Cross-loading**: the PR's saved file loads on main (303,349 KB), and main's loads on the PR (303,349 KB). - **Full disk**: the game folder on a throwaway 400 MB tmpfs in a private mount namespace, holding the 307 MB cache with 110 MB free; ~4 min of play each. - PR: no SIGBUS and no crash. Nothing was saved; usage stayed at 299,484 KB the whole time, and no temporary was ever seen in the 2 s samples, or left at the end. - main: wrote `mkw_vk_pipelines.bin.tmp` until the filesystem was full (112,758,784 bytes, 0 bytes free), and left it there for the rest of the run. No crash either. - No crash, hang, device loss or error in 14 runs plus the two full-disk runs.

github-sync commented

2026-11-12 19:01:48 +00:00

Owner

@hckrman101 approved on GitHub · 2026-11-12 18:27 UTC · original

I merged main into the branch as 9f4ad43a, a fast-forward on top of your b0cc8bdb, so nothing was rewritten. There were three small conflicts:

pcache_init: main's stale-stamp drop (#2684) runs before the read, and the read stays file-backed. The temporary sweep still runs first.
pipeline_play_cache_test.cpp: your mapped_move shims are kept, plus main's census counters that the play-cache code now needs.
docs/worklist.md: both rows are kept, in score order.

The pipeline cache tests and the full suite pass on Linux, apart from failures main has too. The --vk-only build passes.

All the earlier findings are fixed (see my last review). The Linux evidence is in now too: our RX 580 run (#2649) took peak anonymous memory from 1,302 to 984 MB. Start-to-play time and long frames stayed level. The full-disk case had no SIGBUS and left nothing behind, where main leaves a 112 MB .tmp. The truncated cache, the read-only folder and cross-loading all behaved. Together with dankman42's Windows run, that covers both platforms. Approving.

Optional nits:

The 24 h age on the temporary sweep could come down to about an hour. Our kill-mid-save test left a 307 MB orphan that survived a clean start, because it wasn't a day old yet. The writer's lock already protects a live save. Related: since main's stamp check (pcache_stamp::is_cache_file) only knows the old .bin.tmp name, a .bin.tmp-XXXXXX leftover also survives a translator change until the sweep's age passes.
About 50 MB more was written at startup, before the cache read, than on main. We didn't trace it. If you know where it comes from, a line in the description would help.

It's still a draft. Please mark it ready for review when you're happy with it.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 18:27 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#pullrequestreview-5879871382) I merged main into the branch as `9f4ad43a`, a fast-forward on top of your `b0cc8bdb`, so nothing was rewritten. There were three small conflicts: - `pcache_init`: main's stale-stamp drop (#2684) runs before the read, and the read stays file-backed. The temporary sweep still runs first. - `pipeline_play_cache_test.cpp`: your `mapped_move` shims are kept, plus main's census counters that the play-cache code now needs. - `docs/worklist.md`: both rows are kept, in score order. The pipeline cache tests and the full suite pass on Linux, apart from failures main has too. The `--vk-only` build passes. All the earlier findings are fixed (see my last review). The Linux evidence is in now too: our RX 580 run (https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2649) took peak anonymous memory from 1,302 to 984 MB. Start-to-play time and long frames stayed level. The full-disk case had no SIGBUS and left nothing behind, where main leaves a 112 MB `.tmp`. The truncated cache, the read-only folder and cross-loading all behaved. Together with dankman42's Windows run, that covers both platforms. Approving. Optional nits: - The 24 h age on the temporary sweep could come down to about an hour. Our kill-mid-save test left a 307 MB orphan that survived a clean start, because it wasn't a day old yet. The writer's lock already protects a live save. Related: since main's stamp check (`pcache_stamp::is_cache_file`) only knows the old `.bin.tmp` name, a `.bin.tmp-XXXXXX` leftover also survives a translator change until the sweep's age passes. - About 50 MB more was written at startup, before the cache read, than on main. We didn't trace it. If you know where it comes from, a line in the description would help. It's still a draft. Please mark it ready for review when you're happy with it.

github-sync commented

2026-11-12 19:01:48 +00:00

Owner

@Arkex-ai commented on GitHub · 2026-11-12 18:50 UTC · original

Thanks — this covers the Linux runtime and full-disk checks that were missing. I added your results with the tested b0cc8bdb/51da0464 pair, keeping the anonymous-memory reduction separate from total residency and the save-timing limits. Thanks for the merge as well.

> [@Arkex-ai](https://github.com/Arkex-ai) commented on GitHub · 2026-11-12 18:50 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2649#issuecomment-6832847873) Thanks — this covers the Linux runtime and full-disk checks that were missing. I added your results with the tested `b0cc8bdb`/`51da0464` pair, keeping the anonymous-memory reduction separate from total residency and the save-timing limits. Thanks for the merge as well.

github-sync referenced this pull request

2026-11-12 19:02:49 +00:00

feat(menu): a Quality preset "Auto (this PC)" that picks settings from the hardware and tunes them in play #2850

Rows
Columns

perf(vk): persist pipeline caches without full-size heap copies #2649

Summary

Technical changes

Validation

Independent Linux validation