Repository navigation
perf(agc): load the pipeline cache file only on the windowed device and save without re-reading an unchanged file - #6372
Merged
Conversation
…nd save without re-reading an unchanged file The headless device is replaced by the windowed one at window attach and then only lingers for in-flight workers, but it parsed its own full copy of the cache file. It now starts empty (it still merges what it compiles into the file). Load reuses the file buffer as the initial data instead of copying it, and save() writes the live data directly when the file still carries the header this object loaded or wrote, skipping the read-back and the merge cache.
This was referenced Oct 15, 2026
Merged
This was referenced Oct 15, 2026
Closed
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
adriadam10
requested changes
Oct 15, 2026
adriadam10
left a comment
adriadam10
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, the idea (one parse of the cache file instead of two) is sound. One problem and one request:
PipelineCache.cpp:176-189,236: withfileUnchanged,save()stops merging with the file and writes only the live cache, withfloor = max(loadedBytes, settledFile->dataBytes). That only holds for an object that loaded the file. The headless device now hasloadedBytes == 0and an empty live cache: its first save merges with the file and setssettledFileto what it wrote, but the live cache never contains the file's pipelines. On a second save with the file untouched and new pipelines compiled, eithersize < floorand the new pipelines are dropped ("keeping ..."), or the live size exceedsfloorand the file is overwritten without the pipelines that came from disk. Onmainthe merge always happened. Fix direction: only take the direct-write path when this object's live cache is known to contain the file (it loaded it or wrote a merged result it also merged back intocache), otherwise keep merging.tests/Recorder.cpp:7432-7453: each object saves once (the destructor), so the second-save case above isn't covered. Please add a case where an unseeded cache saves twice (the periodic save then the final one, or callsave(false)through a friend/test hook) with a new pipeline in between, and check that the file still holds every pipeline. It should fail on this head.- The PR body says the after-state is not measured. Please post the census line and
[pipeline-cache] loadedcount after the change on the same title, so the 560 MiB claim is backed by a number.
…olds it; test the unseeded double save
Contributor
Author
|
Thanks, both points taken. Head e37e5f0:
|
adriadam10
approved these changes
Oct 15, 2026
adriadam10
left a comment
adriadam10
left a comment
Collaborator
There was a problem hiding this comment.
Checked the interdiff against both points: holdsFile (set only by a successful load()) now gates the direct-write path, so an unseeded cache always merges with the file as on main. The new Recorder.cpp case (unseeded cache, two saves, a new pipeline in between) fails when holdsFile && is removed from fileUnchanged and passes with it. Build + ctest of libSceAgcDriver green apart from failures unrelated to this PR (agc_driver_tessellation segfault also on main, two load timeouts). Merging; the memory saving itself is not measured (stated in the PR).
Contributor
Author
|
After-state, measured on Minecraft Dungeons II (PPSA16064 01.100.100, RTX 5070 Laptop 8 GB, Windows 11, driver 617.42; the Sword of the Sea dump is no longer on this machine). Title screen, 150 s rows, same run copy and warm shader cache,
The cache file of this title is 37 MiB, so the saving here is about 70-100 MiB of working set; the 1.17 GiB figure in the description is the Sword of the Sea census with its 560 MiB file, and I cannot re-measure that one. The frame rate is unchanged, as expected. The census line itself comes from a lab build and is not in this PR; the description now says what was measured where. |
11 tasks done
TheBlenDed
pushed a commit
to TheBlenDed/AnyPS5
that referenced
this pull request
Oct 25, 2026
…ly on the windowed device) at d65cc07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The headless device is replaced by the windowed one at window attach and then only lingers for in-flight workers, but it parsed its own full copy of the pipeline cache file. In Sword of the Sea (PPSA25344) the census showed
pipeline-cache-data: 1.17 GiB in 2 allocationsfor a 560 MiB file, stderr printed[pipeline-cache] loaded 561548 KiBtwice, and the app committed 20-25 GiB in gameplay on a 31 GiB machine.PipelineCachegets aloadFromFileparameter. The headless device passesfalseand starts empty (it still merges what it compiles into the file); only the windowed device loads the file.load()reuses the file buffer as the initial data (the header is erased in place) instead of copying it.save()writes the live data directly when the file still carries the header this object loaded or wrote last (the header holds size and hash), so nobody else added pipelines. It then skips the read-back and the merge cache.tests/Recorder.cppcovers the empty start.Tested
Windows 11, MinGW-w64 GCC 15.2. Rebased onto
origin/main8dc92ae29.ctest.tools/check_conventions.py --base origin/mainpasses on this head.pipeline-cache-data: 1.17 GiB in 2 allocations,[pipeline-cache] loaded 561548 KiBtwice, 20-25 GiB committed in gameplay.[pipeline-cache] loadedonce (37956 KiB) instead of twice (37756 KiB each), app working set 5.65-5.69 GiB against 5.74-5.76 GiB onmain636b1b9 at the title after 135-146 s, title presents per 10 s unchanged (21-24 against 19-23). The census line above comes from a lab build and is not part of this PR. With a 37 MiB cache file the saving is 70-100 MiB; the 1.17 GiB case was not re-measured.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)