Repository navigation
perf(graphics): retain vertex input layouts - #1316
Conversation
10cb61f to
12433de
Compare
|
Rebased again on Fedora 44, GCC 16 The saved 20,000-request layout trace fell from 165.5 to 19.8 ns/call, or 88.0% less time. Each timed batch repeated it eight times. Run medians ranged from 163.3–181.7 ns upstream and 19.7–20.0 ns with this PR. Synthetic cases, ns/call (upstream → PR):
All output checksums match. The cache capacity is 1,024, so the 2,048-variant cases measure eviction on each reuse. Empty layouts and over-capacity workloads regress. These CPU measurements do not establish a frame-time gain. The vertex-input, driver, graphics and PM4 tests pass. Windows was not tested. |
12433de to
36b1817
Compare
36b1817 to
1f8e6b2
Compare
|
Fixed the clean-build failure: the vertex-input test now includes the repository Vulkan headers explicitly. Rebased on Linux Release test and convention checks pass. MinGW compiles the test source. The local Windows link is blocked by unresolved SjLj symbols in libc, so Windows runtime validation is left to CI. I checked #1426: it handles short vertex buffers and does not replace this cache. |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks. The code reads fine and agc_vertex_input passes here (Linux, full ctest 420/420), but two things before this can land:
- Your own numbers show regressions: 8 attributes with 2,048 variants goes 366.4 → 472.2 ns/call and empty layouts 1.6 → 8 ns/call, while the win is on a saved 20,000-request trace. Please show it helps on real draw streams (hit rate of the cache and frame/CPU time on a title, with the title ID), or limit it to the cases that win (e.g. skip the cache when it misses repeatedly or for empty layouts) and drop what measured slower.
- It conflicts with
mainincore/libs/prx/libSceAgcDriver/CMakeLists.txt(test list). Please rebase and rerunagc_vertex_inputandagc_driver_graphicsafterwards.
Not verified on our side: Windows runtime and real-title performance.
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the work. Following up on the earlier request, which has had no reply or push. Two things still block this:
- No evidence on real draws. The win (165.5 → 19.8 ns/call) is a replay of a saved layout trace, while your own table shows two regressions (8 attributes × 2,048 variants 366.4 → 472.2 ns/call; empty layouts 1.6 → 8 ns/call). The most this can save is ~145 ns per draw, against a mutex, an LRU list and a
shared_ptron every draw. Please post the cache hit rate and CPU/frame time on a title (title ID, host), or make the cache pay only where it wins: leave empty layouts on the old path and don't take the lock for a variant that keeps missing. - It now conflicts with
maininGraphics/include/VertexInput.hpp,Graphics/include/Context.hppandlibSceAgcDriver/CMakeLists.txt(the vertex-fetch changes from #1769 and the null-V# / record-count fixes touched the same functions). When you rebase, check that the cached layout keeps honouring those: the zero-fill and null-V# cases must still be validated per draw, not frozen into the cached shape. Rerunagc_vertex_inputandagc_driver_graphicsafterwards.
Not verified on our side: Windows runtime and real-title performance.
1f8e6b2 to
d7718f8
Compare
|
@adriadam10 addressed the rebase and cache-regression requests in Empty inputs now skip layout construction and the cache entirely. Evicted variants, and variants that change shape under the same ID, take an inline bypass before the mutex and use the original value-owned builder. The bounded bypass bitmap can conservatively suppress caching after a hash collision; that tradeoff and first-admission costs are documented in the body. The rebase preserves per-draw address, record-count and zero-fill behavior, and adds Fresh isolated CPU measurements (same executable, main's unchanged builder versus revised selection; six processes/mode, ABBA, CPU 3): eight-attribute reuse 362.0 → 51.8 ns/call, and 2,048 cyclic variants 360.7 → 213.3 ns/call. Empty selection skips construction; fully bypassed cases are near baseline (roughly -4% to +2%, overlapping ranges). The reproducible harness, full table and synthetic hit rates are in the body. These are component timings; title hit rates/frame-time gains remain unmeasured.
|
|
Conflicts with main in One question while you are there: |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the rework; the bypass for empty and evicted layouts addresses the earlier regressions. Still open:
- Conflict:
Graphics/include/Context.hppconflicts with main (sampleLocationSampleCountswas added aftersinglePassStorage). Please rebase and rerun the PR's tests afterwards. - Recipe path (
Graphics/src/Draw.cpp:1117):ValidateVertexAddressesruns on every recipe hit, while main only copiesrecipe->vertexInputthere. That is a behaviour change plus a format decode per attribute per draw, and it is the question @oneandonlydean asked. Please drop it, or say which case it guards and add a test that fails without it. - Evidence on real draws: the numbers are still synthetic, with no hit rate or CPU/frame time from a title. The most this saves is about 150 ns per draw (4 attributes), against a mutex and LRU update on every cached draw. Please post the hit rate and CPU time on a title (ID, host), or explain why the gain is worth the added state.
- The bypass bitmap is never cleared and a hash collision silently disables caching for another variant; a short note on why that is acceptable for long sessions would help.
…474) * fix(shader): verify materialized snapshot identity before memo reuse Ported from upstream PR boykopovar#4757. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): emit both registration layouts of a pixel shader from one program Ported from upstream PR boykopovar#4884. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(recompiler): keep the front-end resource plan in the shader disk cache Ported from upstream PR boykopovar#4883. A warm launch loads the stored resource plan and skips the shader front end. ShaderDiskCache FormatVersion 32 -> 33. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): compile reusable SRT expression plans Ported from upstream PR boykopovar#1341. The disk-cache plan decoder from boykopovar#4883 rebuilds the execution plan after a stored plan is decoded, since it is derived from the value graph and not stored. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): keep only evaluable branch conditions in resource plans Ported from upstream PR boykopovar#5181. The plan values now live in an IrValueArena, so the disk-cache plan codec from boykopovar#4883 walks and fills the arena instead of a vector of owned values. The libSceAgcDriver test hunks and the docs hunk are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): ignore unused descriptor fields for scalar buffer loads Ported from upstream PR boykopovar#5049. The test and docs hunks are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(agc): cheaper shader-memory captures per draw (ShaderMemory half) Ported from upstream PR boykopovar#1193, ShaderMemory.hpp/.cpp of its last commit only: word-mask page marks, pooled pages with a last-page shortcut, and lazily copied 64-byte blocks for pages no recorded GPU work writes. Adapted for native guest memory: a lazy block is read through GuestMemory::Read instead of the guest address as a host pointer, and NativeGuestMemory gains a no-op FlushGpuWrites (it keeps no deferred GPU writes). The SRT evaluator half is superseded by boykopovar#1341; the draw cache commits are stacked on upstream #494/boykopovar#1191 and not ported. The capture profile line keeps its current fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(graphics): retain vertex descriptor decode plans (ShaderInputState half) Ported from upstream PR boykopovar#1313, Graphics/ShaderInputState only. The Metal draw path calls DecodeVertexStageInfo per draw, which now builds a VertexStagePlan and reads only the referenced, coalesced table words. The Vulkan DrawDecode/DrawCache hooks that keep the plan across draws are not ported (that driver is not built on macOS). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(graphics): retain vertex input layouts (Metal draw) Ported from upstream PR boykopovar#1316 and adapted to Metal. VertexInputCache (LRU per shader variant, shape-checked on every hit, a variant seen with two shapes is bypassed) now takes a layout builder, so MetalDraw.mm keeps the layout it built with the Metal format predicate instead of calling BuildVertexInputLayout on every draw. Vertex buffer addresses are still validated per draw. The Vulkan Context/VulkanDevice/Draw.cpp hooks and the libSceAgcDriver test are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(shader): build and run the shader disk cache codec tests on macOS Registers anyps5_metal_shader_disk_cache with ctest in the Metal guest-replay configuration and adds a plan round trip: value arena, rebuilt SRT execution plan (evaluated against the original), phi blocks, indirect image tables, and the 9-field SamplerResource / 5-field SampledResourcePair encodings. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Cache validated layouts by shader variant and exact static shape, independently of buffer transport. Preserve per-draw address checks and share layout ownership with draw recipes. Written with AI assistance.
Preserve current null-descriptor and format-component semantics, keep live validation per draw, and leave empty or bypassed layouts on the value-owned path. Add differential, concurrency, and isolated timing coverage. Written with AI assistance.
A recipe hit copies its stored layout as before; the address validation stays on the cached and uncached non-recipe paths only.
d7718f8 to
7a12992
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Picked this up from here since the branch had no push after the last review:
- Rebased onto current main (conflicts in
CMakeLists.txt,Context.hpp,Draw.cpp). - Recipe path: dropped
ValidateVertexAddresses, a recipe hit copiesrecipe->vertexInputexactly as main does (@oneandonlydean's question). - Hit rate / title numbers: not available to us either; the synthetic table stands as the measured benefit, and the bypass removes the regressions seen before. The note about the never-cleared bypass bitmap is already in the PR body.
agc_vertex_input and the scoped ctest (714 tests) pass on the rebased head; agc_driver_tessellation segfaults inside RADV's pipeline-library link, as on main. Merging once the full build is green.
What
Retain validated vertex layouts for stable shader variants, while bypassing inputs that do not benefit from reuse. Rebased onto main
d146ec1b6, including the current vertex-fetch, null-descriptor and record-count behavior.formatComponentsand null-descriptor state. Live addresses are validated on each cache hit. Record counts, zero-fill decisions and copied guest bytes remain per-draw work. Draw recipes keep their existing value-owned layouts.Depends on: none.
Tested
Fedora 44, GCC 16.2.1, Release:
agc_vertex_input,agc_driver,agc_driver_pm4andagc_driver_graphicspass. The cache test also passes with AddressSanitizer and leak detection (ANYPS5_ENABLE_ASAN=ON,ASAN_OPTIONS=detect_leaks=1:halt_on_error=1). Contribution checks andgit diff --checkpass.Coverage includes 10,000 generated layouts compared with the original builder, live address rejection, null descriptors, format-component changes, record-count and zero-stride/OOB decisions, retained-layout lifetime after eviction, conservative bypass collisions, invalid bypassed inputs, concurrent reuse and concurrent eviction.
Isolated CPU measurements
Ryzen 5 5600, pinned to CPU 3, identical Release executable and format-property callback. The baseline calls
BuildVertexInputLayout, whose implementation is unchanged from maind146ec1b6; the cache mode uses the draw's revised layout-selection path. Three ABBA cycles, six processes per mode, one warm-up plus nine measured batches of 32,768 requests each. The table reports medians of process CPU nanoseconds per request; wall time is also printed by the harness. CPU time avoids charging unrelated scheduling delays to either implementation.All output checksums match. Measured reuse cases have 100% cache hits; the cyclic 2,048-variant cases settle at 50%; already-evicted and changing-shape cases take the bypass path with 0%. Empty layouts skip layout construction entirely. Fully bypassed cases are close to baseline: their measured median changes range from approximately -4% to +2%, with overlapping run ranges. The earlier empty-layout and eviction-every-reuse costs are removed.
These are synthetic layout-component measurements, not a saved title trace or frame-time claim. First-time admission still costs a lookup and cache insertion, and this does not establish a title-specific hit rate or a gain under contended multi-threaded draws. Windows runtime and full Linux/Windows CI still need validation.
Checklist