Skip to content

fix(agc): read the pipeline cache header through the long-path form on Windows - #8554

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
aarushkha:fix/pipeline-cache-header-long-path
Oct 21, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
aarushkha:fix/pipeline-cache-header-long-path

Conversation

@aarushkha

@aarushkha aarushkha commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What

PipelineCache::readHeader opened the cache file with a plain std::ifstream(path). readFile (through ShaderRecompiler::ReadWholeFile) and the atomic writer already resolve the path with NativeCachePath_nid_no_patch. On Windows, past MAX_PATH, the header read fails while the full read and write succeed. save then compares a missing on-disk header with settledFile, treats the file as changed, and re-reads and merges it on every save. This is the remaining header reader confirmed in #7452.

On Windows the header is now read through the extended path, as PipelineJournal does for its directory. If the path cannot be resolved, it returns std::nullopt, matching a failed open. readFile had the same gap in its fallback: when the full read fails it calls std::filesystem::exists to tell a missing file from an unreadable one, and used the plain path. Past MAX_PATH an existing file was reported missing instead of unreadable, so save could write over it. It now checks the extended path too. This was listed in the review of #7462 and is not part of #7933 or open #7945. Linux and macOS are unchanged.

Tested

Windows 10 Pro (10.0.19045), WinLibs GCC 15.2.0 posix UCRT r7, Release, full build with -DBUILD_TESTING=ON, NVIDIA GeForce RTX 3060 (driver 617.42). Both commits applied to main 83751168dd (they cherry-pick cleanly; this branch's own base is older). PipelineCache.cpp compiles with no warnings.

ctest --test-dir build -R "^agc_pipeline_cache(_file)?$" with ANYPS5_REQUIRE_VULKAN=1 and ANYPS5_NO_SHADER_CACHE=1:

Tree agc_pipeline_cache agc_pipeline_cache_file
main 83751168dd passed passed
main + this PR passed 3 of 3 passed 3 of 3

These tests write the cache under a short temporary path, so they check that the change does not break the ordinary path. They do not exercise a path past MAX_PATH. I tried that by pointing TEMP at a 196-character directory, so the cache file lands past MAX_PATH. On main, agc_pipeline_cache_file then hangs rather than failing. That matches the housekeeping hang on long extended roots that #7945 addresses, so this PR's long-path behaviour could not be measured that way, and no long-path regression is added here.

Not run: the Khronos validation layer (VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation), because no Vulkan SDK is installed on this machine; Linux; a title.

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: none (the remaining PipelineCache::readHeader gap from Windows shader and pipeline caches cannot persist under long host paths (PPSA99902) #7452)
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3060, driver 617.42, Windows 10 Pro 19045 (host file I/O only; without the validation layer)
  • AI-assisted: yes

🤖 Generated with Claude Code

aarushkha and others added 2 commits October 20, 2026 21:53
…n Windows

PipelineCache::readHeader opened the cache file with a plain path while
readFile and the writer already use NativeCachePath_nid_no_patch. Past
MAX_PATH the header read failed, so every save saw the file as changed
and merged it again (boykopovar#7452).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…th form

When the full read fails, readFile asked std::filesystem::exists with the
plain path, so on Windows a cache file past MAX_PATH was reported missing
instead of unreadable and could be saved over.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Amoenus

Amoenus commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Current-main integration follow-up, 2026-10-21 UTC

#8554 is merged as 042504ea2820a97e6add8c89829ea65e4244e7a1. We have now adapted the previously offered header regression to the version-directory layout: fixture adjustment 2e844131db. It enumerates the current 16-digit source-version directory, keeps that listing below MAX_PATH, requires the actual cache filename beyond MAX_PATH, and retains persisted-byte equality plus the real cache-creation counter across three idle saves.

The old fixture first failed because it expected a cache file directly under the configured root; that failure is retained. After the fixture correction, agc_pipeline_cache_header passes 3/3 fresh serial processes on each physical GPU: RTX 5080 and AMD Radeon(TM) Graphics on Windows 11 Enterprise 26200. Required Vulkan and the loaded Khronos 1.4.363 layer; no skips, timeouts, VUIDs or validation warnings/errors. The two neighboring ordinary cache groups also pass 3/3 per GPU: 18 passing GPU processes total.

Scope: experimental integration 2e844131db on main 7d7b097407a0d5d1ddd243d98f0ddc5d9cbc5420, with other title-bring-up changes. Its PipelineCache.cpp is byte-identical to that main; this is not an isolated run of #8554's former head or a literal-main whole-build result. The integration's full Windows build and explicit libraries build pass. No new negative-control run, failed-read/existence fault injection, title-speed or console claim. No production change or competing PR here. The earlier control and authorship/proposal history remain below.

AI-assisted: yes (Codex); author responsible.

Original regression offer and historical controls

@aarushkha there is an existing focused regression for the header half that you can reuse: 8d4b4914f0, offered in #7462. Its agc_pipeline_cache_header mode keeps the directory at 220–249 characters and makes only the filename cross MAX_PATH, so it does not need an oversized TEMP. It checks exact persisted bytes and counts real cache creations: three idle saves must neither reload the unchanged cache nor alter the bytes.

The recorded test-only control failed 3/3 on RTX 5080 and 3/3 on Radeon Graphics; the corrected old composition passed on both with Khronos validation. Those results are pinned to main 0d078f75d3 + #7462 3df970aeb7 + the offered commit, not this PR's current head. Full details and limitations are in that earlier report. The fixture needs adaptation to current main and cannot substitute for a regression of your separate failed-read/existence check.

I will not open a competing header-reader PR. Please preserve original authorship if you take the fixture; your existence-check correction remains your separate contribution. AI-assisted: yes (Codex); author responsible.

@oneandonlydean
oneandonlydean merged commit 042504e into boykopovar:main Oct 21, 2026
6 checks passed
@Queuse Queuse mentioned this pull request Oct 21, 2026
12 tasks done
@Queuse

Queuse commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Follow-up regression coverage is in #9013, 8b01446efc, main a6dcea9279; original fixture authorship preserved. It checks long cache filenames while keeping the directory below MAX_PATH, idle saves without reload, and locked-file diagnostics/byte retention.

Current baseline passes both existing cache cases; candidate passes all 3 (Windows 11, RTX 3050 Laptop/617.42, GCC 15.2.0-r7), no skips, missing validation layer or VUIDs. Independent header/existence controls on 27c88a8d75 each fail their expected assertion; restoring passes 3/3. No Linux/title/full-suite claim. AI-assisted: yes.

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

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants