Repository navigation
perf(agc): copy a descriptor region behind pending GPU work on the GPU whatever its size - #13263
EagleBearerYS wants to merge 2 commits into
Conversation
…U whatever its size A misaligned descriptor region inside a host import is copied out of the import by the GPU only up to APS5_GPU_COPY_MAX_KIB (1 MiB). A larger one was read on the CPU, and when a pending GPU write, pending storage results or a unit shadow covered it, that read waited in the flush hook for the producer's batch (the [hooksync] "buffer-upload" syncs). Such a region now takes the GPU copy whatever its size: the copy is recorded after the producer in batch order, behind the barrier and the pending-results flush the GPU path already records, so the bytes are the same and the CPU does not wait. UploadFinish asks the recorder under the GPU mutex; UploadPrepare, which holds no lock, asks the published pending-write snapshot. Larger regions behind no GPU work keep the CPU copy, which never waits for them. Completion write-backs and labels over the range are still synced first by CompletePendingCpuWrites, so a pending write the copy sees has landed in the import on the GPU. The test (agc_buffer_upload_pending_gpu_copy) uploads a 3 MiB misaligned region over a GPU write still in the open batch: the upload makes no flush-hook wait and no submission, and a GPU read of the bound buffer holds the producer's bytes.
github-actions
Bot
added
perf
|
This pull request is labeled |
|
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.
|
github-actions
Bot
added
build
docs
agc
The conflict in core/libs/prx/libSceAgcDriver/CMakeLists.txt from boykopovar#12862's split is resolved with tools/agc_test_split.py resolve, which moves this PR's test registrations into tests/cmake/.
CnRJay
left a comment
CnRJay
left a comment
There was a problem hiding this comment.
The perf(agc) title makes a speed claim, and the evidence for it does not meet the bar yet.
-
No
tools/trace_digest.pysummaries. The only numbers areAPS5_PROFILE_DRAW=1[hooksync]lines formain("65 syncs, 238 ms", "29 syncs, 364 ms" per 10 s). Those say the waits exist, not that removing them makes the scene faster. Please attach trace_digest summaries ofmainand of this branch from the same machine and scene (frame p50/p90/p99, GPU busy per present), with the RE Village menu named by title ID and a log line showing the large pending-write upload is reached onmain. -
No hardware GPU. "GPUs tested" lists llvmpipe/lavapipe only. The change turns a CPU
Readinto avkCmdCopyBufferof unbounded size into the open batch, so the cost moves to the GPU timeline. On a discrete card that copy reads from a host-imported system-memory range over PCIe, and withAPS5_GPU_COPY_MAX_KIBremoved as a bound for this case a 3+ MiB (or much larger) descriptor range is copied on every build behind pending work. Please run it on a real GPU and show GPU busy per present does not rise. -
The size bound is removed, not replaced.
copyBehindGpuWorkhas no upper limit and no per-frame budget. Is there a case where the pending overlap is a few bytes inside a very large descriptor range (PendingWriteOverlaps/AnyShadowedOverlapsonregion.begin..region.end)? Then the whole region is GPU-copied to avoid waiting for a small producer. A test with a small pending write inside a large region, counting copied bytes, would show whether that is bounded.
|
@CnRJay, thanks. This stays a draft until a pair decides the speed claim, and points 1 and 2 are what it needs before it's ready: On point 3, from the code: as you say, the size bound is removed for this case, not replaced. When a pending write, a pending storage result or a shadowed range overlaps any part of the region, the GPU copies the whole region behind that work. For a region over the bound, |
|
PPSA22408 (Hot Wheels Infinite Rush), garage menu: no effect from this PR, and where the GPU copy is skipped Linux, RTX 3080 Ti (NVIDIA 615.71.09), Ubuntu 26.10, Linux 7.3, Release. Same route into the garage each run, default settings, headless,
Both arms contain the same local diagnostic (a stack counter on the flush hook's GPU waits). It is not part of any PR. A and B were alternated, two runs each, 266 s per run, and the table is the garage window (about 110 s on).
The row this PR targets is unchanged. In the first pair of runs, Why the GPU copy isn't reached. I printed the region at the The region passes this PR's eligibility test ( So for this title the limit isn't the size cap or the eligibility rule. It is import coverage of this region. A question for the author: should the GPU copy be possible without a covering import, for example from a copy of the guest bytes? I can paste the four AI-assisted. |
|
@mrfelek, thanks for the pair. It answers the question for Hot Wheels Infinite Rush (PPSA22408): this PR doesn't reach that region. Your guess about the lease matches the code. In On a GPU copy from a copy of the guest bytes: making that copy is the CPU read this PR avoids. The bytes it needs are the ones the pending write produces, so taking them on the CPU means waiting for that write first, as The case that can still go on the GPU is a region that crosses the boundary between two registered allocations, each with its own import. Utku Halis's e10f59a (in #12082, which its author closed) splits such a region into the imports it covers and copies it piece by piece. We keep it, with a follow-up for the pending-work case, on The summary lines aren't needed for this one, since the pair shows no change. This stays a draft: its speed claim still needs a Resident Evil Village (PPSA01556) pair, which isn't planned yet. |
What
When a dispatch's or draw's build uploads a descriptor region that isn't aligned inside its host import, the driver can copy it on the GPU from the import (
recordGpuCopies) instead of reading it on the CPU. Onmainthat GPU copy is allowed only up toAPS5_GPU_COPY_MAX_KIB(1024 KiB by default,gpuCopyEligible). A larger region takes the CPU copy incopyRegion, and itsGuestMemory::Readgoes through the flush hook. So when GPU work is still pending over the region, the CPU waits for it, under the GPU mutex inUploadFinish:This change copies such a region on the GPU whatever its size, when there is pending GPU work over it: a pending write, pending storage results, or a unit shadow.
Why that's safe: the bound isn't there for correctness. The GPU copy uses the same host-visible buffer as the CPU copy, and for a region with no GPU work over it the CPU copy never waits, so keeping that region on the CPU stays the cheaper choice. Behind GPU work, the GPU path already records everything the CPU would have waited for:
UploadFinish.How it decides:
copyBehindGpuWork(region, locked): holds for a GPU-copyable region the active recorder has GPU work over.Unchanged:
APS5_CPU_COPIES=1, which disables every GPU copy.The
APS5_GPU_COPY_MAX_KIBrow inENVIRONMENT.mdnow says the cap doesn't apply behind GPU work.Pending:
Resident Evil Village (PPSA01556)
Static menu, profiled boot (
APS5_PROFILE_DRAW=1) onmainebb4075 plus #12478. Its[hooksync]lines show these buffer-upload waits per 10 s:0x0 DISPATCH_DIRECT buffer-upload, 65 syncs, 238 ms, average access 11,632 KiB over an average noted range of 3,162 KiB;0x28 DISPATCH_DIRECT buffer-upload, 29 syncs, 364 ms.Not confirmed yet: whether those regions lie inside one host import, which is the case this change covers, or cross several registered allocations, which it doesn't. A profiled boot with this change will tell, from its
[buffers]line and thebuffer-uploadsyncs left.Tested
agc_buffer_upload_pending_gpu_copy(new), run asagc_driver_recorder_tests --pending-upload-gpu-copy-only. The full recorder run includes it too.vkCmdCopyBufferin the open batch writes a 1 MiB pattern at +1 MiB, noted as a pending write.mainplus the test only): "the upload of a misaligned 3 MiB region over a pending GPU write waited for it on the CPU (1 flush-hook waits, 1 submissions)".ctest -R '^agc'once, 1,136 tests: 1,060 passed, 75 skipped, 1 failed. The one failure,agc_driver_image_sample_force_degamma, is that VM's known llvmpipe JIT illegal instruction (vpdpwssd) and fails onmainthere too.main's code,agc_buffer_upload_pending_gpu_copyfails with the message above, and so doesagc_driver_recorder, the full recorder run that includes it.VUID-VkDeviceCreateInfo-pNext-pNext, whichmain's run reports too, so the fix adds none. The layer loaded in both runs. In this modemain's side runsmain's own tree, so the new test is only in the fix's side.check_conventions.pyandcheck_test_registration.pypass. MinGW-w64-fsyntax-onlyshows no new diagnostic.Overlap:
recorder->ReadBuffer. That's still a CPU read under the same site, so it composes with this change.main0a148be (run). build(agc): register each AGC test executable in its own CMake fragment #12862 movedlibSceAgcDriver's test registrations intotests/cmake/; the conflict inCMakeLists.txtis resolved withtools/agc_test_split.py resolve, which moves this PR's registrations intotests/cmake/agc_driver_recorder_tests.cmake(+6 lines). Nothing else conflicted, and apart from the move this PR's diff is unchanged. With-DBUILD_TESTING=ON,ctest -Ngives 3106 tests:main's 3105 plus its 1 test, each listed once. The tests of this PR's verify set pass 10 runs in a row. Linux: 5 run, 5 pass, 0 skip, 0 fail. Windows: 5 run, 0 pass, 5 skip (no Vulkan device), 0 fail. Suites: libc and libkernel pass on Linux (199 and 308) and on Windows (226 and 330). The suites ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.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)