Skip to content

fix(agc): include draw committer work in CPU traces - #6341

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
JJenkx:fix/draw-committer-trace
Oct 19, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
JJenkx:fix/draw-committer-trace

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

The pipelined draw committer does not call PerformanceTrace::BindThread on main. Its existing timed scopes therefore disappear from CPU traces and phase totals even when the summary says trace_dropped: false.

Register the thread once as draw committer. Tracing-disabled execution gets no event buffer; draw scheduling and renderer policy are unchanged. This is a profiling correction, with no FPS or title compatibility claim. It does not establish complete coverage of other background threads.

One linear topic commit rebased onto main 84c1a4d9a26429cb483ec963195cff8ef25d128b: 4de8f7a41c19b2736da31a978a957bcd47e08725. The additive CMake conflict retains main's storage-file-write timeout and suspend-drain setting alongside this PR's trace tests. Main's handoff-spin implementation is retained. Independent of #6188.

Tested

Rebase validation on main 84c1a4d9a2: Release with bundled dependencies builds libs, agc_driver_draw_pipeline_trace_tests, and agc_performance_trace_tests. All three focused CTest cases pass without skips (0.06 seconds). The local validated commit b03061dcc6 and public head 4de8f7a41c have the identical Git tree 7073af7897ccb32275f6e3a0d80d5815650afa58. Conventions and whitespace checks pass. Native evidence below remains from the earlier source pins; this rebase is not a new title-performance measurement.

Arch Linux 7.2.9-arch1-1, GCC 16.2.1 Release, Ryzen 9 7950X. Full libs, agc_driver_draw_pipeline_trace_tests and agc_performance_trace_tests build. The published SPIRV-Tools revision 7f2d9ee926f98fc77a3ed1e1e0f113b8c9c49458 is clean; bundled dependencies are enabled, with no private dependency changes.

  • agc_driver_draw_pipeline_trace, agc_driver_draw_pipeline_trace_disabled, agc_performance_trace: 3/3 pass, no skips, 0.05 seconds. The real DrawPipeline fixture verifies slices and accumulated totals; the separate unconfigured process checks that no committer trace buffer exists.
  • The original negative control removed only the registration call and made the unchanged regression fail with draw committer CPU spans are absent from the trace. A current-main native trace independently confirms that the track is absent without this topic.
  • Conventions and whitespace checks pass. Windows and the complete CTest suite were not run.

Native trace visibility

Override 2: Super Mech League PPSA01907, v01.007.000, main menu with no input. RX 6650 XT / RADV Mesa 26.2.4-arch1.1, 1920x1080 guest output, 3072x1728 window. This earlier full runtime uses main 4524944396dfe91bd7c6949a3d7ac01935a2b840 plus topic 9f45b3b24816f26ddff8930c13fe1e9f6556c220. The populated shader and pipeline caches are retained. APS5_TRACE_SECONDS=60:10, documented buffer capacity APS5_TRACE_EVENTS=4194304, no additional diagnostic feature switch or log redirection; no builds, tests or debugger sampling during capture.

The completed capture has 498 presents, trace_dropped: false, and a draw committer track containing 46,111 Graphics.Draw, 6,812 Recorder.Submit and 496 Recorder.Sync slices. The digest accounts for about 10.2 ms of committer busy work per present in this capture. The run closes normally after 93.23 seconds and restores the temporary udmabuf limit. The title renders its UI but its 3D scene is very dark; this is trace-visibility evidence, not visual-correctness or full-playability evidence.

Validation, trace and digest: committer-main452-validation-public.json, committer-main452.trace.json.gz, committer-main452-trace-digest.txt. The separate main-only main452-pipelined.trace.json.gz also has no dropped events but lacks the committer track. It records a different scene/window and is not used for a speed comparison. Timing-only traces contain no guest code or memory.

Independent validation

EdouardSence tested Astro Bot PPSA21567 on Linux / RX 9070 XT, RADV Mesa 26.2.4, main eb901bcf4 plus this topic (be83ad3eb). Their trace contains the committer's Graphics.Draw/Recorder.Submit scopes and reports the thread in trace_dropped_threads when its buffer fills. Their older-baseline comparison supports visibility, not a speedup. The original investigation is in Astro bring-up; earlier traces cannot gain the missing spans retroactively.

Amoenus independently validated the exact current topic head on Windows, in composition 29318982ba on main 376c040294: full relinker/library build and all three focused groups pass three fresh processes each. Removing only the committer registration makes the enabled-trace test fail 3/3; restoring the source restores all passes. These fixtures create no Vulkan device. The updated report also records a completed RTX 5080 Sonic title replay in composition e4fccf25fe incorporating main 05171bc66b, reaching menus and changing nighttime story cinematics. That replay uses periodic summaries only, so it does not establish a separate draw-committer event lane or title speedup. An explicit full event-window capture remains the Windows title visibility check; worker_busy_pct intentionally lists only queue-worker threads.

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 under Silent stubs in their library's technical debt file
  • 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
  • Unblocks: none
  • GPUs tested: AMD Radeon RX 6650 XT, RADV Mesa 26.2.4-arch1.1, Linux 7.2.9-arch1-1 (native title trace); the three regression fixtures are CPU-only and create no Vulkan device
  • AI-assisted: yes

This was referenced Oct 15, 2026
@github-actions

github-actions Bot commented Oct 15, 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
PR 7246 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp L134
PR 7644 core/libs/prx/libSceAgcDriver/CMakeLists.txt L1865

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 1341 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 1368 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 1395 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 2434 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 4891 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5021 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5371 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5426 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5431 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5499 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5925 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6044 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6185 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6186 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6188 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 6192 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6280 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6300 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6360 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6454 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6457 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6479 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6481 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6486 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6501 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6535 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6538 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 6606 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6683 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6701 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6730 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6747 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6759 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6763 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6788 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6808 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6856 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6868 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6937 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6941 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6946 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 6947 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 6963 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7201 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7223 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7505 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7604 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7642 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp

@EdouardSence

EdouardSence commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Checked on a title, Linux / RX 9070 XT (RADV Mesa 26.2.4). The build was main eb901bcf4 with this PR merged (be83ad3eb), and the scene was Astro Bot PPSA21567 -lvl hub_crashsite with APS5_TRACE_SECONDS=60:60.

With this PR, the trace has a draw committer thread. trace_digest.py reports it at 19.4 ms per frame inside slices, mostly Graphics.Draw 16.0 and Recorder.Submit 3.2, and it shows up in trace_dropped_threads once its buffer fills (16.2 s into the window). Frame times stayed the same as on a slightly older main (aff725bf6) without it (p50 124.4 ms here vs 123.8 and 126.2 ms in two runs there), as the description says.

Numbers and context: #357 (reply in thread)

Tested and written by Claude (AI assistant) on a contributor's machine. AI-assisted: yes.

@JJenkx
JJenkx force-pushed the fix/draw-committer-trace branch from 9378e37 to 9f45b3b Compare October 16, 2026 16:14
@JJenkx

JJenkx commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Additional native title coverage for the committer trace binding: Override 2 PPSA01907 01.007.000, animated main menu, 1920x1080, RX 6650 XT / RADV Mesa 26.2.4 / Linux 7.2.9. Clean integration 46458ce on main e9a3343 includes this PR plus the declared #6959/#6934/#6913/#862/#6781 title fixes; it is not an isolated performance comparison.

Warm shader/driver caches, no validation or RenderDoc mapped, stdout and stderr verified /dev/pts/1, only APS5_TRACE_SECONDS=100:45 as the trace switch, no input during capture. The completed window has 1,996 presents: p50 22.361 ms, p90 25.318 ms, p99 28.577 ms; GPU busy 18.181 ms/present. The draw committer is present in the trace, and the complete scope totals include Graphics.Draw at 11.771 ms/present. No speedup is claimed for this instrumentation change.

Detailed events overflowed after 8.342 s on queue worker 0 and 16.451 s on the committer; both report totals_full=false. Their summary scope totals are complete, but digest values reconstructed from the truncated timelines are not whole-window CPU averages. The native run closed normally at 243.23 s / exit 0 and restored its temporary kernel limit.

Source, trace, summary line, digest, protocol and limits.

AI-assisted: yes.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@JJenkx
JJenkx force-pushed the fix/draw-committer-trace branch from 9f45b3b to 683673a Compare October 17, 2026 22:32
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@Amoenus

Amoenus commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Independent Windows validation of head 683673a8246422d1e6e1a1b272421fe14adfc679.

I merged the exact PR head, retaining its authorship, into our Sonic bring-up composition on main 376c040294370ce7de355485947d5d00a9417e29: tested composition 29318982ba, validation branch. The delta from its pre-PR parent is exactly this PR's three files / 62 added lines. This is validation of a composition, not a pristine PR-head build.

Windows 11 Enterprise 26200, WinLibs GCC 15.2.0 Release:

  • Full relinker/library build plus both affected fixture targets passes.
  • agc_driver_draw_pipeline_trace, agc_driver_draw_pipeline_trace_disabled and agc_performance_trace each pass three fresh processes. Slowest selected execution was 0.07 s; no skips or timeouts.
  • Negative control: removing only PerformanceTrace::BindThread("draw committer") and rebuilding makes the enabled-trace test fail 3/3 with draw committer CPU spans are absent from the trace. The disabled-trace and existing performance-trace groups still pass three runs each.
  • Exact source bytes restored, full relinker/library build repeated, then all three groups pass three fresh processes again. Worktree is clean. Conventions pass against newer main c4c2b3aa73.

Reproduction commands for the positive/restored groups:

cmake --build <build> --target relinker libs agc_driver_draw_pipeline_trace_tests agc_performance_trace_tests --parallel 2
ctest --test-dir <build> -R "^(agc_driver_draw_pipeline_trace(_disabled)?|agc_performance_trace)$" --repeat until-fail:3 --output-on-failure --timeout 30 -j 1

These checks exercise the real draw-committer queue and trace collection without creating a Vulkan device: they confirm its name, nonempty slices/totals and disabled-trace behavior. No physical-GPU execution or title speedup is claimed for these tests.

Title event-trace confirmation — 2026-10-19

The promised ordinary Sonic PPSA99902 check now confirms this visibility on composed fork 7ce7fa53f3, incorporating main 0c25997a5a and the now-merged #6341. Matching full Windows relinker/library and affected fixture builds pass; the game retains its original converted executable and unchanged original inputs/configuration.

An explicit APS5_TRACE_SECONDS=120:10, APS5_TRACE_EVENTS=1048576 window closed normally: 134 presents over 10,115.031 ms, dirty=false, trace_dropped=false, and no trace buffer full marker.

The full Chrome trace contains a separate draw committer track, trace tid=2, distinct from queue worker tid=1 and presenter tid=3. These are trace track IDs, not native Windows thread IDs. It holds 56,989 complete CPU slices, including:

  • Graphics.Draw: 17,420
  • Graphics.Draw.draw_and_resource_release: 17,420
  • Resources.Prepare: 14,037
  • Resources.Complete: 6,293
  • Recorder.Submit: 1,157
  • Recorder.Reap: 404

The remaining slices are validation and label-lock scopes. Actual device: RTX 5080 / NVIDIA 610.62 (32.0.16.1062), Windows 11 Enterprise 26200; Khronos 1.4.363 insertion verified from loader output. The normal system implicit layers also loaded. The game reached its title menu and accepted ordinary Start/Cross input; the longer observation remains in progress.

Trace retained as aps5-trace-7ce7fa53f32e-70852.json.gz, SHA-256 decc014ac74517f0c452fc7f572c3e97ba015e7870fca8d6bbd167da14a865da. The 1,053,662-byte compressed file and parsed verification receipt are preserved locally.

This closes the separate event-lane visibility check. It is not a title speedup, steady-gameplay benchmark, full-playability result, or explanation of the earlier guest fence/watchdog messages. worker_busy_pct still intentionally lists queue workers only; the full event track is the relevant evidence here. The earlier CPU tests/controls above keep their original pins.

Earlier title observations and the previously pending check

Our completed Sonic PPSA99902 diagnostic at pre-PR integration 0e7fba38b5 has queue-worker/presenter/GPU tracks but no draw-committer track. The completed title run reaches initial gameplay, movement and 7 rings; pinned title result and limits. The diagnostic 600:10 trace window captured loading, not that gameplay scene, and is not a performance benchmark. A separate RTX 5080 title replay including the credited PR head is now complete at integration e4fccf25fe, incorporating main 05171bc66b: menus and changing nighttime story cinematics within a configured 3,600-second observation, with runtime/archive restoration independently verified. Completed title evidence and diagnostic limits.

That replay used periodic summaries only, so it does not yet provide a separate draw-committer event lane. Source inspection confirms worker_busy_pct deliberately lists only names beginning queue worker , while phase totals aggregate all registered threads. An absent committer percentage is therefore expected schema, not evidence that this patch failed. A title run with an explicit full event window remains the next visibility check; no title speedup or underlying fence-failure cause is claimed. Existing Linux/title evidence above retains its own pins.

AI-assisted validation/reporting: yes, Codex; author responsible.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@JJenkx
JJenkx force-pushed the fix/draw-committer-trace branch from 683673a to 03c954a Compare October 18, 2026 22:44
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@adriadam10 adriadam10 self-assigned this Oct 18, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@adriadam10
adriadam10 force-pushed the fix/draw-committer-trace branch from 03c954a to 4de8f7a Compare October 18, 2026 23:35
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@adriadam10
adriadam10 force-pushed the fix/draw-committer-trace branch from 4de8f7a to 26ce3f2 Compare October 19, 2026 00:39
@adriadam10
adriadam10 merged commit ad502d7 into boykopovar:main Oct 19, 2026
10 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.

4 participants