Skip to content

perf(pipeline): reuse key storage and hash keys in blocks - #1331

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
TibixDev:pr/pipeline-key-storage
Oct 10, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
TibixDev:pr/pipeline-key-storage

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

What

Reuse pipeline-key storage under the existing cache lock. Hash the serialized key in blocks and retain exact key comparison. Cache ownership and eviction rules stay the same.

Related: #1202 also reuses key storage through a separate last-pipeline cache. This smaller change applies to all lookups and also replaces the byte-at-a-time hash. It does not replace that PR's draw scheduling work.

Tested

Fedora 44, Release: graphics tests and RX 6600 pipeline tests with Vulkan validation. Checks key growth and shrinkage, changed state, unidentified shaders, cache clearing, and retained pipeline lifetime.

Earlier paired component tests on our combined branch measured 0.34/0.61/1.37 µs before and 0.16/0.26/0.45 µs after for 0/8/32 attributes. These are warm lookup timings, not game FPS. Windows was not tested.

Checklist

  • Current main; related perf(agc): pipelined draws on by default #1202 checked and described above.
  • One topic.
  • No new code comments.
  • Unsupported paths still throw; UI stubs are documented.
  • No notes, logs, or images in the repository.
  • No new third-party code.
  • General behavior.
  • Depends on: none.
  • AI-assisted: yes.

@TibixDev

TibixDev commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Isolated measurements for the request in #1324. Baseline: upstream a9ba39c927713cf294b8e1b7877ddb365d3a1d6f. Comparison: that revision plus this PR alone. Both use the same test harness; the baseline has no production changes. Separate shared libraries were checked through the loader.

Linux, Ryzen 5 5600, Fedora 44, GCC 16 Release (-O3 -DNDEBUG), llvmpipe. Runs pinned to CPU 3, in four ABBA cycles: eight processes per version. Each process warms the key, then measures nine passes of 65,536 real CachedPipeline lookups per key size. Medians below are over the eight process medians.

Vertex attributes Main (us/lookup) Main + PR (us/lookup) Less time
0 0.363 0.178 51.0%
8 0.638 0.279 56.3%
32 1.437 0.493 65.7%

The ranges of process medians do not overlap in any case. The key-identity, changed-state, unidentified-shader and cache-lifetime checks pass. The proposed version also passes Vulkan core and synchronization validation without errors.

This measures warmed CPU cache lookups. It does not measure frame time, pipeline compilation or game FPS. Command: agc_driver_recorder_tests --benchmark-pipeline-cache.

AI-assisted measurement and reply.

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

Copy link
Copy Markdown
Collaborator

Conflicts with main in Pipeline.cpp and CMakeLists.txt; please rebase. In pipelineKey, keep main's PipelineVariantId() (from #639) in the early return.

TibixDev and others added 2 commits October 10, 2026 01:03
Keep serialized lookup scratch under the existing pipeline cache mutex and use the standard byte-string hash for indexing. Preserve exact key comparison and private handling of unidentified shaders. Add a real-device cache lifetime fixture and warmed lookup benchmark.

Written with AI assistance.
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased on current main ourselves, keeping PipelineVariantId() in the early return as asked.

Added:

  • agc_pipeline_cache registered in ctest (agc_driver_recorder_tests --pipeline-cache-only). The test was only reachable through a manual flag and the target is EXCLUDE_FROM_ALL, so CI never ran it.

Checked: build and ctest for libSceAgcDriver/libSceAgc green; the new test passes on a local driver and on lavapipe, and fails if the key.clear() in pipelineKey is removed. The timings in the description are not reproduced here. Merging once CI is green.

@adriadam10
adriadam10 force-pushed the pr/pipeline-key-storage branch from c6ba437 to e5c6099 Compare October 9, 2026 23:12
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#1547 core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp L382-383
#2469 core/libs/prx/libSceAgcDriver/CMakeLists.txt L493

@adriadam10
adriadam10 merged commit 7fae9c8 into boykopovar:main Oct 10, 2026
8 checks passed
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 10, 2026
The Windows runner has no Vulkan driver, so agc_pipeline_cache (boykopovar#1331)
fails at device creation: 'AGC graphics: vkCreateInstance: Vulkan
result -9'. Skip with 77 like the other agc execution tests, unless
ANYPS5_REQUIRE_VULKAN is set, as on the Linux job.
oneandonlydean added a commit that referenced this pull request Oct 10, 2026
#1331 registered the agc_pipeline_cache ctest, which runs
agc_driver_recorder_tests, without removing that executable from
KNOWN_UNRUN, so check_test_registration fails on main:
'agc_driver_recorder_tests is run by ctest now; remove it from KNOWN_UNRUN'.
The list is empty now; keep it a set for the set operations below.
oneandonlydean added a commit that referenced this pull request Oct 10, 2026
…-tests

fix(ci): restore main's ctest after #1331 and #1496
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
perf(pipeline): reuse key storage and hash keys in blocks
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
boykopovar#1331 registered the agc_pipeline_cache ctest, which runs
agc_driver_recorder_tests, without removing that executable from
KNOWN_UNRUN, so check_test_registration fails on main:
'agc_driver_recorder_tests is run by ctest now; remove it from KNOWN_UNRUN'.
The list is empty now; keep it a set for the set operations below.
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
The Windows runner has no Vulkan driver, so agc_pipeline_cache (boykopovar#1331)
fails at device creation: 'AGC graphics: vkCreateInstance: Vulkan
result -9'. Skip with 77 like the other agc execution tests, unless
ANYPS5_REQUIRE_VULKAN is set, as on the Linux job.
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
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