perf(shader): admit repeated materialized results to the memo

[TibixDev]

Status after review

Awaiting a demonstrated memo-eviction workload.

Current main still has a 256-entry per-source result memo, but the insertion point is now materializePreparedMemoized keyed by variant->artifact.variantId. This policy would need to move there. I confirmed that main already reports memo hits, misses, evictions and Populate time under APS5_PROFILE_DRAW, so another instrumentation API is not needed.

The earlier unique/repeated microbenchmarks do not demonstrate the proposed benefit: the reviewer reproduced a slowdown in both. The needed comparison is a mixed hot-set plus one-use stream that actually evicts useful results, followed by a title profile showing that eviction/Populate cost matters. No such isolated title gain has been established. I have removed the description's implication that the older component numbers justify landing it; no new code push or merge-readiness claim.

A subsequent 180-second Skyrim run with the same main-based experimental NGS library reached recurring draws/dispatches. In three later 10-second memo intervals, main recorded 154/182/208 evictions and only 1.4/1.2/1.2 ms total Populate time. This confirms that evictions occur, but does not show that admission would improve hit rate or materially reduce frame time; the hot-set-versus-stream trace and isolated A/B are still missing.

What

Results used once fill the shader-result cache and evict useful entries. Retain a result only after its key occurs twice. A fixed history of 256 keys avoids allocation and preserves the existing result lifetime.

Tested

Earlier branch-specific tests and measurements remain in the discussion. They have not been rerun on a redesigned current-main version of this PR. The review audit and shared NGS-assisted title exploration do not establish that this head is merge-ready. No full Linux/Windows platform CI was dispatched in this pass.

Checklist

• Current-main implementation and outstanding review requirements complete.
• Isolated title benefit / reference validation established where required.
• Depends on: none.
• AI-assisted: yes.

[TibixDev]

Rebased on bc1fbe81. Kept the upstream buffer-device-address test registration alongside memo-admission coverage.

Fedora 44 Release: agc_result_memo_tests, agc_shader_memory_tests build; the targeted tests and contribution checks pass. Windows CI is pending.

[adriadam10]

Thanks for the PR. Findings:

• The claimed gain does not reproduce. I ran this PR's own agc_result_memo_tests --benchmark (3 runs each, same machine, same binary built with and without the admission check; baseline = admit forced to true):

  • one-use: PR 0.317–0.341 µs, baseline 0.302–0.308 µs
  • repeated64: PR 0.060–0.098 µs, baseline 0.050–0.051 µs

  So the PR is slower in both rows here. Both numbers are also sub-microsecond CPU microbenchmarks, which CONTRIBUTING doesn't accept as a reason for a perf PR on their own. Please post before/after numbers on the same tree (main vs main + PR), several runs, ideally from a real draw/dispatch profile (APS5_PROFILE_DRAW) showing the memo's eviction actually costs something.

• Cost vs. benefit. It adds a 69-line custom open-addressing hash plus a 159-line test to save at most a few hundredths of a microsecond per call, and every first occurrence of a result is now rebuilt once more before it is retained. Please show a workload where evictions of one-use results hurt hit rate; otherwise this isn't worth the code.

• Conflicts with main: materializeMemoized was split into materializePreparedMemoized (variant->artifact.variantId) and libSceAgcDriver/CMakeLists.txt changed. Needs a rebase.

• agc_result_memo shows as "Not Run" in a scoped ctest unless agc_result_memo_tests is built explicitly; it is registered correctly, just noting it.

Happy to look again with measurements that show a gain.

[TibixDev]

Review-pass update:

Current main still has a 256-entry per-source result memo, but the insertion point is now materializePreparedMemoized keyed by variant->artifact.variantId. This policy would need to move there. I confirmed that main already reports memo hits, misses, evictions and Populate time under APS5_PROFILE_DRAW, so another instrumentation API is not needed.

The earlier unique/repeated microbenchmarks do not demonstrate the proposed benefit: the reviewer reproduced a slowdown in both. The needed comparison is a mixed hot-set plus one-use stream that actually evicts useful results, followed by a title profile showing that eviction/Populate cost matters. No such isolated title gain has been established. I have removed the description's implication that the older component numbers justify landing it; no new code push or merge-readiness claim.

A subsequent 180-second Skyrim run with the same main-based experimental NGS library reached recurring draws/dispatches. In three later 10-second memo intervals, main recorded 154/182/208 evictions and only 1.4/1.2/1.2 ms total Populate time. This confirms that evictions occur, but does not show that admission would improve hit rate or materially reduce frame time; the hot-set-versus-stream trace and isolated A/B are still missing.

I have updated the PR description to match this status. The PR remains open; the outstanding review requirements are not being marked resolved. AI-assisted audit.

[fma-rc]

A title workload for the comparison asked for above: Astro's Playroom (PPSA01325) in its first level evicts on practically every memo miss. This is main's own [recompile] result memo report under APS5_PROFILE_DRAW=1, four consecutive 10 s windows in gameplay (Linux, RTX 5080, about 2,900 draw packets per frame at 12 FPS):

hits	misses	hit rate	evictions
189,724	416,773	31.3%	416,773
199,584	437,000	31.4%	436,998
194,365	428,351	31.2%	428,349
196,260	431,692	31.3%	431,692

The title needs more than main to reach the level, so the build is main d371ed3 plus the since-merged PR 4162 and local changes (a depth-plane copy check and draw-path work). Recompiler.cpp is unchanged from that commit, so the memo is main's 256-entry one.

Measured since: a larger memo does not help here. With ResultMemoEntries raised from 256 to 4096 the hit rate stays at 31.7% (four windows, 228,171 to 240,551 hits against 494,754 to 519,003 misses). A counting run shows why: of 5.2 million misses, 731,851 had a key seen before, while 5,198,754 had been seen before once buffer and vertex buffer addresses are left out of the key (1,246 distinct). The game places per-draw constants at new addresses every frame, so nearly every miss is a new key and nearly every evicted entry is never asked for again.

These two runs are on the same stack with one more local memo change (a miss reuses the storage of the evicted result). I have not run this PR's policy on it.

[oneandonlydean]

Closing for now: the author found no demonstrated gain from the memo. With the queue this large we're closing stalled perf PRs that don't show a gain in a title; reopen it with before/after numbers from a title scene (the summary lines from the performance report form) if you pick it up again.