Skip to content

perf(agc): maintain draw fingerprints on register writes - #1312

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
TibixDev:pr/incremental-draw-fingerprints
Oct 13, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
TibixDev:pr/incremental-draw-fingerprints

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

Maintain draw-register fingerprints on writes, using the current draw dependency ranges. Runtime user words stay live; device serial and shader registry identity remain in the final key. Copy, restore, erase, clear and PM4 writes keep the fingerprint consistent. APS5_VERIFY_REGISTER_KEYS=1 checks it against full recomputation.

Rebuilt directly on main d146ec1b6, removing the old #1311 stack and its draw-plan API. This is a single-topic, seven-file change using the current prepared-shader draw path.

Tested

Fedora 44, GCC 16.2.1, Release:

  • 30,000 differential mutations plus dependency ranges, masks, missing-versus-zero registers, retained write references, copies/moves, context restore and clear.
  • Register-key, PM4, graphics and prepared-graphics CTests all pass (3.98 seconds total). Debug recomputation enabled; GPU cases use RX 6600 RADV with core/synchronization validation.
  • Separate benchmark executables compiled with the exact main and proposed register containers, including write maintenance. Six runs per side in ABBA order, CPU 3 pinned, process CPU time, 32,768 lookups per case. With 24 live user words and 0/1/16/64/256 additional writes, main medians were 1.386/1.323/1.416/1.739/3.028 microseconds; proposed medians were 0.144/0.155/0.275/0.632/2.032 microseconds. These are warm component costs, not frame-time improvements.
  • Conventions and diff checks pass.

No new isolated in-game performance claim. Plain main currently stops this Skyrim boot at unsupported NGS reverb channel conversion; a shared experimental NGS integration baseline is being checked separately. Windows and full platform CI were not run locally; those require the repository's collaborator-dispatched workflow.

Checklist

  • Based on main; prerequisite and related work checked.
  • 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.

Title artifact correction

A binary-hash audit found that the earlier title runtime copies all contained the same patched main AGC library. Those runs establish the main + experimental NGS baseline only. They do not validate #1312/#1313 in-game. The separately built CTests and CPU benchmarks used the new unpatched libraries/source and remain valid. The earlier PR-specific title claim is withdrawn; corrected results follow.

Verified runtime rerun

Corrected runtime result: the NID-patched #1312 library has SHA-256 a15e22c1b1305f79ab39089c7640ab9b6dd0fb9e6bffc0910d053cddfc7d51eb, differs from baseline, and contains the fingerprint verification code. /proc confirms the game loaded that file and the shared experimental NGS library. A fresh 120-second run with APS5_VERIFY_REGISTER_KEYS=1 reported at least 1,706 register-key lookups, no fingerprint mismatch, and presented frames. It stopped at the imposed timeout (124). No performance comparison is claimed with this verification enabled.

@TibixDev

TibixDev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased on c081eb90. Updated #1311 after #1134 merged. Non-indexed mesh draws retain the upstream null index descriptor; retained plans and their invalidation coverage remain intact.

Fedora 44 Release: agc_driver_draw_plan_tests, agc_driver_tests, agc_driver_graphics_tests, agc_driver_pm4_tests build; the targeted tests and contribution checks pass. Windows CI is pending.

@TibixDev
TibixDev force-pushed the pr/incremental-draw-fingerprints branch 2 times, most recently from a144cbb to 913404d Compare October 8, 2026 07:38
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 8, 2026
@TibixDev
TibixDev force-pushed the pr/incremental-draw-fingerprints branch from 913404d to c9e08e6 Compare October 8, 2026 08:19
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not mergeable as is, and I can't rebase it mechanically:

  • It conflicts with main: #639 replaced compileDrawStage with materializeDrawStage (built from invocations prepared at registration), so the draw path this PR extends no longer exists. As @oneandonlydean already noted, the retained compile results from #1311 have to be redone on top of it, and this PR sits on that work.
  • Please rebase the stack bottom-up (#1311 first, then this PR) onto current main and keep this PR to its own commit.
  • Please post before/after numbers for this change alone against current main (the fingerprint maintenance on register writes), not against the pre-#639 tree: with shader compilation at registration, the per-draw cost profile changed.
  • Re-run the draw-plan, driver, graphics and PM4 tests after the rebase, including the debug fingerprint-vs-recomputation check.

Happy to review again once it's on top of main.

@TibixDev
TibixDev force-pushed the pr/incremental-draw-fingerprints branch from c9e08e6 to 748d8d3 Compare October 12, 2026 04:47
@TibixDev

TibixDev commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Reworked this directly on main d146ec1b6 in 748d8d3af; #1311 and the old draw-plan stack are no longer dependencies. The fingerprint now feeds the existing draw key, preserving the current device/shader registry inputs and prepared-shader path.

Register-key differential/mutation tests, PM4, graphics and prepared graphics pass with APS5_VERIFY_REGISTER_KEYS=1; GPU tests use RADV core/synchronization validation. The new proxy also preserves compound writes and snapshot reads used by the graphics fixtures.

For an isolated comparison, I compiled separate benchmarks using main's exact register container and this branch's container. Six ABBA runs per side, pinned CPU, process CPU time, including writes: 24 live user words cost 1.386 → 0.144 µs; with 256 additional writes, 3.028 → 2.032 µs. Intermediate 1/16/64-write cases also improve. These are warm component numbers; the body no longer relies on combined-fork Helgen evidence or implies a measured title gain.

Full Linux/Windows CI still needs the collaborator-dispatched workflow. AI-assisted update.

Artifact correction: the title runtime copies were subsequently found to contain the same patched main AGC library. The saved PRESS ANY BUTTON frame therefore validates the main + experimental NGS integration, not #1312 or #1313. The CTests/CPU benchmarks used the new unpatched builds and remain valid. The earlier PR-specific title claim is withdrawn; corrected results follow.

Corrected runtime result: the NID-patched #1312 library has SHA-256 a15e22c1b1305f79ab39089c7640ab9b6dd0fb9e6bffc0910d053cddfc7d51eb, differs from baseline, and contains the fingerprint verification code. /proc confirms the game loaded that file and the shared experimental NGS library. A fresh 120-second run with APS5_VERIFY_REGISTER_KEYS=1 reported at least 1,706 register-key lookups, no fingerprint mismatch, and presented frames. It stopped at the imposed timeout (124). No performance comparison is claimed with this verification enabled.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@github-actions

github-actions Bot commented Oct 13, 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
#4855 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp L4
#4943 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawRegisterKey.cpp L8-9, L28-38

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked on current main: merged tree builds, agc_driver_graphics, agc_driver_pm4, agc_driver_prepared_* and the new agc_driver_register_key pass. The new test does not compile without the change, and dropping the fingerprint update in Registers::erase makes it fail ("incremental fingerprint missed a mutation"). The Reference proxy also keeps any raw uint32_t& write from bypassing the fingerprint at compile time.

Note: #4943 touches DrawRegisterKey.cpp too and conflicts with this one, so whichever lands second needs a rebase. Merging once the full Build is green.

@adriadam10

Copy link
Copy Markdown
Collaborator

The full Build failed on Windows only in agc_driver_register_key (exit code 0xc0000135, DLL not found at start-up); the other 1051 tests passed. The new test target linked only libSceAgcDriver, while the sibling AGC driver tests (agc_driver_pm4_tests, agc_driver_graphics_tests) also link libc and libkernel.

Added on top of your commit, rebased on current main:

  • core/libs/prx/libSceAgcDriver/CMakeLists.txt: agc_driver_register_key_tests now links libc and libkernel like its siblings.

Merging once the full Build is green on the new head.

@adriadam10
adriadam10 force-pushed the pr/incremental-draw-fingerprints branch from 748d8d3 to 9b19909 Compare October 13, 2026 10:47
TibixDev and others added 3 commits October 13, 2026 14:49
Track mutations against the current draw dependency mask and preserve device and shader registry identity in the draw key. Validate mutations, projected masks, PM4 packets and prepared graphics against full recomputation.

Written with AI assistance.
…ibling tests

The Windows build failed to start it (0xc0000135, DLL not found).
… Windows

The sibling AGC driver tests get this through the agcTest loop; without it the test exits 0xc0000135 (DLL not found) on Windows.
@adriadam10

Copy link
Copy Markdown
Collaborator

The full Build on the previous head still failed on Windows only in agc_driver_register_key (0xc0000135, DLL not found at start-up). Linking libc and libkernel was not enough: the sibling AGC driver tests also get PATH=path_list_prepend:$<TARGET_FILE_DIR:libc> on Windows from the agcTest loop, and this test did not.

Added on top of your commit, rebased on current main:

  • core/libs/prx/libSceAgcDriver/CMakeLists.txt: agc_driver_register_key prepends libc's directory to PATH on Windows, like its siblings.

Merging once the full Build is green on the new head.

@adriadam10
adriadam10 force-pushed the pr/incremental-draw-fingerprints branch from 9b19909 to dd49e1d Compare October 13, 2026 12:59
@adriadam10
adriadam10 merged commit 21bdab0 into boykopovar:main Oct 13, 2026
8 checks passed
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.

2 participants