Skip to content

perf(agc): the draw pipeline retires finished commits on a reaper thread - #6617

Closed
TheAggie wants to merge 5 commits into
boykopovar:mainfrom
TheAggie:perf/agc-draw-commit-reaper
Closed

TheAggie wants to merge 5 commits into
boykopovar:mainfrom
TheAggie:perf/agc-draw-commit-reaper

Conversation

@TheAggie

Copy link
Copy Markdown
Contributor

What

With pipelined draws the committer is the thread the queue worker waits for. After each commit it popped the item from the pipeline's deque under the pipeline mutex, which destroyed the commit's closure right there: the decode's programs, the compiled results, the stage captures, the regions, the snapshots and the rest, twenty or more heap frees a draw, on the critical thread and with the lock held while the worker's enqueue waited for it.

  • DrawPipeline::run moves the finished item out under the lock and hands it to retire, which queues it for a reaper thread. The reaper destroys batches: it is woken only when 64 items have gathered, and otherwise checks every 2 ms, so the committer pays one deque move and no wake per draw. (Waking it per commit cost more than it saved.)
  • APS5_NO_COMMIT_REAPER=1 destroys the item in place as before, though after the lock.
  • The drain, the outstanding count and the failure reporting are unchanged: a commit counts as done when it has run, as before; only the memory of its closure is released later. Nothing in a closure's destruction depends on the thread: the stage variants and entries return to mutex-guarded pools, ShaderMemory returns its pages to the destroying thread's pool (as it already did on the committer), and the rest are plain frees.

Tested

Windows 11, NVIDIA GeForce RTX 5070 Ti, MinGW-w64 GCC 15.2.0, on main ce991f1ec.

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. Written with Claude Code under my direction.

🤖 Generated with Claude Code

The committer destroyed a finished commit in place, under the pipeline's mutex: the closure's programs, results, captures, regions and snapshots, twenty or more heap frees a draw on the thread the queue waits for, while the queue worker's enqueue waited on the same lock. A finished commit is now moved out under the lock and handed to a reaper thread, which destroys batches of 64 and wakes on its own 2 ms timer otherwise, so the committer pays one deque move and no wake per draw. APS5_NO_COMMIT_REAPER=1 restores destruction in place, after the lock.

On Ratchet & Clank under RetroArch's PCSX2 core, audio produced in the heavy intro scenes 52-53% of real time to 55-56%, two alternating runs each.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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 1896 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md
PR 5431 docs/dev/ENVIRONMENT.md
PR 6185 docs/dev/ENVIRONMENT.md
PR 6186 docs/dev/ENVIRONMENT.md
PR 6188 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 6501 docs/dev/ENVIRONMENT.md
PR 6924 docs/dev/ENVIRONMENT.md
PR 7246 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 7449 docs/dev/ENVIRONMENT.md
PR 7457 docs/dev/ENVIRONMENT.md
PR 7484 docs/dev/ENVIRONMENT.md
PR 7514 docs/dev/ENVIRONMENT.md
PR 7604 docs/dev/ENVIRONMENT.md
PR 7622 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 7642 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md
PR 7643 docs/dev/ENVIRONMENT.md
PR 7922 docs/dev/ENVIRONMENT.md
PR 8004 docs/dev/ENVIRONMENT.md
PR 8077 docs/dev/ENVIRONMENT.md
PR 8160 docs/dev/ENVIRONMENT.md
PR 8161 docs/dev/ENVIRONMENT.md
PR 8246 docs/dev/ENVIRONMENT.md
PR 8484 docs/dev/ENVIRONMENT.md
PR 8542 docs/dev/ENVIRONMENT.md
PR 8887 docs/dev/ENVIRONMENT.md
PR 8896 docs/dev/ENVIRONMENT.md
PR 8911 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 8963 docs/dev/ENVIRONMENT.md
PR 9012 docs/dev/ENVIRONMENT.md
PR 9098 docs/dev/ENVIRONMENT.md
PR 9129 docs/dev/ENVIRONMENT.md
PR 9133 docs/dev/ENVIRONMENT.md
PR 9176 docs/dev/ENVIRONMENT.md
PR 9243 docs/dev/ENVIRONMENT.md
PR 9248 docs/dev/ENVIRONMENT.md
PR 9252 docs/dev/ENVIRONMENT.md
PR 9287 docs/dev/ENVIRONMENT.md
PR 9289 docs/dev/ENVIRONMENT.md
PR 9345 docs/dev/ENVIRONMENT.md
PR 9346 docs/dev/ENVIRONMENT.md
PR 9356 docs/dev/ENVIRONMENT.md
PR 9385 docs/dev/ENVIRONMENT.md
PR 9387 docs/dev/ENVIRONMENT.md
PR 9400 docs/dev/ENVIRONMENT.md
PR 9401 docs/dev/ENVIRONMENT.md
PR 9439 docs/dev/ENVIRONMENT.md
PR 9490 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 9508 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md
PR 9599 docs/dev/ENVIRONMENT.md
PR 9626 docs/dev/ENVIRONMENT.md
PR 9669 docs/dev/ENVIRONMENT.md
PR 9676 docs/dev/ENVIRONMENT.md
PR 9702 docs/dev/ENVIRONMENT.md
PR 9703 docs/dev/ENVIRONMENT.md
PR 9712 docs/dev/ENVIRONMENT.md
PR 9723 docs/dev/ENVIRONMENT.md
PR 9852 docs/dev/ENVIRONMENT.md
PR 9875 docs/dev/ENVIRONMENT.md
PR 9879 docs/dev/ENVIRONMENT.md
PR 9898 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md
PR 9965 docs/dev/ENVIRONMENT.md
PR 9989 docs/dev/ENVIRONMENT.md
PR 10043 docs/dev/ENVIRONMENT.md
PR 10062 docs/dev/ENVIRONMENT.md
PR 10069 docs/dev/ENVIRONMENT.md
PR 10193 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md
PR 10194 docs/dev/ENVIRONMENT.md
PR 10231 docs/dev/ENVIRONMENT.md
PR 10244 docs/dev/ENVIRONMENT.md
PR 10254 docs/dev/ENVIRONMENT.md
PR 10264 docs/dev/ENVIRONMENT.md
PR 10272 docs/dev/ENVIRONMENT.md
PR 10382 docs/dev/ENVIRONMENT.md
PR 10388 docs/dev/ENVIRONMENT.md
PR 10419 docs/dev/ENVIRONMENT.md
PR 10420 docs/dev/ENVIRONMENT.md
PR 10423 docs/dev/ENVIRONMENT.md
PR 10442 docs/dev/ENVIRONMENT.md
PR 10443 docs/dev/ENVIRONMENT.md
PR 10472 docs/dev/ENVIRONMENT.md
PR 10487 docs/dev/ENVIRONMENT.md
PR 10547 docs/dev/ENVIRONMENT.md
PR 10668 docs/dev/ENVIRONMENT.md
PR 10677 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
PR 10678 docs/dev/ENVIRONMENT.md
PR 10689 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
docs/dev/ENVIRONMENT.md

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On Linux this crashes draw tests at process exit. Each test prints its final success line, then dies with double free or corruption (out) or a SegFault. APS5_NO_COMMIT_REAPER=1 makes the crash go away.

Machine: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, Ubuntu 26.04 (kernel 7.0.0-38), i7-8700. GCC 15.2 (glibc), Release, BUILD_TESTING=ON, ANYPS5_GPU="NVIDIA GeForce RTX 3070". Head 74dad0097 merged into main 5d9dcc42d (clean merge).

main 5d9dcc4 this PR this PR + APS5_NO_COMMIT_REAPER=1
agc_driver_draw_cache_reuse_off, 20 runs passes in every suite run 15/20 fail 0/20 fail
agc_driver_inactive_vertex_point_size, 20 runs passes in every suite run 20/20 fail 0/20 fail
pipelin|draw_cache|agc_driver_pm4 (30 tests), 3 runs 30/30 2 or 3 fail per run (draw_cache_reuse*) 30/30
  • ctest -R "^agc|shader|recompiler" -j1 (flip, display and lds_atomics64 excluded): 7 failures, against 1 on main. The 6 new ones are all SegFault or Subprocess aborted at about 1.58 s: draw_cache_reuse_off, inactive_vertex_point_size, mesh_index_cache, msaa_enable_single_sample, sample_mask_draw and stencil_ref_export_unused. The remaining failure is store_over_untracked, which fails on main too (#6347).
  • Verbose output of one failing run: A repeated draw reused the cached draw without graphics pipeline libraries, then double free or corruption (out), then Subprocess aborted. The test's own checks have already passed at that point.

This is probably a lifetime problem. reap() loops forever and nothing stops or joins reaper. Queue0()'s pipeline is leaked on purpose, so the reaper is still running during exit(). Whatever was retired in the last ~2 ms is then destroyed on the reaper thread while static destructors tear down the pools and caches their closures return memory to. On main the closure is destroyed before outstanding drops, so nothing outlives the last drain. I did not take a backtrace. The env toggle and the timing (after the test's last line) both point there. A drain or shutdown that flushes retired on the caller would probably fix it, as would joining the reaper.

GPUs tested: NVIDIA GeForce RTX 3070, driver 595.99.02, Ubuntu 26.04

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@fma-rc fma-rc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I measured this in Astro's Playroom's heavy gameplay scene on an RTX 5080. The base is main 5d9dcc4 with and without this PR, hidden, with a 20 s window and two alternating rounds. Main gave 12.17 and 11.72 FPS; this PR gave 12.76 and 12.81. In the trace, queue 0 waits 4.4 µs per draw for room instead of 6.0, and the frames match main's.

The exit crash jotitadev reported reproduces on AMD too. On a Radeon 680M (RADV), agc_driver_draw_cache_reuse_off aborts with double free or corruption (!prev) in 5 of 10 runs with this PR. It passes 10 of 10 with APS5_NO_COMMIT_REAPER=1, and 10 of 10 on main. My Playroom runs end with a kill, so they never reach that path.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 16, 2026
@parl0v

parl0v commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Data point from a CPU-bound scene on Windows: RTX 3060 12 GB (NVIDIA 617.42), i5-12400F, Windows 10 19045, MinGW GCC Release. Minecraft Dungeons II (PPSA16064 01.100.100), title screen, no input, 200 s rows, APS5_PROFILE_DRAW=1 APS5_PROFILE_GPU=1; main bc3c0fc vs main + this PR, interleaved with five other PRs in one batch, two rounds. FPS is the mean of the last 10 [present] windows, GPU busy and idle gaps per frame over the last 8. This scene is CPU-bound on this machine (45-47 ms of idle gaps per frame).

libs FPS GPU busy idle gaps
main 6.70 / 6.60 105 / 106 ms 45 / 46 ms
+ this PR (74dad00) 6.88 / 6.95 (+4.0 %) 104 / 102 ms 42 / 42 ms

Above main in both rounds and the only one of the six outside the spread: the idle gaps drop by 3-4 ms per frame with GPU busy unchanged, so the time comes off the CPU side, as you'd expect from moving the retirement off the recording thread. In earlier batches on this scene the largest single-PR gains were +1.3 to +1.6 %. 0 fatal.

AI-assisted: yes (Claude Code)

@parl0v

parl0v commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

A second title for comparison, same box and build (main bc3c0fc vs main + 74dad00): Sword of the Sea (PPSA25344) title screen, 200 s rows, three rounds alternating.

libs FPS GPU busy idle gaps
main 9.60 / 9.54 / 9.60 80 / 80 / 80 ms 24 / 25 / 24 ms
+ this PR 9.59 / 9.60 / 9.60 (+0.2 %) 80 / 80 / 80 ms 25 / 25 / 24 ms

Flat here, which fits: this scene has about half the idle gaps per frame of the Minecraft Dungeons II title (24 ms against 45 ms), so there is less CPU-side waiting for the reaper to take off. No regression, 0 fatal.

AI-assisted: yes (Claude Code)

@eliyarson

Copy link
Copy Markdown
Contributor

I tested this PR locally and found a double free in some agc_ tests. Below: the finding, then a measurement of destroying the closure outside the mutex on its own.

Setup. Upstream main at 72c5eed plus local diagnostic patches (my local branch f62f3f762). Section 1 and the earlier batch in section 2 have #6617 applied on top; the table in section 2 is that base without #6617. Release, Linux (CachyOS), Ryzen 9 5950X, RTX 3080 Ti, NVIDIA 610.57.04. Hades II hub, pinned to cores 8-15,24-31 with taskset, save data restored and checked before every run.

1. Double free in the tests.

  • With the reaper on (default), a full parallel ctest -R '^agc_' run aborted in agc_driver_draw_cache_reuse, agc_driver_draw_cache_reuse_off and agc_driver_msaa_enable_single_sample, and agc_driver_stencil_ref_export_unused crashed. A -j1 re-run of the failing tests aborted in the same four. With APS5_NO_COMMIT_REAPER=1 or APS5_PIPELINED_DRAWS=0 they passed.
  • No backtrace: a gdb run of draw_cache_reuse passed. The cause is from reading the code: the reaper is never joined and DrawPipeline is leaked, so up to 63 retired items can outlive device.Reset() in Driver::stop() and be destroyed afterwards without the GPU lock.
  • DrawPipeline::Quiesce() (wait for items empty, set a stop flag under retireMutex, join the reaper, destroy the rest), called in Driver::stop() before the GPU lock, gave no abort in 11 runs with the reaper on (ten at -j1, one at -j4). That is not proof the race is fixed. It is about 30 lines; I can send it as a commit.
  • A window is left: run() moves the item to a local done, drops the lock, then calls retire(std::move(done)), so Quiesce() can return while the committer holds the last popped item. A retiring counter (incremented under the lock when the item is moved out, decremented after the destroy), with Quiesce() waiting for items.empty() && retiring == 0, closes it for Quiesce(). With the reaper, making Drain() wait for it too would likely cost the gain; that is inference, not measured. I have not added the counter on top of the reaper.

2. Destroying the closure outside the mutex accounts for part of the gain on its own. Same binary, only that change, no reaper thread, toggled by an environment variable. n=4 per arm, interleaved (OFF ON ON OFF ON OFF OFF ON), no crashes:

Arm FPS (mean ± sd) 1% low (mean ± sd)
baseline 62.45 ± 0.67 45.85 ± 0.15
closure destroyed outside the mutex 66.93 ± 0.60 50.11 ± 0.24

In an earlier batch (n=4 per arm, interleaved, Quiesce fix applied), #6617 with the reaper gave 71.09 ± 0.47 FPS and with APS5_NO_COMMIT_REAPER=1 66.27 ± 1.47. The reaper-off path also destroys the item after the lock is released, so its roughly 4 FPS gain over a plain baseline build is probably the same effect (untested). If so, about half the gain is the unlocked destroy and half the reaper, which costs about 2.2 ms/frame of CPU on another core. The batches used different binaries at different times, so I am not comparing numbers across them.

One title, one machine, pinned cores. No unpinned runs, no other game, no TSan.

3. The patch. The variant from section 2 is below. The game runs used the opt-in version of the switch; with the default flipped (APS5_NO_COMMIT_DESTROY_UNLOCKED=1 restores destroying inside the lock) I re-ran the agc_ tests in both modes, not the game. It is against current main (85643f7): git apply --check passes and DrawPipeline.cpp compiles; I have not done a full build or run tests on that base. It does not apply on #6617's head (74dad00): it replaces #6617's run() change instead of stacking on it, so on the PR the retiring decrement would move to after retire() returns. It covers only the no-reaper case. I can push it as a branch on my fork or as a commit on top of this PR if you want it; I will not open a separate PR.

diff --git a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
--- a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
+++ b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
@@ -58,6 +58,7 @@ private:
     std::exception_ptr failure;
     std::atomic<std::size_t> outstanding{0};
     std::uint32_t idleWaiters = 0;
+    std::uint32_t retiring = 0;
     bool committerWaiting = false;
     bool stopped = false;
     std::thread thread;
diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
--- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
+++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
@@ -117,6 +117,10 @@ void DrawPipeline::report(std::chrono::steady_clock::time_point now) {
 }
 
 void DrawPipeline::run() {
+    static const bool destroyUnlocked = [] {
+        const char* text = std::getenv("APS5_NO_COMMIT_DESTROY_UNLOCKED");
+        return text == nullptr || text[0] != '1';
+    }();
     for (;;) {
         Commit* commit = nullptr;
         {
@@ -146,12 +150,23 @@ void DrawPipeline::run() {
             stopped = true;
             if (!shutdown) Driver::Get().ReportFailure(fatal);
         }
-        std::lock_guard lock(mutex);
+        std::unique_lock lock(mutex);
+        std::optional<Item> finished;
+        if (destroyUnlocked) {
+            finished = std::move(items.front());
+            ++retiring;
+        }
         items.pop_front();
         ++commits;
         commitNs += spent;
         if (failed) ++commitErrors;
         if (fatal != nullptr && failure == nullptr) failure = fatal;
+        if (finished.has_value()) {
+            lock.unlock();
+            finished.reset();
+            lock.lock();
+            --retiring;
+        }
         outstanding.fetch_sub(1, std::memory_order_release);
         if (idleWaiters != 0) idle.notify_all();
     }
@@ -164,7 +179,7 @@ void DrawPipeline::Drain(DrainReason reason, std::uint32_t opcode) {
     const auto start = std::chrono::steady_clock::now();
     std::unique_lock lock(mutex);
     ++idleWaiters;
-    idle.wait(lock, [&] { return items.empty(); });
+    idle.wait(lock, [&] { return items.empty() && retiring == 0; });
     --idleWaiters;
     const auto index = static_cast<std::size_t>(reason);
     ++drains[index];
  • Test (not posted): the destructor signals entry and sleeps 20 ms, then Drain() must return only after it finished. Without && retiring == 0 it failed 20 of 20; restored, 0 of 20.
  • Overlaps() and PendingLabel() scan items only. Before, the closure was destroyed inside pop_front() under the mutex, so they could not run during the destroy; now they can, and the item is not visible to them while it is destroyed. That assumes closure destructors do not touch guest memory.

LLM assistance used. I ran the builds and tests.

@milescoviello

Copy link
Copy Markdown
Contributor

A Linux data point on how much this costs the committer in another title, from a profile and not a measurement of this PR. Metal Gear Solid 4 (PPSA18194), Ubuntu 24.04, i7-13700K, RTX 4070 Ti (595.99.02), Release, pad input. main 0dc1562 plus what MGS4 needs here (#6814, #6842, #6237) and #6917. In this scene the committer holds the GPU mutex for about 7.9 s of every 10 s (APS5_PROFILE_DRAW, 124k commits per 10 s).

perf record -F 199 --call-graph dwarf for 6 s at 225 s; 555 of the samples have DrawPipeline::run on the stack:

  • 81.4% under Driver::commitDraw.
  • 10.5% (58 samples) in the std::function manager of Driver::draw's commit closure, that is destroying it from items.pop_front(): _Sp_counted_base::_M_release, free, _int_free.

So in MGS4 about a tenth of the committer's time goes to the closure frees this PR moves off it. I haven't run this PR on MGS4.

AI-assisted: yes (Claude Code)

@Pacuka

Pacuka commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Linux data point: The Smurfs - Dreams (PPSA21607, Unreal Engine), CachyOS, Ryzen 9 5950X, RX 6650 XT / RADV Mesa 26.2.4, main 9f25d0b + this PR + #6566 + #6208 + #6501 (+ unrelated local rendering fixes). Same spot in the Smurfs' Village, standing still, APS5_PROFILE_DRAW=1:

The committer is the bottleneck in this title (~120k commits per 10 s), so the commit time maps straight to frame rate. No rendering differences or aborts.

AI-assisted (Claude Code).

@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.

Thanks for the measurements. Reading the diff, the crash jotitadev, fma-rc and eliyarson reported has two causes in DrawPipeline.cpp:

  • reap() loops forever (l.198-209), so the runningCommitters.fetch_sub that used to end run() now sits after it (l.210) and never runs; run() no longer decrements it. Put it back at the end of run().
  • The reaper thread is never stopped or joined: Stop() (l.213) joins only the committer. Retired items (up to 63) outlive device.Reset() in Driver::stop() and are destroyed afterwards without the GPU lock, which fits the double free or corruption at exit that goes away with APS5_NO_COMMIT_REAPER=1. The reaper needs a stop flag set under retireMutex, a final drain of retired, and a join in Stop(), so every closure is destroyed before the device goes. eliyarson's Quiesce() plus a retiring counter (his comment above) covers the window between popping the item and retire().

Also, the perf label asks for the aps5-summary.jsonl lines of a capture on main and one with the change, from the same title and scene, plus the tools/trace_digest.py output; the third-party numbers help but the form asks for them in the PR.

@JJenkx

JJenkx commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Following the teardown reports above, I tested a shutdown follow-up on main 7a015c0946e2f421b7977129dc7fc255a133019b plus this PR at 74dad00977cd0485dfc2965ed6371b68fe779cb6.

Source: integration branch, retaining this PR's authored commit. The follow-up 9b3ce9fc4d joins the committer first, then signals the reaper to flush its last batch and joins it before Stop() returns. This covers the pop-to-retire window on the normal external shutdown path without changing ordinary Drain(). It also puts the committer-count decrement back at the end of run(); merging this PR into this main otherwise leaves it after the endless reap() loop.

Regression tests cover a captured object's blocked destructor and a 65-item batch tail. The blocked-destructor test passes on main, fails with the unmodified PR integrated into main (Stop returned while a closure destructor was still running), and passes with the follow-up. This reproduces premature shutdown, not an allocator-corruption backtrace.

Validation on the same Linux / Radeon RX 6650 XT / RADV machine, with Vulkan required, shader cache disabled and Khronos validation enabled:

  • Main plus the regression tests: 11/11 pass.
  • PR plus follow-up: 11/11 pass, including all four draw_cache_reuse modes and inactive_vertex_point_size.
  • Follow-up with APS5_NO_COMMIT_REAPER=1: 11/11 pass.
  • Repeated both new Stop tests, draw_cache_reuse_off, and inactive_vertex_point_size 20 times each: 80/80 executions pass.
  • No skips, missing validation layer or VUID- errors in those passing runs. Conventions check passes after documenting the existing fallback switch.

The expanded suite command is:

ctest --test-dir build-full -R '^(agc_draw_pipeline_stop_(active|tail)|agc_driver_(pipelined_fill|pm4|same_queue_label_wait|pipelined_predication_read|draw_cache_reuse(_off|_verify|_inline)?|inactive_vertex_point_size))$' -V --timeout 30

ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation were set; exact commands, layer/ICD paths, patches and verbose logs are in the evidence bundle.

These are shutdown/test results. The subsequent title measurement is pinned to the original sources above and is only one ordered pair. The existing committer self-stop/detach behavior is preserved and is not covered by the synchronous shutdown claim. AI-assisted: yes.

Refresh on main 48790e0233fd2237aac77f25683339d3bc0d4c8f

Replayed the same four patches onto this main without merge commits: refreshed integration branch, candidate a976a27f7afba52ddaa06c938fbb4f610257b042, shutdown follow-up. git range-diff reports all four patches unchanged, retaining the original PR author's commit.

Reran the same 11 tests on main plus the regression tests, the refreshed candidate, and the candidate with APS5_NO_COMMIT_REAPER=1: 11/11 pass in each arm on the same RX 6650 XT / RADV machine with Vulkan required, shader cache disabled, and Khronos validation enabled. No skips, missing-layer messages, or VUID- errors. Full libs build and conventions pass. Fresh logs, exact commands, source patches, and range-diff.

The earlier negative control, 80-run stress result, and native benchmark were not repeated on this refreshed base. No new performance claim. AI-assisted: yes.

@JJenkx

JJenkx commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Native-title follow-up to the shutdown tests above: Record of Lodoss War -Deedlit in Wonder Labyrinth- (PPSA05347), stationary opening forest with tutorial text. Same machine, scene, scripted inputs and settings; separate per-source caches warmed by running the scene once and restarting. One main-then-candidate pair.

  • Baseline main: 7a015c0946e2f421b7977129dc7fc255a133019b.
  • Candidate: 1365624ca52d572b5c0d772249f9de8c53f262f5 = that main + this PR's authored commit + the shutdown follow-up/regression tests linked above.
  • Main 23.26 FPS, candidate 23.93 FPS (+2.85%). This is a small single-pair result, not evidence of a repeatable gain or a full-game improvement.
  • All four launches (warmup/capture for each build) exited 0 through ordinary window close, native PIDs are gone, and the temporary kernel limit was restored after every launch.
  • Scene captures show the same character position, scene and readable tutorial text, with animation differences. Both measured runs logged the same four storage-image destruction warnings at exit; no clean-warning or console-equivalence claim. Native performance capture did not enable validation; the separately linked focused-test validation is the correctness evidence.
  • Captured baseline was main at test start. Main advanced to b6583dd8687f359c2caea295cde23e4732b73433 by the post-test PR audit; these numbers apply to the exact pins above.

main official summary:

{"commit":"7a015c0946e2","dirty":false,"env":{"APS5_DRAW_CACHE_ENTRIES":"4096","APS5_HOST_IMPORT_MIB":"16384","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"1","APS5_SPIRV_OPT":"full","APS5_TRACE_SECONDS":"200:30"},"gpu":"AMD Radeon RX 6650 XT (RADV NAVI23)","driver":"radv Mesa 26.2.4-arch1.1","os":"Linux 7.2.9-arch1-1","resolution":"3840x2160","window":{"spec":"seconds 200:30","first_present":9410,"ms":30003.439},"presents":698,"frame_ms":{"p50":41.744,"p90":52.656,"p99":68.522,"max":79.143},"gpu_busy_ms_per_present":2.218,"worker_busy_pct":{"queue worker 0x0":43.62},"top_phases_ms_per_frame":{"Driver.Packet":18.746,"Driver.Packet.device_lock":12.798,"Graphics.Draw":9.591,"Graphics.Draw.validate":7.781,"Driver.Packet.prepare_memory":5.045,"GpuMutex.wait.label":3.465,"Recorder.Sync":3.323,"GpuMutex.wait.other":2.98},"trace_dropped":false,"trace":"aps5-trace-7a015c0946e2-1334271.json.gz"}

candidate official summary:

{"commit":"1365624ca52d","dirty":false,"env":{"APS5_DRAW_CACHE_ENTRIES":"4096","APS5_HOST_IMPORT_MIB":"16384","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"1","APS5_SPIRV_OPT":"full","APS5_TRACE_SECONDS":"200:30"},"gpu":"AMD Radeon RX 6650 XT (RADV NAVI23)","driver":"radv Mesa 26.2.4-arch1.1","os":"Linux 7.2.9-arch1-1","resolution":"3840x2160","window":{"spec":"seconds 200:30","first_present":9494,"ms":30008.19},"presents":718,"frame_ms":{"p50":40.368,"p90":51.067,"p99":63.197,"max":72.338},"gpu_busy_ms_per_present":2.28,"worker_busy_pct":{"queue worker 0x0":42.605},"top_phases_ms_per_frame":{"Driver.Packet":17.802,"Driver.Packet.device_lock":12.24,"Graphics.Draw":9.282,"Graphics.Draw.validate":7.487,"Driver.Packet.prepare_memory":4.658,"GpuMutex.wait.label":3.052,"Recorder.Sync":2.941,"GpuMutex.wait.other":2.898},"trace_dropped":false,"trace":"aps5-trace-1365624ca52d-1495193.json.gz"}

Official tools/trace_digest.py outputs: main, candidate. Traces: main, candidate. Evidence bundle includes exact source patch/attribution, protocol, commands, reports, full measured terminal log, screenshots and digests. No dropped trace events. Ryzen 9 7950X / RX 6650 XT / RADV Mesa 26.2.4, Linux 7.2.9. AI-assisted: yes.

@eliyarson

Copy link
Copy Markdown
Contributor

Thanks @adriadam10. I read @JJenkx's follow-up 9b3ce9fc4d (branch review/commit-retirement-sync156) against the two causes and the pop-to-retire window I described earlier. This is from reading the code; I have not built or run it.

  • runningCommitters.fetch_sub is back at the end of run().
  • Stop() joins the committer, then sets reaperClosing under retireMutex; the reaper destroys what is left in retired and exits, and Stop() joins it. Driver::stop() calls Stop() before device.Reset().
  • The pop-to-retire window is closed on that path: the committer leaves run() only once items is empty, and each retire() returns before it checks again, so the last one has finished before the reaper is told to close. With APS5_NO_COMMIT_REAPER=1 the destroy runs on the committer, which is joined first.

That covers what my Quiesce() and retiring counter were for, so neither is needed and I am not offering a patch for them. The one path I can't say is covered is Stop() called from the committer thread itself (detach, no reaper join), which @JJenkx already notes.

@TheAggie, if you pull this into #6617: @JJenkx's replay of it on newer main is 3f563df36 on review/commit-retirement-sync158, which they report unchanged by range-diff.

The no-reaper variant from my earlier comment (closure destroyed after the lock is released, no reaper thread) is now on my fork as branch rb/commit-destroy-unlocked, rebased on current main a6dcea927. It is on by default; APS5_NO_COMMIT_DESTROY_UNLOCKED=1 turns it off. Its retiring counter only makes Drain() wait for that unlocked destroy; it is not the Quiesce() counter above, and like the patch in that comment the branch replaces #6617's run() change instead of stacking on it. On Linux (RTX 3080 Ti) it builds and ctest -R agc passes apart from one skipped test and the three agc_driver_sample_order* tests, which also fail on that main. I have no FPS numbers on this base: Hades II on current main aborts during the hub load (native multisample depth metadata fill transition is not implemented, the path #6300 works on), so the numbers in my earlier comment are from the older base only.

For the perf label: @JJenkx posted one main/candidate pair for Record of Lodoss War. I can add a Hades II pair (aps5-summary.jsonl lines for main and for the change, same scene, plus tools/trace_digest.py output) if that helps. I have not run it yet; on current main it needs the hub-load abort above fixed first.

LLM assistance used.

@oneandonlydean oneandonlydean self-assigned this Oct 22, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

I'm picking this up alongside @adriadam10 because it is part of a stack that gets PPSA04263 further on current main. The two open change requests still apply to 74dad00, in particular @adriadam10's: reap() loops forever and teardown double-frees (as @jotitadev saw on Linux). I'm checking whether that still reproduces on current main and how small the fix is. @TheAggie, if you can push a fix, that's the quickest path. Otherwise I'll propose one here for you and the reviewers to look at.

@johandavidcm

Copy link
Copy Markdown
Contributor

Windows data point from Yakuza 0 Director's Cut (PPSA31327), which is committer-bound in gameplay. Windows 11 Pro 26200, RTX 4070 (617.42), Ryzen 7 7800X3D, WinLibs GCC 15.2.0, Release, character standing still in Kamurocho.

Build p50 per run (ms) Mean p50 Presents in 60 s
Base 343.7, 330.8, 344.7, 343.8, 343.8, 339.0 341.0 174-181
This PR 302.4, 334.7, 302.1, 310.9 312.5 (-8.3 %) 180-199
  • Three of the four runs are 9-11 % faster. One (334.7 ms) is within the base's spread.
  • GPU busy per present is unchanged.
  • In the traces, the queue worker's Driver.Packet goes from 338 to 309 ms per frame, and Graphics.Draw from 290 to 281 ms, consistent with the worker waiting less on the committer.

Replicated on current main 0c6a5bd (+ #8098 + #8117). This PR still merges cleanly. Same scene, window 140:60, 2 runs each:

Build p50 per run (ms) Mean p50
Base 346.4, 360.0 353.2
This PR 310.8, 312.5 311.7 (-11.7 %)
Summary lines (one run of each build)
{"commit":"a286c08cd91f","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"170:60"},"gpu":"NVIDIA GeForce RTX 4070","driver":"NVIDIA 617.42","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31892,"title_id":"PPSA31327","title":"Yakuza 0 Director's Cut","content_version":"01.015.000","resolution":"1920x1080","window":{"spec":"seconds 170:60","first_present":2932,"ms":60110.216},"presents":174,"frame_ms":{"p50":343.785,"p90":352.964,"p99":443.525,"max":448.872},"gpu_busy_ms_per_present":74.113,"worker_busy_pct":{"queue worker 0x0":98.708},"top_phases_ms_per_frame":{"Driver.Packet":340.744,"Driver.Packet.draw":310.283,"Driver.Draw":307.147,"Graphics.Draw":292.543,"Graphics.Draw.draw_and_resource_release":204.982,"Driver.Draw.shader_compile_and_link":159.927,"Shader.DrawStage":125.474,"Driver.Draw.draw_enqueued":116.955},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.265,"events_lost":4557401,"totals_full":false},"draw committer":{"events_full_after_s":8.264,"events_lost":1660133,"totals_full":false}},"trace":"aps5-trace-a286c08cd91f-18056.json.gz"}
{"commit":"8af2a67c84e7","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"170:60"},"gpu":"NVIDIA GeForce RTX 4070","driver":"NVIDIA 617.42","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31892,"title_id":"PPSA31327","title":"Yakuza 0 Director's Cut","content_version":"01.015.000","resolution":"1920x1080","window":{"spec":"seconds 170:60","first_present":2988,"ms":60071.381},"presents":198,"frame_ms":{"p50":302.144,"p90":314.216,"p99":328.471,"max":405.043},"gpu_busy_ms_per_present":72.331,"worker_busy_pct":{"queue worker 0x0":98.613},"top_phases_ms_per_frame":{"Driver.Packet":298.984,"Graphics.Draw":271.880,"Driver.Packet.draw":270.452,"Driver.Draw":267.294,"Graphics.Draw.draw_and_resource_release":192.471,"Driver.Draw.shader_compile_and_link":150.712,"Shader.DrawStage":118.400,"Driver.Draw.draw_enqueued":88.169},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.384,"events_lost":4513922,"totals_full":false},"draw committer":{"events_full_after_s":7.938,"events_lost":1744082,"totals_full":false}},"trace":"aps5-trace-8af2a67c84e7-8036.json.gz"}

AI-assisted: yes

oneandonlydean and others added 2 commits October 23, 2026 07:14
The reaper loop never returned, so the committer count it decremented
after the loop never dropped: run() now decrements it itself. Stop()
now also stops the reaper under retireMutex, lets it destroy what is
still retired and joins it, so no finished commit outlives
device.Reset(). A retire after the stop destroys the item in place.

The shutdown test checks that the reaper stopped and that a finished
commit's closure is gone once the driver has shut down.

Co-authored-by: Scott Schuster <scott.n.schuster@gmail.com>
@github-actions

Copy link
Copy Markdown

This pull request is labeled perf, so it needs the summary line of a capture on main and one with the change, from the same title, scene and machine, as the Performance report form asks. Please paste both aps5-summary.jsonl lines, attach the two traces and add the output of tools/trace_digest.py.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@fma-rc @adriadam10 the exit crash you both reported is fixed on 4fe2bfb, which keeps @TheAggie's commit and adds the fix (8326fb0) on top of current main:

  • run() decrements runningCommitters again, so ShutdownThreads sees the committer gone.
  • Stop() joins the committer and then stops and joins the reaper after a final drain, so every finished closure is destroyed before Driver::stop() reaches device.Reset(). A retire after the stop destroys the item in place.
  • ShutdownThreads now also checks that no reaper is left and that a finished commit's closure has expired after shutdown.

On an RTX 4090, without the fix: shutdown_threads_draw_committer 0/5, draw_cache_reuse_off 6/20, inactive_vertex_point_size 4/20 (the rest SegFault/abort). With it: 5/5, 20/20, 20/20. Full CI on 4fe2bfb passed on Linux and Windows. APS5_NO_COMMIT_REAPER=1 stays as the off switch and now has its row in docs/dev/ENVIRONMENT.md.

@fma-rc, if you can repeat your RADV exit run (5/10 before) on this head, that would settle the crash on AMD too.

@fma-rc

fma-rc commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

@oneandonlydean confirmed on RADV, the exit crash is gone on 4fe2bfb. Same Radeon 680M laptop and tests as before:

  • agc_driver_draw_cache_reuse_off: 10/10 pass, no double free (5/10 crashed on 74dad00)
  • the same test with all 16 cores busy: 20/20 pass (2/20 crashed before)
  • shutdown_threads_draw_committer: 10/10 pass

@fma-rc fma-rc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Read 8326fb0, it fixes the crash I reported. The reaper now exits on reaperStopping, drains what was retired before it stopped, and Stop() joins it after the committer, so nothing retired outlives the device. A retire after the stop destroys the item outside retireMutex, which is right. closing and reaperStopping both stay set after Stop(), so later commits run and are destroyed inline, which matches how the committer already behaves after shutdown. The new test checks both the reaper count and the closure lifetime. Runs on RADV are in my comment above.

@oneandonlydean
oneandonlydean dismissed adriadam10’s stale review October 22, 2026 19:31

All four points are addressed at 4fe2bfb: run() decrements runningCommitters again; the reaper's stop flag is set under retireMutex, its last batch is drained and Stop() joins it; the pop-to-retire window is closed by ordering (Stop() joins the committer before stopping the reaper, and Driver::stop calls Stop() before device.Reset()); the perf-label summary lines and trace_digest output are in JJenkx's 19 Oct comment. Dismissing so the review state matches the head; please re-request changes if I've missed something.

@EdouardSence

Copy link
Copy Markdown
Contributor

A Linux/RADV data point for 4fe2bfb, from ASTRO BOT (PPSA21567), whose Sky Garden is committer-bound on this machine.

Setup: ASTRO BOT 01.019.000, -lvl underwater_aerial_garden, no input. Ryzen 7 7800X3D, 31 GiB, RX 9070 XT (RADV Mesa 26.2.4), CachyOS kernel 7.3.0, headless gamescope at 3840x2160. APS5_TRACE_SECONDS=90:50, warm caches, APS5_HOST_IMPORT_MIB=4096 (without #7844 this scene lands at random in a 4–5 s per frame state on main).

One build: main a6646f8 + #9852 (the atomic-only render passes) + this PR at 4fe2bfb, all clean merges. Each run turns the two changes off or on with APS5_NO_ATOMIC_PASSES=1 and APS5_NO_COMMIT_REAPER=1. Two interleaved rounds. Frame p50 in ms:

Switches round 1 round 2 GPU busy ms/present
both off (main paths) 286.1 284.5 142
reaper on 272.9 268.4 138–142
atomic passes on 246.8 245.7 107–110
both on 233.9 232.2 105–109
  • On main paths the reaper takes the frame p50 down by 5 %. With perf(agc): draws that only update a buffer atomically continue one render pass #9852 on, the committer becomes the bottleneck and the reaper gains 5.5 %. Together the two changes take 18.5 % off the frame p50.
  • 8 runs of 180 s, with no hang and no double free. Some runs print VideoOut: presentation thread cannot stop itself when timeout sends SIGINT. That happens with the reaper off too, and on older main runs here, so it does not come from this PR.

Traces for round 2: both off, reaper on, atomic passes on, both on. The trace buffers filled after 3 s on queue worker 0 and after 10 s on the committer, so use the summary lines for timings, not the per-thread busy columns.

trace_digest.py, both off

seconds 90:50 from present 978: 177 presents, frame p50 284.5 / p90 292.3 / p99 318.1 / max 325.7 ms, GPU busy 141.6 ms per present
AMD Radeon RX 9070 XT (RADV GFX1201), radv Mesa 26.2.4-arch3.1, Linux 7.3.0-2-cachyos, 3840x2160, commit 9bf2da0c4fe1, dropped: yes

{"commit":"9bf2da0c4fe1","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"4096","APS5_NO_ATOMIC_PASSES":"1","APS5_NO_COMMIT_REAPER":"1","APS5_NO_SNAPSHOT_CHECK":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"90:50"},"gpu":"AMD Radeon RX 9070 XT (RADV GFX1201)","driver":"radv Mesa 26.2.4-arch3.1","os":"Linux 7.3.0-2-cachyos","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31228,"title_id":"PPSA21567","title":"ASTRO BOT","content_version":"01.019.000","resolution":"3840x2160","window":{"spec":"seconds 90:50","first_present":978,"ms":50157.282},"presents":177,"frame_ms":{"p50":284.47,"p90":292.31,"p99":318.055,"max":325.655},"gpu_busy_ms_per_present":141.631,"worker_busy_pct":{"queue worker 0x0":99.342,"queue worker 0x40":99.542,"queue worker 0x48":99.409,"queue worker 0x46":0.476},"top_phases_ms_per_frame":{"Driver.Packet":846.114,"Driver.Packet.memory_wait":534.315,"Driver.Packet.draw":221.155,"Driver.Draw":220.201,"Graphics.Draw":170.101,"Graphics.Draw.draw_and_resource_release":148.931,"Driver.Draw.shader_compile_and_link":138.885,"Shader.DrawStage":124.594},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.581,"events_lost":3352814,"totals_full":false},"draw committer":{"events_full_after_s":11.278,"events_lost":912995,"totals_full":false}},"trace":"aps5-trace-9bf2da0c4fe1-607889.json.gz"}

Slowest frames (largest slices inside each, inclusive)

  • present 1087: 325.7 ms at 30840 ms: Driver.Packet (queue worker 0x48) 325.5, Driver.Packet.memory_wait (queue worker 0x48) 325.2, Driver.Packet (queue worker 0x40) 324.9, Driver.WorkerWait (queue worker 0x46) 324.1
  • present 992: 318.1 ms at 4017 ms: Driver.Packet (queue worker 0x48) 317.9, Driver.Packet.memory_wait (queue worker 0x48) 317.7, Driver.Packet (queue worker 0x40) 317.3, Driver.WorkerWait (queue worker 0x46) 316.8
  • present 979: 311.2 ms at 282 ms: Driver.Packet (queue worker 0x48) 311.1, Driver.Packet.memory_wait (queue worker 0x48) 310.8, Driver.Packet (queue worker 0x40) 310.4, Driver.WorkerWait (queue worker 0x46) 309.9
  • present 983: 311.0 ms at 1444 ms: Driver.Packet (queue worker 0x48) 310.8, Driver.Packet.memory_wait (queue worker 0x48) 310.5, Driver.Packet (queue worker 0x40) 310.3, Driver.WorkerWait (queue worker 0x46) 309.4
  • present 996: 302.0 ms at 5209 ms: Driver.Packet (queue worker 0x48) 301.8, Driver.Packet.memory_wait (queue worker 0x48) 301.5, Driver.Packet (queue worker 0x40) 301.3, Driver.WorkerWait (queue worker 0x46) 300.7

Threads (ms per frame)

thread busy wait untraced in slices < 20 us largest
queue worker 0x0 17.7 (6 %) 0.6 (0 %) 265.0 19.8 Driver.Packet 18.3, Driver.Completion 0.0
title 0.2 (0 %) 0.0 (0 %) 283.2 0.7 Shader.ResolveGraphicsStagesAbi 0.2, Shader.ResolveAbi 0.0
title 0.1 (0 %) 0.0 (0 %) 283.3 0.3 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
title 0.0 (0 %) 0.0 (0 %) 283.3 0.0 Shader.ResolveGraphicsStagesAbi 0.0, Shader.ResolveAbi 0.0
title 0.1 (0 %) 0.0 (0 %) 283.3 0.9 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
draw committer 36.4 (13 %) 3.2 (1 %) 243.7 19.8 Graphics.Draw 32.8, Recorder.Submit 3.4
queue worker 0x40 19.1 (7 %) 262.5 (93 %) 1.8 2.6 Driver.Packet 281.4, Driver.EndSubmission 0.2
queue worker 0x48 0.3 (0 %) 281.4 (99 %) 1.7 0.2 Driver.Packet 281.5, Driver.EndSubmission 0.1
presenter 0.5 (0 %) 0.8 (0 %) 282.1 0.0 VideoOut.Flip 1.3, Driver.Present 0.0
queue worker 0x46 0.5 (0 %) 281.1 (99 %) 1.7 0.7 Driver.Packet 1.1, Driver.EndSubmission 0.1
title 0.1 (0 %) 0.0 (0 %) 283.2 0.3 Shader.ResolveGraphicsStagesAbi 0.1
gpu 141.6 (50 %) 0.0 (0 %) 141.8 0.0 batch 141.6

Largest slices (ms per frame, inclusive, all threads): Driver.Packet 582.4, Driver.Packet.memory_wait 534.1, Driver.WorkerWait 280.4, Driver.WorkerWait.dequeue_order 276.3

Longest waits

  • Driver.Packet.memory_wait (queue worker 0x48) 325.5 ms at 30873 ms, present 1087
  • Driver.WorkerWait (queue worker 0x46) 315.2 ms at 3954 ms, present 991
  • Driver.Packet.memory_wait (queue worker 0x48) 314.4 ms at 4051 ms, present 992
  • Driver.Packet.memory_wait (queue worker 0x48) 311.0 ms at 1477 ms, present 983

Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)

  • batch 39307: 12.2 ms at 46857 ms, present 1143: GPU no ranges; recorded by no CPU slices
  • batch 41218: 12.1 ms at 49130 ms, present 1151: GPU no ranges; recorded by no CPU slices
  • batch 38351: 11.9 ms at 45693 ms, present 1139: GPU no ranges; recorded by no CPU slices
  • batch 20572: 11.9 ms at 24502 ms, present 1064: GPU no ranges; recorded by no CPU slices
  • batch 41455: 11.6 ms at 49429 ms, present 1152: GPU no ranges; recorded by no CPU slices
trace_digest.py, reaper on

seconds 90:50 from present 995: 187 presents, frame p50 268.4 / p90 277.3 / p99 282.3 / max 292.9 ms, GPU busy 137.8 ms per present
AMD Radeon RX 9070 XT (RADV GFX1201), radv Mesa 26.2.4-arch3.1, Linux 7.3.0-2-cachyos, 3840x2160, commit 9bf2da0c4fe1, dropped: yes

{"commit":"9bf2da0c4fe1","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"4096","APS5_NO_ATOMIC_PASSES":"1","APS5_NO_SNAPSHOT_CHECK":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"90:50"},"gpu":"AMD Radeon RX 9070 XT (RADV GFX1201)","driver":"radv Mesa 26.2.4-arch3.1","os":"Linux 7.3.0-2-cachyos","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31228,"title_id":"PPSA21567","title":"ASTRO BOT","content_version":"01.019.000","resolution":"3840x2160","window":{"spec":"seconds 90:50","first_present":995,"ms":50172.378},"presents":187,"frame_ms":{"p50":268.425,"p90":277.314,"p99":282.347,"max":292.884},"gpu_busy_ms_per_present":137.755,"worker_busy_pct":{"queue worker 0x0":99.32,"queue worker 0x40":99.508,"queue worker 0x48":99.448,"queue worker 0x46":0.508},"top_phases_ms_per_frame":{"Driver.Packet":801.115,"Driver.Packet.memory_wait":507.814,"Driver.Packet.draw":201.085,"Driver.Draw":200.185,"Graphics.Draw":160.379,"Graphics.Draw.draw_and_resource_release":140.264,"Driver.Draw.shader_compile_and_link":135.277,"Shader.DrawStage":120.608},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.668,"events_lost":3289833,"totals_full":false},"draw committer":{"events_full_after_s":11.45,"events_lost":887735,"totals_full":false}},"trace":"aps5-trace-9bf2da0c4fe1-609717.json.gz"}

Slowest frames (largest slices inside each, inclusive)

  • present 1069: 292.9 ms at 19806 ms: Driver.Packet (queue worker 0x48) 292.7, Driver.Packet.memory_wait (queue worker 0x48) 292.4, Driver.Packet (queue worker 0x40) 292.3, Driver.WorkerWait (queue worker 0x46) 291.0
  • present 1144: 282.3 ms at 39899 ms: Driver.Packet (queue worker 0x48) 282.2, Driver.Packet.memory_wait (queue worker 0x48) 281.9, Driver.Packet (queue worker 0x40) 281.4, Driver.WorkerWait (queue worker 0x46) 281.2
  • present 1007: 281.3 ms at 3260 ms: Driver.Packet (queue worker 0x48) 281.1, Driver.Packet.memory_wait (queue worker 0x48) 280.8, Driver.Packet (queue worker 0x40) 280.5, Driver.WorkerWait (queue worker 0x46) 280.0
  • present 1130: 279.9 ms at 36123 ms: Driver.Packet (queue worker 0x48) 279.7, Driver.Packet.memory_wait (queue worker 0x48) 279.5, Driver.Packet (queue worker 0x40) 279.1, Driver.WorkerWait (queue worker 0x46) 278.5
  • present 1009: 279.2 ms at 3820 ms: Driver.Packet (queue worker 0x48) 279.0, Driver.Packet.memory_wait (queue worker 0x48) 278.8, Driver.Packet (queue worker 0x40) 278.5, Driver.WorkerWait (queue worker 0x46) 278.0

Threads (ms per frame)

thread busy wait untraced in slices < 20 us largest
queue worker 0x0 17.0 (6 %) 0.7 (0 %) 250.6 20.1 Driver.Packet 17.7, Driver.Completion 0.0
title 0.1 (0 %) 0.0 (0 %) 268.2 0.7 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
title 0.0 (0 %) 0.0 (0 %) 268.3 0.0 Shader.ResolveGraphicsStagesAbi 0.0, Shader.ResolveAbi 0.0
title 0.1 (0 %) 0.0 (0 %) 268.2 0.3 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
title 0.1 (0 %) 0.0 (0 %) 268.2 0.9 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
queue worker 0x40 10.5 (4 %) 256.0 (95 %) 1.8 2.6 Driver.Packet 266.3, Driver.EndSubmission 0.2
queue worker 0x48 0.3 (0 %) 266.5 (99 %) 1.5 0.2 Driver.Packet 266.7, Driver.EndSubmission 0.1
draw committer 34.4 (13 %) 0.6 (0 %) 233.3 20.7 Graphics.Draw 30.1, Recorder.Submit 4.2
presenter 0.5 (0 %) 0.6 (0 %) 267.2 0.0 VideoOut.Flip 1.0, Driver.Present 0.1
queue worker 0x46 0.5 (0 %) 266.2 (99 %) 1.6 0.7 Driver.Packet 1.1, Driver.EndSubmission 0.1
title 0.1 (0 %) 0.0 (0 %) 268.2 0.3 Shader.ResolveGraphicsStagesAbi 0.1, Shader.ResolveAbi 0.0
gpu 137.8 (51 %) 0.0 (0 %) 130.5 0.0 batch 137.8

Largest slices (ms per frame, inclusive, all threads): Driver.Packet 551.8, Driver.Packet.memory_wait 507.1, Driver.WorkerWait 265.5, Driver.WorkerWait.dequeue_order 261.5

Longest waits

  • Driver.Packet.memory_wait (queue worker 0x48) 291.5 ms at 19839 ms, present 1069
  • Driver.WorkerWait (queue worker 0x46) 286.5 ms at 37662 ms, present 1135
  • Driver.WorkerWait (queue worker 0x46) 284.4 ms at 46317 ms, present 1167
  • Driver.WorkerWait (queue worker 0x46) 284.1 ms at 45779 ms, present 1165

Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)

  • batch 43627: 12.6 ms at 50017 ms, present 1181: GPU no ranges; recorded by no CPU slices
  • batch 36592: 12.5 ms at 41907 ms, present 1151: GPU no ranges; recorded by no CPU slices
  • batch 31668: 12.4 ms at 36247 ms, present 1130: GPU no ranges; recorded by no CPU slices
  • batch 39433: 12.3 ms at 45152 ms, present 1163: GPU no ranges; recorded by no CPU slices
  • batch 33041: 12.3 ms at 37855 ms, present 1136: GPU no ranges; recorded by no CPU slices

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

@tibzejoker

Copy link
Copy Markdown
Contributor

A data point from GTA V (PPSA04264) on Windows (RTX 4090, i9-13900K, MinGW GCC 15.2). It is from a profile on an older base, not a run of this PR.

A note on measuring GTA: the open world trace buffers fill after about 4 s (queue worker events), like in the runs above. So I would report committer us/commit and the [hooksync] waits from APS5_PROFILE_DRAW=1, plus the two aps5-summary.jsonl lines for the record. Where the save respawns also moves FPS a lot (2-11 FPS in one run), so FPS alone is not a good signal here.

I can run 4fe2bfb against its main base in the GTA open world, with the reaper on and with APS5_NO_COMMIT_REAPER=1, and post the summary lines and committer numbers. @oneandonlydean, if you are already testing it in PPSA04263, tell me and I will not duplicate the work.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 23, 2026
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 23, 2026
@johandavidcm

Copy link
Copy Markdown
Contributor

Re-measured on current main 6cd36e6 (+ #8098), Yakuza 0 Director's Cut (PPSA31327), same scene and script as my earlier comment. Windows 11 Pro 26200, RTX 4070 (617.42), Ryzen 7 7800X3D.

Build p50 per run (ms) Mean Presents in 60 s
Base 322.0, 322.8 322.4 186, 186
Base + this PR 332.4, 332.5 332.4 (+3.1 %) 179, 180

On the base I measured on 10-22 (before #6946) this PR gave -8 %. On current main it is 3 % slower: #6946 already retires items off the committer (Enqueue reclaims them on the worker), so the reaper only adds a second consumer of the same retired deque. GPU time per present is unchanged. One scene and one title, so take it as a data point for the rebase rather than a verdict.

Summary lines (base, then base + this PR)
{"commit":"13abb3589418","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"140:60"},"gpu":"NVIDIA GeForce RTX 4070","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31892,"title_id":"PPSA31327","title":"Yakuza 0 Director's Cut","content_version":"01.015.000","resolution":"1920x1080","window":{"spec":"seconds 140:60","first_present":2825,"ms":60158.708},"presents":186,"frame_ms":{"p50":322.007,"p90":334.950,"p99":351.833,"max":431.291},"gpu_busy_ms_per_present":74.935,"worker_busy_pct":{"queue worker 0x0":98.794},"top_phases_ms_per_frame":{"Driver.Packet":319.309,"Graphics.Draw":290.048,"Driver.Packet.draw":289.367,"Driver.Draw":287.064,"Graphics.Draw.draw_and_resource_release":205.158,"Driver.Draw.shader_compile_and_link":145.218,"Shader.DrawStage":118.438,"Driver.Draw.draw_enqueued":115.814},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.262,"events_lost":4558755,"totals_full":false},"draw committer":{"events_full_after_s":7.808,"events_lost":1754780,"totals_full":false}},"trace":"aps5-trace-13abb3589418-91032.json.gz"}
{"commit":"c19a99fde524","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_SECONDS":"140:60"},"gpu":"NVIDIA GeForce RTX 4070","driver":"NVIDIA 621.01","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31892,"title_id":"PPSA31327","title":"Yakuza 0 Director's Cut","content_version":"01.015.000","resolution":"1920x1080","window":{"spec":"seconds 140:60","first_present":2837,"ms":60262.110},"presents":179,"frame_ms":{"p50":332.441,"p90":352.828,"p99":435.396,"max":476.511},"gpu_busy_ms_per_present":75.849,"worker_busy_pct":{"queue worker 0x0":98.762},"top_phases_ms_per_frame":{"Driver.Packet":332.302,"Driver.Packet.draw":302.081,"Graphics.Draw":301.942,"Driver.Draw":299.738,"Graphics.Draw.draw_and_resource_release":213.515,"Driver.Draw.shader_compile_and_link":167.281,"Shader.DrawStage":132.778,"Driver.Draw.draw_enqueued":101.120},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.363,"events_lost":4570590,"totals_full":false},"draw committer":{"events_full_after_s":7.917,"events_lost":1749870,"totals_full":false}},"trace":"aps5-trace-c19a99fde524-37624.json.gz"}

AI-assisted: yes

@oneandonlydean

Copy link
Copy Markdown
Collaborator

@TheAggie thanks for this, and @johandavidcm thanks for the careful measurements. I'm closing it for now, for three reasons:

  • On current main it measures slower: Yakuza 0 on Windows (RTX 4070) went from 322.4 ms to 332.4 ms p50 (3.1%) with this change.
  • The GTA A/B on 23 Oct showed no gain either.
  • perf(agc): draw finisher pipeline (APS5_DRAW_PIPELINE) #6946 has since landed and already does the retiring of finished work this PR was aimed at. The branch also conflicts with main now.

If you see a case where the reaper thread still wins on top of current main, please reopen with the numbers and we'll look again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* needs rebase Conflicts with main, rebase needed perf numbers missing perf pull request without a before and after summary line perf Performance change

Projects

None yet

Development

Successfully merging this pull request may close these issues.