Repository navigation
Conversation
This was referenced Oct 15, 2026
Draft
Open
Closed
9 tasks done
11 tasks done
Contributor
|
Windows / AMD results for this head, in a scene where queue worker 0 is the limit: ASTRO BOT (PPSA21564 01.007.000), Host: Windows 11 Pro 26200, Ryzen 7 5800X3D, Radeon RX 9070 XT (AMD proprietary driver 32.0.23033.1002), 32 GB, WinLibs GCC 15.2.0 r7, Release. Builds.
Method. No input, warm shader cache (1,904 hits, 0 misses),
Seven launches with this PR went through boot, loading and at least 3 minutes in the hub: the two measured above, and five more held 3 minutes each, at 6.62-6.67 FPS. None had a stop, a hang or an error in the log. AI-assisted (Claude Code); the builds and runs are on my machine. |
meh7an
force-pushed
the
perf/agc-draw-path-locks
branch
from
October 15, 2026 11:29
6170a0c to
54b0ce0
Compare
11 tasks done
|
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.
|
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 15, 2026
… report boykopovar#6292 moves the APS5_PROFILE_DRAW check in Driver::timed to the top and rewrites the report's condition, the two lines this branch's driver-time line sat between, so the two conflicted. The line now follows the report. It reads the same flag and times, so it counts the same.
This was referenced Oct 15, 2026
This was referenced Oct 15, 2026
Contributor
|
Follow-up on the integration/conventions blockers at head 54b0ce0: I prepared a candidate against main a5e2a80. The actual Git merge has one include conflict in DepthSurface.cpp. The candidate keeps the existing GuestMemory.hpp include and adds Mutex.hpp once. It also replaces the two hand-written TLS imports with <processthreadsapi.h> inside the existing Windows x86-64 GNU guard, and removes three prose comment lines in GuestMemory.cpp flagged by the comment policy. The mutex algorithm, spin count and TLS load/store code are unchanged; my earlier waitStart clock patch is separate. Validation:
The attached bundle includes the three resolved files, checked application helper, exact-revision instructions, probe source, both tested headers and results. This is a local integration candidate, not a push to your branch. Full emulator build, CTest, other platforms, GPU/title execution and FPS were not tested; hosted CI still needs to run after integration. Prepared and reviewed with AI assistance. |
12 tasks done
meh7an
force-pushed
the
perf/agc-draw-path-locks
branch
from
October 15, 2026 16:37
54b0ce0 to
adcaa64
Compare
Contributor
Author
|
Thanks, all. Rebased onto main 92e5041 as adcaa64, with these changes:
On the spin, I tried dropping it (
Each failure is the test's "the sentinel fill did not land": the fill is not in guest memory yet when |
11 tasks done
meh7an
force-pushed
the
perf/agc-draw-path-locks
branch
from
October 15, 2026 19:17
adcaa64 to
091a0cc
Compare
Contributor
Author
|
@simonbronk, the spin is gone now (091a0cc, rebased onto main 44df216): a contended lock waits on the word at once. The race it was hiding turned out to be main's, #6512. In On main 44df216 with #6522 and this PR without the spin, runs of that test:
The full suite passes 1299 of 1300 there; the one not run is |
Contributor
|
@meh7an thanks for the update. Here is a title measurement of 091a0cc on Windows. It's inconclusive for this scene. Onimusha: Way of the Sword (PPSA27836), Windows 11 26200, RTX 5070 (NVIDIA 617.42), WinLibs GCC 15.2, Release. Base:
AI-assisted: Claude Code. |
This was referenced Oct 15, 2026
This was referenced Oct 16, 2026
11 tasks done
12 tasks done
12 tasks done
meh7an
force-pushed
the
perf/agc-draw-path-locks
branch
from
October 16, 2026 18:59
091a0cc to
7ffce1c
Compare
meh7an
force-pushed
the
perf/agc-draw-path-locks
branch
from
October 16, 2026 19:21
7ffce1c to
c14f4c4
Compare
This was referenced Oct 16, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On Windows the driver is built with MinGW:
std::mutexlocks and unlocks through winpthreads (pthread_mutex_lock,pthread_mutex_unlock);thread_localis emulated: every access calls__emutls_get_address, which callspthread_getspecific.A draw takes a few dozen different mutexes, and every packet reads several thread-locals. On the S2 page of Sonic Origins Plus's main menu, the queue worker spent these samples per frame (APS5 sampler, 15 s):
pthread_mutex_lock1.22,pthread_mutex_unlock0.82,__emutls_get_address2.15 (1.80 of it inpthread_getspecific), and the device use gate 0.37. That is about 10% of its frame.AgcDriver::Mutex(Execution/include/Mutex.hpp) is an inline mutex on one atomic word.std::atomic::wait, which spins 12 pauses and 4 yields itself before it sleeps). Likestd::mutex, it is neither recursive nor fair.std::mutexwith 4 to 8 threads on one lock, and that waiting at once beatsstd::mutexat every thread count. Without the spin, main'sagc_driver_multisample_fill_host_onlyfailed most runs: waiters that sleep at once widen a race main already has (agc_driver_multisample_fill_host_only fails intermittently: the sentinel fill after an absorbed multisampled clear does not land #6512), a multisampled target's clear recorded on another device's recorder. fix(agc): record a multisampled target's commands on its own device #6522 fixes that race, so this PR depends on it. The game numbers below come from the spinning version; on that page the contended path takes no samples (0.00 per frame in three sampled runs of my fork's dev), so they hold.std::mutexes of the draw path. perf(agc): pipeline queue-0 draw commits behind the packet worker #1201 already made two more aSlimMutex(the pending storage writes and the unit shadows), and those stay as they are.std::mutex: the five astd::condition_variablewaits with, and the sparse and settle queue mutexes, which the context hands out asstd::mutex*.DeviceUseGatekeeps its user count and the replacement flag in one atomic word instead of a mutex and a condition variable. A packet enters and leaves with one atomic instruction each.GpuMutextakes anAgcDriver::Mutexand counts its own recursion. Its owner token is the thread environment block's address (CurrentThreadToken) instead of a thread-local's.HostThreadSlot(libc/include/HostThreadSlot.hpp) holds a per-thread value of up to 8 bytes.TlsAlloc), read and written through the thread environment block in one or two instructions. Elsewhere it is athread_local.ThreadOwned).Driver::timedreads no clock unlessAPS5_PROFILE_DRAWasks for its report. A wait packet reads it only forAPS5_TRACE_GPU's slow-wait line (suggested by @simonbronk).agc_driver_locks(tests/execution/Locks.cpp) checks:try_lock, and a sleeping waiter's wake;With this change, per frame:
pthread_mutex_lock1.22 to 0.31,pthread_mutex_unlock0.82 to 0.20,__emutls_get_address2.15 to 1.48, and the device use gate 0.37 to 0.05. What remains is the mutexes kept or outside the draw path, and thread-locals this change does not touch.Tested
Windows 11 (10.0.26300), WinLibs GCC 15.2.0 (UCRT), Release, NVIDIA GeForce RTX 2080 Ti, driver 617.14.
drawCacheMutex, main added the draw relocation fields (perf(agc): find cached draws across moved user-data pointers #4943), and perf(agc): journal the pipelines of a run and build them ahead on the next one #5875 keyed the compute pipelines byComputePipelineKeyand dropped the pipeline store'sscratchKey.drawCacheMutex,computePipelinesMutexand the pipeline store's mutex stayAgcDriver::Mutexes, and every lock of them is astd::lock_guardorstd::unique_lockas before. The collect memo ring goes through its thread slot as before. perf(agc): pipeline queue-0 draw commits behind the packet worker #1201's per-epoch collect map next to it keeps itsthread_local, like the other thread-locals main added since.tools/check_conventions.pypasses:HostThreadSlottakesTlsAllocandTlsSetValuefrom<processthreadsapi.h>, and the GPU mutex's unlock keeps main's comment where it was.agc_driver_gpu_indirect_arguments_misaligned_writer_ahead, which main itself does not compile with MinGW), including the newagc_driver_locks.agc_driver_multisample_fill_host_onlyfailed 0 of 200 runs (8 at a time) on main 72c5eed. On 44df216 it also failed 0 of 400 withAPS5_PIPELINED_DRAWS=0(4 at a time) and 0 of 40 alone, and without fix(agc): record a multisampled target's commands on its own device #6522's line it failed 40 of 200 and 7 of 40.APS5_TRACE_SECONDS=200:15on the same machine in one session, caches warm. Traces attached.This pair overstates the change. The baseline's window ran slow outside the draw path:
Driver.Packetminus its draw part is 37.0 ms against 11.3 ms with this change. Its title FPS, 6.70, is also below every other run of that build (7.51 to 8.34). The sampled pair, 7.75 to 8.38 FPS (+8%), is the better estimate.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)Traces and summary lines (baseline b8acbb0, this change 824084e):
locks-traces.zip