Skip to content

perf(agc): hand out the cached graphics context without copying it - #854

Closed
meh7an wants to merge 1 commit into
boykopovar:mainfrom
meh7an:perf/agc-graphics-context-by-reference
Closed

meh7an wants to merge 1 commit into
boykopovar:mainfrom
meh7an:perf/agc-graphics-context-by-reference

Conversation

@meh7an

@meh7an meh7an commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

What

VulkanDevice::graphicsContext() returns the cached Graphics::Context by value: about 1.4 KB and a shared_ptr<BufferPool> (two locked reference count operations) copied for every draw, dispatch, buffer fill, copy and label write, and once more per draw in KnownDrawRejection.

An overload now takes local storage and returns a reference to the cached context; the storage is only used before setup has built it or under APS5_NO_CONTEXT_CACHE. The per-operation callers use it, and the setup paths keep the by-value form.

Tested

Windows 11, MinGW GCC 15.2, RTX 2080 Ti. This branch builds, and the 165 agc driver, recompiler and execution test targets pass with it stacked on the other changes I'm sending (#849 and the perf PRs opened with this one). I haven't measured this change alone: together with 14 changes of the same kind on my working branch it took Sonic Origins Plus's heaviest menu page (about 1,650 draws per frame) from 48.0 to 43.6 ms per frame (20.8 to 22.9 FPS).

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions github-actions Bot added the agc libSceAgc* label Oct 6, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks. The reference stays valid as far as I can see: state->context is only assigned once in the constructor, and the storage is only used before that or under APS5_NO_CONTEXT_CACHE, so callers get the same context as before.

One thing before it can go in. Performance PRs need before/after numbers for the change itself (the owner asked for this on #218). The 48.0 to 43.6 ms figure is for 15 changes together, so it doesn't show what this one gives. Could you measure main against main plus this PR alone on the same Sonic Origins Plus page? The mean frame time over a few runs is enough, or the share of graphicsContext() and the BufferPool reference counting in the AGC worker profile.

@meh7an

meh7an commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Measured alone as asked: over six interleaved rounds on the same page, the base and this PR average 11.52 and 11.54 FPS, within the noise, and the context copies were only about 0.2% of the AGC worker's samples on the base (13 of 7,404 over three 15 s profiles, none with this PR). It doesn't help measurably on its own, so I'm closing it, as asked on #218.

@meh7an meh7an closed this Oct 7, 2026
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