Skip to content

perf(agc): drop the second validation of draw packets - #855

Closed
meh7an wants to merge 1 commit into
boykopovar:mainfrom
meh7an:perf/agc-single-draw-packet-validation
Closed

meh7an wants to merge 1 commit into
boykopovar:mainfrom
meh7an:perf/agc-single-draw-packet-validation

Conversation

@meh7an

@meh7an meh7an commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

What

Pm4::ResolveDraw started with Validate(packet, 0). The worker only executes packets of submission.commands, the copy Driver::validate already accepted at submit and at the rewind tail, so the packet sizes ResolveDraw indexes are guaranteed by then. The call is removed; the checks ResolveDraw makes itself, the index range check included, stay.

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. I checked that no throw path is lost. The only caller outside the tests is Driver::draw, which runs on submission.commands. copyCommands inlines the jumps into that buffer, and Driver::validate checks every packet in it with the real queue number, at submit and for the rewind tail. That check is stricter than the removed Validate(packet, 0). The ResolveDraw cases in tests/Pm4.cpp use well-formed packets, and the malformed ones go through Validate directly.

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. Could you measure main against main plus this PR alone on the same page? The mean frame time over a few runs is enough, or the Validate share of the AGC worker profile.

@meh7an

meh7an commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Measured alone as asked: over three interleaved rounds on the same page, the base and this PR average 11.70 and 11.60 FPS, within the noise, and Validate under ResolveDraw was only about 0.16% of the AGC worker's samples on the base (8 of 5,106 over two 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