Repository navigation
Conversation
CnRJay
reviewed
Oct 14, 2026
Contributor
There was a problem hiding this comment.
Correction (2026-11-01): the title-ID/log demand below is not a CONTRIBUTING requirement; Unblocks: none is allowed. The technical points below are unaffected.
I think this has the same problem the review of #4868 raised for indirect draws. main already makes both of these decisions: DispatchRecipe / dispatch report outcome.cpuReason as IndirectNotImported or IndirectPendingImage, and countIndirect records them. This makes the same decisions earlier, at a cost:
RegisteredReadableCoversandStorageTexture::FlushPendingnow run on every GPU-side indirect dispatch, before the path the GPU would have taken anyway.FlushPendingreturning true means pending storage-image results were written back to guest memory: a GPU→CPU synchronization on the queue thread. The CPU then reads the arguments and issues a direct dispatch. That turns a GPU-resolved indirect dispatch into a full round trip. The point of the GPU indirect path is that the producer's results stay on the GPU and only a barrier separates the two dispatches. A flush can't take the "~1.3 µs per dispatch" in the table.- The new debt entry says unregistered arguments "fall back to CPU-side execution" and talks about unmapped buffers. "Not covered by a host import" isn't the same as unmapped:
ReadDispatchArgumentsreads mapped guest memory that just has no Vulkan import. If the memory really is unmapped, the CPU read faults; it isn't a fallback. As it stands the entry documents behaviour that this code doesn't have. - The added test covers the registered path, which
mainalready takes. It doesn't show a case that behaves differently with the change.
As with #4868, please reopen this with a title that reaches these paths. For a perf( change, also attach the aps5-summary.jsonl lines and traces showing that the earlier decision is cheaper than the one main makes.
Merged
11 tasks done
Kikoarl
changed the title
Contributor
Author
|
putting this pr on hold while i re-evaluate telemetry and profiling on full in-engine scenes. |
Kikoarl
marked this pull request as draft
October 14, 2026 12:04
|
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.
|
This was referenced Oct 15, 2026
Merged
Closed
11 tasks done
12 tasks done
12 tasks done
Contributor
Author
|
closing to clear obsolete telemetry and rebase overhead; will revisit once in-engine profiling is ready. |
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
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
Pre-checks memory registration and flushes pending storage images ahead of indirect compute dispatches (part 3 of the indirect decoupling requested by @oneandonlydean in #4868):
dispatchIndirect(IndirectDispatch.cpp), checksGraphics::RegisteredReadableCovers(arguments, 12)before submitting unresolved packets to the GPU. If unregistered or unmapped, routes toIndirectNotImportedto perform a safe CPU-side argument read and direct dispatch.dimX,dimY,dimZ), flushes them viaGraphics::StorageTexture::FlushPending(arguments, 12, nullptr, "indirect dispatch arguments")and routes toIndirectPendingImage.docs/dev/debt/libSceAgcDriver.md.tests/execution/IndirectArgumentsWrittenAhead.cppto exercise both the unregistered (CPU-fallback) and registered (GuestAllocations) indirect dispatch execution paths.Performance & Telemetry (Astro Bot PPSA21567)
Measured across alternating 10-second runs on Astro Bot intro/gameplay transition scene against base
8299b2f7e:main(8299b2f7e)main + PRmain(8299b2f7e)main + PRmain(8299b2f7e)main + PRTested
PPSA21567)agc_driver_indirect_arguments_written_ahead_tests(passes via ctest).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)