Skip to content

fix(shader): support long Windows cache file paths - #7462

Closed
AlexanderCalli wants to merge 1 commit into
boykopovar:mainfrom
AlexanderCalli:fix/windows-cache-long-paths
Closed

AlexanderCalli wants to merge 1 commit into
boykopovar:mainfrom
AlexanderCalli:fix/windows-cache-long-paths

Conversation

@AlexanderCalli

Copy link
Copy Markdown

What

Fix Windows shader and pipeline cache file I/O under long host paths. The shared atomic writer could fail for a 235-character destination once its temporary suffix crossed MAX_PATH; longer ordinary paths also failed on reads. Passing an already-extended path through the previous std::filesystem::absolute call was mishandled by the supported MinGW toolchain.

Resolve ordinary Windows paths with GetFullPathNameW, then use the extended namespace consistently for directory creation, the temporary stream, atomic replacement and reads. Preserve already-extended/device paths and map UNC paths to the extended UNC form. Keep the existing FILE_RENAME_INFO replacement and delete-sharing flags.

Contract: Microsoft long-path documentation and FILE_RENAME_INFO. This does not depend on a host executable's longPathAware manifest. No shader semantics or cache format changes.

Fixes #7452.

Tested

Windows x86-64, supported WinLibs GCC 15.2.0 r7, Release relinker-only build.

  • Built shader_cache_file_read_tests; its existing allocation-failure/read cases and new Windows filesystem cases pass. Three consecutive cold-process CTest runs pass in 0.50 seconds total; a final run also passes.
  • Negative control: compiled the expanded test against the unchanged main header; it fails with cannot write cache path, after passing the original five read cases.
  • Standalone real-helper probe: before the change, ordinary atomic writes at path lengths 235, 269 and 320 fail, and already-extended atomic writes fail even at 180. Afterward, ordinary/extended writes and ordinary reads succeed at 180, 235, 269 and 320.
  • Regression covers both aliases, long parent directories created by the helper, Unicode names, a relative path with .., missing-file reads, replacement while an old reader retains the original bytes, and absence of leftover temporary files.
  • UNC normalization was checked locally; no SMB share was available for end-to-end UNC I/O testing.
  • tools/check_conventions.py --base origin/main and git diff --check pass. tools/pr_overlap.py --branch fix/windows-cache-long-paths --repo boykopovar/AnyPS5 reports no overlap with open pull requests.
  • No full renderer build, GPU test, title execution or FPS improvement claimed.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: Fixes Windows shader and pipeline caches cannot persist under long host paths (PPSA99902) #7452
  • Unblocks: none
  • GPUs tested: none (host cache filesystem tests; no shader execution)
  • AI-assisted: yes

@github-actions github-actions Bot added shader core/shader fix error Closes an issue (Fixes #N) labels Oct 18, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 18, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks. Windows CI fails shader_cache_file_read with the default "Cache file read failed" (run 39112227465), after the five ordinary cases. Most likely it's the relative-path case: root comes from temp_directory_path(), which is on C: on GitHub's runner, while the test's current directory is the build tree on D:, so lexically_relative returns an empty path and Require(!relative.empty()) fails. Could you build the relative case from a directory on the same drive (for example set the current path to root for that case, or derive the path relative to root.root_path()), and give every Require in VerifyCachePaths its own message so a future failure names its check? The long-path cases themselves look fine. None of their explicit messages printed.

@Amoenus

Amoenus commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for picking up #7452. I independently validated 3df970aeb7 on main cb1d6e5bd6 with a test-only overlay; the cache production files are byte-identical to your head.

  • Your shader_cache_file_read and the existing agc_shader_disk_cache pass three fresh runs each. Nine additional host-only runs also pass, including a deterministic 259-character destination whose temporary suffix exceeds MAX_PATH, ordinary/extended/relative/dot paths, deep parents, Unicode, and retained-reader bytes.
  • An additional real cache API test still fails 3/3: a long-path cache entry was not persisted and mapped. It uses a 225-character cache root and a derived entry path over 260. It first verifies that the published bytes exactly equal EncodeEntry, so the shared writer has succeeded before the mapping failure.
  • MapFile still passes the ordinary long path to CreateFileW. Normalizing that call with your NativeCachePath_nid_no_patch gets persistence/mapped-SPIR-V round trips through, then exposes a second 3/3 failure: a long-path cache pack was rejected: cannot open. The pack constructor has the same remaining raw open.
  • Applying the same normalization to both mapped opens passes the new persistence/pack test and the existing two groups, three fresh runs each (9 passes; no skips/timeouts).

I pushed the test-only reproduction and the two-call-site correction on top of your exact work, preserving attribution. Focused comparison: four files, including the regression/registration; no replacement of your shared helper. You can take the correction/regression here, or it can follow your PR.

Validation of focused head 0a3621bbbf is now complete: full Windows and Linux builds pass with SPIRV-Tools ON. The selected repeated matrix has 51 passes and one explicit Linux skip: NVIDIA 9, AMD 9, Windows CPU 15, Linux 18; the Linux pipeline-file fixture reports that lavapipe does not retain pipelines in a pipeline cache. All executed tests finish within 2.07 s. The GPU fixtures select RTX 5080 (10de:2c02, driver 610.62 / 32.0.16.1062) and Radeon(TM) Graphics (1002:13c0, driver 32.0.21036.18), Windows11 Enterprise26200; Vulkan validation is loaded with no validation warnings/errors in those selected runs. Linux uses Ubuntu24.04, GCC13.3 and lavapipe/Mesa25.2.8. Cppcheck then clang-tidy cover the two changed production translation units and the cache fixture: no clang-tidy diagnostics, one Cppcheck portability advisory on an unchanged serializer line also present in the parent; no parser/configuration errors. This is a selected matrix, not the full CTest suite. The original parent also needs the independently reproduced cross-drive fixture correction below; local same-drive success does not address that CI failure. This deeper-directory mapping case is additional coverage: the original Sonic replay had 254-character shader entries and 269-character pipeline files. It is not established as the cause of that replay's startup time. No performance claim.

Ordinary title validation is now complete: integration e4c6c8f970 combines main 9caecfd, your exact head, our mapped follow-up and the cross-drive fixture correction. A bounded180s Sonic/PPSA99902 replay at the original long host path reaches Continue/opening cinematic and records 60 shader writes,60 resource-plan writes, zero load/write failures;124 cache files (26,452,618 bytes) are archived, including the269-character pipeline-cache file. A second90s replay with that archived application cache records 56 shader hits, zero misses/writes/load failures, and loads5461.2KiB from the pipeline-cache file. Both runs have zero logged VUIDs, validation warnings/errors or guest fence failures in these intervals. All127 runtime-library hashes and145 restored prior files (including3 prior cache files), plus protected originals and launch/config hashes, independently verify for each run. Original executable/guest module/config/assets unchanged; copied archived save, no field edits. This validates persistence/readback in the composed title runtime; it is not an isolated performance comparison or a new gameplay-completion claim. Full source pins and limits.

AI-assisted: yes, Codex.

@Amoenus

Amoenus commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Confirmed the cross-drive diagnosis in #7462 (comment): with a C: temporary directory and a Z: working-directory alias, unchanged 3df970aeb7 fails3/3 after 5 cache file read cases passed, printing the default Cache file read failed.

Focused fixture correction de74182891 sets the current directory to the temporary root for the relative case, restores it through an RAII guard, and gives every Require in VerifyCachePaths a descriptive message. No production changes. It passes3/3 same-drive and3/3 split-drive runs (all under0.14s), with the original Windows path/replacement checks intact. Z: was a temporary local subst alias, removed afterward; no second physical volume or SMB claim.

This is separate from the mapped-cache follow-up above and can be taken directly into this PR. WinLibs GCC15.2.0 r7, optimized static Windows executable. AI-assisted: yes.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Amoenus's report checks out: on 3df970a MapFile (ShaderDiskCache.cpp:198) and CachePack (ShaderCachePack.cpp:43) still open the raw path, so a long-path entry is written but never mapped. Could you take their 0a3621bbbf (two call sites through NativeCachePath_nid_no_patch) and the 83ad998790 regression, together with the relative-path test fix, in one push? PipelineJournal.cpp:348 and PipelineCache::readHeader open plain paths too, so either convert them or note them in the debt file.

@LuisBiceps

Copy link
Copy Markdown
Contributor

Besides the four sites above, these still take the plain path on 3df970a:

  • The rest of the journal's file calls: exists (:467, :650), file_size (:764, :781), resize_file (:768), the append stream (:794) and validTail's read (:834). With only the lock at :348 converted, every append still fails with cannot open the journal for writing.
  • exists in PipelineCache::readFile (PipelineCache.cpp:96), so an unreadable long-path file is reported missing.
  • Housekeeping's last-used stamp (ShaderDiskCache.cpp:1597-1599), its removal of stale .tmp. files (:1616, :1619), and the prebuilt status checks (:1431-1432, :1444).

A caution for the directory listings (:1435, :1601 with its remove_all at :1609, :1613). On Windows 10 with WinLibs GCC 15.2.0 r7, std::filesystem::directory_iterator on an ordinary 270-character directory path fails with ENOENT. On the same directory in \\?\ form, it returned the entries of the process's working directory as children of the long path, and remove_all on it did not return. At 200 and 250 characters both forms list correctly. So these are safer left on the plain path, where a directory that long is skipped; that limit could go in the debt file note. On main they already receive an ANYPS5_SHADER_CACHE_DIR given in \\?\ form unchanged (:51).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks @LuisBiceps, that matches what I see on 3df970a. Adding to the earlier ask:

  1. File operations under the cache root: please convert, or scope them in the debt note. These still take the plain path:

    • The journal's file calls: exists (PipelineJournal.cpp:467, :650), file_size (:764, :781), resize_file (:768), the append stream (:794) and validTail's read (:834). With only the lock at :348 converted, a journal under a long root is never written ("cannot open the journal for writing"). That is the same class of failure as the mapped reads, so I'd convert these with NativeCachePath_nid_no_patch like the rest.
    • PipelineCache::readFile's exists (PipelineCache.cpp:96): a long-path file is reported missing.
    • Housekeeping's single-file calls: the last-used stamp (ShaderDiskCache.cpp:1597-1599), the .tmp. remove (:1616, :1619) and the prebuilt status checks (:1431-1432, :1444). These are safe to convert because they act on one known file each.
  2. Directory listings: please keep them on the plain path, and guard the \\?\ case. LuisBiceps measured on WinLibs GCC 15.2 that directory_iterator over a \\?\ directory of about 270 characters lists the process's working directory as its children, and that remove_all on it didn't return. Housekeeping iterates the cache root (:1601) and calls remove_all (:1609) on old 16-hex-digit subdirectories, so a \\?\ root could make it act on entries that aren't in the cache, or hang at startup. So:

    • leave the iterators at :1435, :1601 and :1613 (and the remove_all) on plain paths, where a directory that long simply fails with ENOENT and is skipped;
    • since ANYPS5_SHADER_CACHE_DIR given in \\?\ form already reaches them unchanged on main (ShaderDiskCache.cpp:51), strip a leading \\?\ (and \\?\UNC\) from the root before housekeeping, or skip housekeeping for a prefixed root;
    • note the ~260-character directory limit for listings/eviction in the debt entry.

A test for item 2 would be cheap: housekeeping with a \\?\ root that contains one stale 16-hex-digit directory must not touch anything outside the root and must return.

@Amoenus

Amoenus commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

@AlexanderCalli @oneandonlydean Confirmed the PipelineCache::readHeader point with a focused regression and correction: 8d4b4914f0, based on main 0d078f75d3 plus your exact 3df970aeb7. It reuses your native-path helper for this reader and adds the required shader-cache reviewed-name entry (host file I/O, no shader-output input).

  • The test-only control fails 3/3 on RTX5080 and 3/3 on Radeon Graphics: an unchanged long-path pipeline cache was reloaded for an idle save. Reverting just the reader on the final fixture reproduces the same failure on each GPU. The diagnostic AMD control counted cache creations rising1→4 across three idle saves.
  • With the correction: full Windows and Linux relinker/library builds pass. Three cache groups pass3 runs per physical GPU (18 Windows passes); lavapipe passes the header and key/lifetime groups3 times each (6 passes), with the existing file-merge test explicitly skipping because that driver does not retain pipelines. All tests finish below30s. Windows Khronos1.4.363 loaded, no logged VUIDs/errors/warnings in these checks. Windows11 26200; NVIDIA driver32.0.16.1062, AMD32.0.21036.18. Linux Ubuntu24.04, GCC13.3, lavapipe Mesa25.2.8/LLVM20.1.2; no Linux validation-layer claim.
  • The fixture keeps directory listings below260 characters (long parent220–249); only the cache filename exceedsMAX_PATH. Its cleanup root is normalized, with a bounded temporary-root length. It does not exercise the reported270-character directory-listing case.

The focused commit is available to take into this PR alongside the already-requested follow-ups. This establishes redundant reloads, not a measured game speedup. Cppcheck and clang-tidy completed; clang-tidy has no selected diagnostics, while Cppcheck reports45 diagnostics outside the added ranges in the existing recorder fixture. No clean-analysis claim. The running Sonic composition does not yet include this reader correction. AI-assisted: yes.

@savvaniss

Copy link
Copy Markdown
Contributor

Windows data: shader_cache_file_read fails on main and passes with this PR, on Windows 11 Pro 26300 with WinLibs GCC 15.2.0 r7, Release.

shader_cache_file_read, ×3 Result
This head 3df970aeb merged into main f3059421f 3 of 3 pass
This PR's CacheFileReadTests.cpp on main f3059421f, with none of its other changes fails 3 of 3: 5 cache file read cases passed, then cannot write cache path

Full CTest (--parallel 4, 1513 tests) with this head merged, on an Intel UHD 770 (driver 32.0.101.7088) and an AMD RX 9070 XT (driver 32.0.31041.1004), fails the same tests as main alone on this machine, none of them in the shader cache. I also ran full CTest with this PR and the 14 other open AGC PRs I tested in the same batch merged together (#7647, #7641, #7630, #7627, #7603, #7600, #7591, #7579, #7535, #7533, #7474, #7050, #7048, #7650). Neither GPU showed a failure beyond main's own, except #7650's new test.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 20, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thank you for this. #7933 has now merged (d14a186) and carries this change, with your commits cherry-picked so the credit stays yours, together with the MapFile/CachePack long-path fix. So I'm closing this one as covered by #7933.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix error Closes an issue (Fixes #N) needs rebase Conflicts with main, rebase needed shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows shader and pipeline caches cannot persist under long host paths (PPSA99902)

6 participants