Skip to content

perf(agc): reserve resource keys for their serialized payload - #4783

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
Scramblrrrr:codex/resource-key-capacity
Oct 14, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
Scramblrrrr:codex/resource-key-capacity

Conversation

@Scramblrrrr

@Scramblrrrr Scramblrrrr commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Resource content keys reserve 8 + 12 * binding_count words, even though descriptor payloads, packed masks and sampler associations can make the key much larger. Constructing such keys repeatedly grows the vector and copies its contents.

Reserve the complete serialized word count before emitting the key. The count respects the existing data-word and movable-buffer policies and checks cumulative size against the vector limit. VisitContentKey and the serialized key contents are unchanged. No cache validation or shader behavior changes.

Tested

Windows 11, Ryzen 7 7800X3D, MinGW GCC15.2, Release. Rebased base 88107b7de6cd7c7952c496345ffc4f413bef374b; head 8e7cdd975f47c5f57b9641ff9d4442bda7cc7795.

  • Full agc_driver_graphics mocked graphics suite passed (5.01s). The focused key mode and the preserved sparse-buffer-holes control also passed.
  • The new fixture checks descriptor counts0/1/31/32/33/16384, all four data-word/movable-buffer modes, written-buffer masks, final packed flags and sampler associations. It also runs in the regular graphics suite.
  • Same harness with only production ShaderResources.cpp restored to baseline passes the policy checks. All24 cases have matching key lengths, FNV checksums and benchmark checksums between variants. Restoring/rebuilding the patch passes again.
  • Branch conventions and git diff --check pass. No Vulkan device is used by the key benchmark.

Earlier benchmark on base 584cc4c57 (not rerun after the rebase): three ABBA cycles, six processes per variant,256 key constructions per timing case; identical compiler flags/harness and no game, build or other test running concurrently. Representative process medians with data words off and movable buffers off:

Synthetic image descriptors Baseline us/key Reserved us/key Change
0 0.191 0.090 about-53%
33 0.581 0.496 about-15%
16384 436.139 177.580 -59.3%

The large-case ranges were419.143–443.021us baseline and176.343–200.913us patched. Across its four modes, large-key medians decreased47–59%. Some small-case ranges overlap; they are sub-microsecond measurements. The large key's capacity falls from180224 to152128 words (152385 with data words). These are isolated CPU key measurements, not overall title FPS results or a game-unblock claim.

Reproduce with agc_driver_graphics_tests --resource-content-key and agc_driver_graphics_tests --benchmark-resource-content-key. To compare, leave the harness unchanged and restore only Graphics/src/ShaderResources.cpp from the recorded base.

Checklist

  • Based on recorded current main; checked open allocation/key PRs (perf(agc): group resource metadata allocations by lifetime #1366 changes resource-owned metadata arenas, a different allocation).
  • One topic per PR.
  • No code comments added.
  • Unsupported paths unchanged; no silent stubs added.
  • No investigation files, title data, binaries or images added.
  • No new third-party code.
  • Generic behavior.
  • Depends on: none.
  • Unblocks: none.
  • AI-assisted: yes (Codex).

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Acknowledged the overlap report. #1345 normalizes buffer ranges and #4206 clears recycled sparse-buffer holes. This PR changes neither path: its production change is confined to ContentKey reservation in ShaderResources.cpp. The reported overlaps are shared includes and fixture/CLI dispatch in Graphics.cpp. It does not require either PR; their test modes/assertions must be retained when combining the runner changes. Current GitHub mergeability is MERGEABLE; no claim that future merges will remain conflict-free. AI-assisted: Codex.

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

The updated overlap report also lists #4795. Its production change concerns pipeline-cache synchronization, while #4783 only reserves ContentKey payload storage. The shared Graphics.cpp dispatch location should retain both independent test modes when combined; neither PR depends on the other. Current mergeability remains MERGEABLE. AI-assisted: Codex.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 13, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@Scramblrrrr
Scramblrrrr force-pushed the codex/resource-key-capacity branch from 253ac3c to 8e7cdd9 Compare October 13, 2026 05:56
@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Rebased and pushed 8e7cdd9 on main 88107b7. The Graphics.cpp conflict was between the new sparse-buffer-holes CLI mode and our resource-content-key mode; both are retained. Full mocked graphics PASS 5.01s, focused content-key policy/count checks PASS, preserved sparse-buffer-holes control PASS. Scoped conventions/diff checks pass; source clean. Production reservation unchanged. Earlier ABBA measurements stay explicitly tied to base 584cc4c, rather than claimed as a rerun on this base. No libraries from this build staged into 007. AI-assisted (Codex).

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

Copy link
Copy Markdown
Collaborator

Checked that the reserved count matches what VisitContentKey emits (5 header words, 18 fixed words per binding, packed masks, sampler associations, descriptor words under the data-word/movable-buffer policy) and that the key contents are unchanged. agc_driver_graphics passes on the PR merged onto current main; merging once the full Build is green.

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Thanks for the review and current-main graphics cross-test. Head remains8e7cdd975f47c5f57b9641ff9d4442bda7cc7795 and it still merges cleanly. I checked the current checks: conventions/report/eligibility succeeded, but windows, linux, relinker, register-context and write-watch were skipped. That does not establish the full Build is green. If a maintainer-triggered Build is required, that gate is still outstanding; I have not represented those skipped jobs as passes.

AI-assisted: yes (Codex).

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

Copy link
Copy Markdown
Collaborator

Checked that the reserved count matches what VisitContentKey emits and that the key contents are unchanged. agc_driver_graphics passes on the PR rebased onto current main.

Added on top of your commit:

  • Rebased onto main: the Graphics.cpp conflict was the neighbouring CLI modes; both kept.
  • ResourceContentKeyTests now also requires capacity() == size(), so the test fails without the reservation (it passed on main before).

Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the codex/resource-key-capacity branch from 8e7cdd9 to 49a88a7 Compare October 14, 2026 10:47
@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10
adriadam10 force-pushed the codex/resource-key-capacity branch from 49a88a7 to 6d7b1d1 Compare October 14, 2026 11:08
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased again onto current main: Graphics.cpp conflicted once more on the neighbouring CLI modes (--undithered-alpha-to-coverage-state); all modes kept. Nothing else changed since the last comment (your commit plus our capacity() == size() assertion in ResourceContentKeyTests). agc_driver_graphics passes on the rebased head.

Merging once the full Build is green.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@adriadam10
adriadam10 merged commit 968d326 into boykopovar:main Oct 14, 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*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants