Skip to content

fix(agc): merge the pipeline cache file instead of overwriting it with one device's cache - #4915

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
fdfawef324:fix/agc-pipeline-cache-one-writer
Oct 14, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
fdfawef324:fix/agc-pipeline-cache-one-writer

Conversation

@fdfawef324

@fdfawef324 fdfawef324 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

A process holds two Graphics::PipelineCache objects for one cache file. Driver::AttachWindow keeps the replaced headless VulkanDevice in replacedDevices and creates the windowed one. Each device builds its own PipelineCache in its constructor, and both resolve to the same vk-pipelines-<vendor>-<device>-<driver>-<uuid>.bin. On main, PipelineCache::save wrote the whole file from its own VkPipelineCache alone. It ran from the periodic run() thread every 10 s and once more from the destructor. It wrote whenever the cache size differed from savedBytes, which is the size this object last loaded or wrote. Nothing checked what the other object had stored, so the last writer won.

Evidence from Demon's Souls (PPSA01342) runs on this machine, from the [pipeline-cache] log lines:

  • cold1: one object loaded 106798.4 KiB and saved 107111.6 KiB and then 107582.1 KiB. The second object started empty: its load printed nothing, because main's load is silent when the file cannot be read, so the logs do not say why. From then on, the second object's periodic saves replaced the file with its own cache, growing from 2054.1 KiB to 12558.7 KiB. The first object's cache stopped growing after the device was replaced, so its size matched savedBytes and it never wrote again. The process then crashed, so neither teardown save ran.
  • warm1: both objects loaded 12558.7 KiB, which is the last size cold1 wrote. The 107582.1 KiB file written earlier in cold1 was gone.
  • warm2: two loaded 31289.8 KiB lines at start, then saves of 31651.1 KiB and 32172.1 KiB.

The fix keeps one VkPipelineCache per device and changes how a save writes the file:

  • Writers in one process are serialised by a mutex keyed by the file path. The load at start takes the same mutex.
  • A save reads the file and validates it exactly as the load does. This covers the wrapper header, the checksum, and the Vulkan header's vendor, device and UUID. If the file is valid, the save creates a temporary cache from it and calls vkMergePipelineCaches with the temporary cache as destination and this object's cache as source. It then writes the merged data with the existing atomic write.
    • The brief asked to merge into this object's own cache instead. The Vulkan spec requires host access to dstCache to be externally synchronised, and worker threads create pipelines with the live cache at any time. So the live cache is only ever a merge source.
  • If the merge added nothing (the merged size equals the file's), the file is left alone. Without this, two objects would keep rewriting each other's file every 10 s.
  • A file that is not a valid cache is replaced. This covers another format, a truncated file, a bad checksum, another device or driver, and a file the driver refuses to load. The load ignores the same files.
  • If the file exists but cannot be read, the save does not write over it. The load now prints cannot read instead of starting empty silently.
  • The save does not write a result smaller than the file on disk or the data loaded at start, unless the file was invalid.
  • If the cache size and the file's 24-byte header have not changed since the last save, the save stops before reading the whole file.

Behaviour change: a save that has something to write now also reads the file and has the driver parse it. The saved ... in N ms line includes that time and says (merged with the file) when a merge happened. No speed claim is made.

Not covered: two AnyPS5 processes sharing one cache directory. The atomic write keeps the file valid, but one process can still drop pipelines the other wrote between its read and its rename.

Related

#4795 (MarcZX100, open) serializes PipelineCache::save()'s vkGetPipelineCacheData calls against pipeline creation with a shared/exclusive lock, as a workaround for the Intel crash in #4781. It conflicts with this PR in Graphics/src/PipelineCache.cpp, but the two are complementary: #4795 decides when the live cache may be read, this PR decides what is written to the file. Merge #4795 first. This PR will then be rebased onto its device-cache guard, preserving exclusive access for the initial live-cache size read and the merge that reads the live cache; reads of the temporary merged cache and file I/O stay outside that guard. The file-path mutex still covers the file read/merge/write transaction.

Tested

Windows Server 2025, RTX 3060, GCC 15.2, build-dev (Ninja).

  • New test agc_pipeline_cache_file (agc_driver_recorder_tests --pipeline-cache-file-only). Two PipelineCache objects on the same path each build a different compute pipeline (SampleLod.comp, SampleArray.comp) and are destroyed in both orders. A third object loads the file, and each pipeline is built into its cache again. vkGetPipelineCacheData must report the same size afterwards, which means the pipeline was already in the cache. The test also checks two more cases:

    • An object that adds nothing leaves the file's bytes unchanged.
    • A corrupt file is replaced with the saving object's pipelines.

    It exits with 77 if the driver keeps nothing in a pipeline cache. VK_PIPELINE_CREATE_FAIL_ON_PIPELINE_COMPILE_REQUIRED_BIT is not used, because the test device does not enable pipelineCreationCacheControl.

    • Red on main with only the test applied: AGC graphics: the cache file lost the pipeline of the object that saved first. The objects saved 6.2 KiB and 6.4 KiB, and the third object loaded 6.4 KiB.
    • Green with the fix: the second save wrote 12.6 KiB (merged with the file) in both orders. It passed 10 of 10 runs with --repeat until-fail:10.
  • Every ctest registration of the touched executables passed, 26 tests in all:

    • all 17 tests of agc_driver_recorder_tests, including agc_pipeline_cache
    • agc_shader_disk_cache
    • the VulkanDevice tests agc_buffer_pool, agc_compute_pipeline_capture, agc_driver_buffer_load_store, agc_driver_cmask_fast_clear, agc_driver_copied_writer_copy, agc_driver_d16_memory, agc_driver_depth_plane_guest_layers and agc_driver_draw_offset_indices
  • python -I tools/check_test_registration.py build-dev: 741 test executables, none built and not run.

  • Not run: the other VulkanDevice test executables of libSceAgcDriver, lavapipe, Linux, and any title with this change. The floor that refuses a smaller result is not reached by any test, because merges only add entries on the tested driver.

  • Open PR fix(agc): serialize pipeline cache snapshots #4795 ("fix(agc): serialize pipeline cache snapshots") implements the separate device-cache synchronization. Its diff was inspected; this PR depends on it and must preserve its guard after rebase.

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: fix(agc): serialize pipeline cache snapshots #4795
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3060 12 GB, driver 591.86 (32.0.15.9186), Vulkan 1.4.325, Windows Server 2025 10.0.26100
  • AI-assisted: yes

🤖 Generated with Claude Code

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared this save path with #4795 and issue #4781. The pathMutex here serializes file access between devices, but it does not synchronize either vkGetPipelineCacheData call in save() with concurrent vkCreate*Pipelines calls on the same live cache. #4795 adds that device-cache synchronization for Intel and its test exercises snapshot-vs-pipeline use. Could you clarify whether #4915 should depend on #4795 or include equivalent synchronization? Otherwise the file merge can still hit the exact Intel saver race reported in #4781.

@fdfawef324

Copy link
Copy Markdown
Contributor Author

You are right that pathMutex only serializes access to the file between the process's cache objects. It does not serialize reads of a live Vulkan cache against pipeline creation using that cache.

I propose #4795 first and have updated the description to Depends on: #4795. Once its device-cache guard lands, I will rebase this file-merge change onto it and preserve that guard for the initial live-cache size read and the merge that reads the live cache. Reads of the temporary merged cache and the file I/O can stay outside the device-cache guard. The file mutex still covers the read/merge/write transaction so two cache objects cannot overwrite one another's entries.

The current RTX 3060 tests cover the file-loss bug. They do not reproduce or validate the Intel saver crash; I will keep that distinction in the description.

AI-assisted: yes.

@fdfawef324
fdfawef324 force-pushed the fix/agc-pipeline-cache-one-writer branch from 0c07a99 to 2c62442 Compare October 13, 2026 18:11
@MarcZX10

Copy link
Copy Markdown
Contributor

Thanks for confirming the ordering and updating the dependency. I agree #4795 should land first: its device-cache guard needs to cover the live-cache reads and merge, while the file mutex continues to protect the file transaction between cache objects. I also agree the RTX 3060 validation should stay distinct from the unreproduced Intel saver crash. I’ll review the combined result after the rebase.

AI-assisted: yes; reviewed by MarcZX100.

@fdfawef324
fdfawef324 force-pushed the fix/agc-pipeline-cache-one-writer branch 4 times, most recently from b0ffde6 to d495299 Compare October 14, 2026 00:58
@MarcZX10

Copy link
Copy Markdown
Contributor

Update: #4795 has merged, so the device-cache synchronization you listed as a prerequisite is now on main. This PR currently reports a conflict with main. Please rebase and retain the guard for the live-cache size/read and merge-source access; the path mutex still covers the file transaction between cache objects.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@fdfawef324
fdfawef324 force-pushed the fix/agc-pipeline-cache-one-writer branch from d495299 to f2efd60 Compare October 14, 2026 02:49
@fdfawef324

Copy link
Copy Markdown
Contributor Author

Rebased onto main 0df6e295c after #4795 merged, at f2efd60e1. The live-cache size query, live merge-source access, and the live-source size/data read retain PipelineCacheSynchronization::LockForSnapshot(). The path mutex still covers the file read/merge/write transaction. Reads of the private temporary merged cache and file I/O stay outside the live-cache guard.

The combined source was independently checked against the complete expected merge tree, with original authors and commit messages preserved. This update is source-only; the earlier RTX 3060 file-loss tests do not validate or reproduce the Intel saver crash. Thank you for reviewing the guard placement.

AI-assisted: yes.

Co-Authored-By: Codex noreply@openai.com

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 14, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@fdfawef324
fdfawef324 force-pushed the fix/agc-pipeline-cache-one-writer branch from f2efd60 to 446356d Compare October 14, 2026 20:44
…h one device's cache

The driver keeps the replaced headless VulkanDevice alive next to the
windowed one, so a process holds two Graphics::PipelineCache objects for
the same vk-pipelines-*.bin. Each saved its own VkPipelineCache over the
whole file, periodically and at teardown, so the last writer dropped the
other object's pipelines and those of earlier runs.

A save now reads the file under a per-path mutex, validates it as the load
does, creates a temporary cache from it and merges this object's cache into
that temporary one, then writes the result atomically. Merging into the
temporary cache keeps the live cache free of the external synchronisation
vkMergePipelineCaches requires of its destination. A merge that adds
nothing leaves the file as it is, an unreadable file is not written over,
and a result smaller than the file on disk or the data loaded at start is
not written; an invalid file or one the driver refuses is replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fdfawef324
fdfawef324 force-pushed the fix/agc-pipeline-cache-one-writer branch from 446356d to 8c8cd86 Compare October 14, 2026 20:54
@fdfawef324

Copy link
Copy Markdown
Contributor Author

Rebased onto main 4e210a106 at 8c8cd86cf. It also merges cleanly with the current main 7289938ce.

Conflicts:

  • The only conflict was in the include block of core/libs/prx/libSceAgcDriver/tests/Recorder.cpp. Main still includes <random>, so the PR does not add it again. The revert of test(agc): register the full recorder run with CTest #5211 removed <filesystem> from main, so the PR adds <filesystem> and <fstream> back for the new test.
  • core/libs/prx/libSceAgcDriver/CMakeLists.txt merged without conflicts. The agc_pipeline_cache_file registration and its SKIP_RETURN_CODE/PATH properties sit next to agc_pipeline_cache, as before.
  • PipelineCache.hpp and PipelineCache.cpp applied unchanged, because main has not touched them since the last rebase. git range-diff against the previous head shows only the include change above.

Guard: PipelineCacheSynchronization::LockForSnapshot() is still held for three things:

  • the live-cache size query
  • vkMergePipelineCaches with the live cache as merge source
  • the size and data read when the live cache is the source, which happens when there is no valid file

The path mutex still covers the file read, merge and write. Reads of the private temporary merged cache and the file I/O stay outside the guard.

Tests: Windows Server 2025 10.0.26100, NVIDIA GeForce RTX 3060 12 GB with driver 591.86 (from nvidia-smi), GCC 15.2, build-dev Release (-O3). Built libSceAgcDriver and agc_driver_recorder_tests.

  • agc_pipeline_cache_file passes. With agc_pipeline_cache, it passed 5 of 5 runs under --repeat until-fail:5, and the second save logs (merged with the file) in both orders.
  • With only PipelineCache.hpp/.cpp reverted to main, agc_pipeline_cache_file fails with the cache file lost the pipeline of the object that saved first. The fix was restored afterwards.
  • All 36 ctest registrations of agc_driver_recorder_tests on this base pass, none skipped. They include agc_pipeline_cache, agc_pipeline_library, agc_pipeline_library_off and agc_pipeline_library_fast_only.
  • The conventions check (main's tools/check_conventions.py) passes with the current title and body.

As before, these tests cover the file-loss bug. They do not reproduce or validate the Intel saver crash from #4781.

AI-assisted: yes.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
tools/check_skips.py accepts only device-limitation skip reasons, so the Linux job rejected this one.
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Pushed a one-line fix: the Linux full build rejected the agc_pipeline_cache_file skip because tools/check_skips.py only accepts device-limitation wording, so it now says skipped, the device does not keep pipelines in a pipeline cache. The skip itself was right.

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

Labels

agc libSceAgc* build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants