Repository navigation
Conversation
Zenodeon
marked this pull request as ready for review
October 17, 2026 22:42
|
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.
|
adriadam10
requested changes
Oct 19, 2026
adriadam10
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for splitting this out. The mechanism reads as bounded and safe (timeout, prefix-only REWIND ack, flips/conditionals excluded), but a few things need fixing before it can merge:
- No measurement on current main. The PR body says the numbers come from a frozen integration binary with other PRs and local changes, and are "not current-main versus PR measurements". A performance change needs the main vs PR summary lines from the same machine and scene, with traces, as the PR template asks. Please add that (ideally with
APS5_SUBMIT_PROGRESSon and off on the same build), or say what the feature gives without the other patches. - Scheduler-dependent assertions in
tests/execution/SubmitProgress.cpp:89(ready >= 48of 64) and:122(prefixesReady >= 12of 16). The PR body notes they are sensitive to host scheduling; on shared CI runners and lavapipe they will flake. Keep the invariants that hold regardless of timing (no lost completion store, no early REWIND continuation, bounded return) and drop or loosen the readiness counts. - Technical debt placement.
docs/dev/debt/libSceAgcDriver.mdgets a new### Experimental submission progresssection at the top, which isn't one of the file's headings, and the text describes the feature rather than a gap. Keep only what is actually unverified or deviating (e.g. the 2 ms budget is a guess, no hardware validation across titles) as short lines under the existing headings.
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
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
Add experimental, off-by-default
APS5_SUBMIT_PROGRESS=1pacing for eligible graphics-queue submissions. A guest that immediately polls a terminal GPU release can otherwise return from submit before its small batch is recorded, miss the release, and enter another sleep/poll cycle.An internal per-submission ticket acknowledges the recorded prefix and its actual recorder timeline. The submitting call shares a nominal two-millisecond budget between waiting for that acknowledgement and waiting for that specific timeline value. Expiry or an unavailable timeline retains asynchronous acceptance; neither reports GPU completion nor retires work. OS scheduling can overshoot the requested timeout. No whole-device wait, guest sleep change, exported API/NID change, or shader change is added.
Eligibility excludes flips, conditional ranges and unproven CP waits. A preceding nonpredicated literal memory write may prove a 32/64-bit wait's compared bytes using the existing PM4 comparison; the real wait still executes. Unknown writes and intervening GPU draw/dispatch work discard that proof. A final REWIND acknowledges only its finite prefix: its continuation and logical completion remain blocked until the guest releases them. Current-main queue-zero completion wakeups are retained.
This is a standalone extraction of the local SuperTuxKart PPSA69421 experiment. No resource-ownership, residency, selected-image-import, write-watch, game configuration or shader experiment is included. No dependency on our earlier PRs is needed.
Overlap review: the refreshed bot report identified a textual conflict with #6507 at the include/helper insertion points in
Submission.cpp. Follow-up544d5ae8brelocates the unchanged progress helper and its include. Before the change, combining both PRs overmain9caecfd2dconflicted in that file; afterward, the combined merge tree is clean and retains both implementations (#6507 headdb091f5fb). No #6507 code is included and no dependency is introduced. #6867 waits for referenced shader preparations, #6507 changes CPU flip scheduling, #5925/#6188 compile ahead, and #6946/#6947 change draw/descriptor preparation; those mechanisms remain separate.Tested
Source base
b4f05ab32, original validated candidate8582ce0e69deb768bf3ce8b5ba3aa73ea9b34e2a, current head544d5ae8b58d04614d800c67f71b80cfc7bf5081(still merges cleanly with fetchedmain9caecfd2d). GCC 15.2, Ninja, Release (-O3 -DNDEBUG),BUILD_TESTING=ON,ANYPS5_ENABLE_SPIRV_TOOLS=ON, bundled dependencies.libsrebuild; 21/21 focused tests pass with the surrounding suite configured off, and 21/21 on, zero skips. Coverage includes resource uploads/write-back/retirement, arrays/aliases/clears, graphics, pixel ancillary/fixed-point semantics, REWIND watchdog, nanosleep and trace handling.origin/mainandgit diff --checkpass. No full-suite, fresh cross-title, completed-race or 15-minute stability acceptance is claimed.Historical investigation only, not a clean-main performance result: a single short Abyss/Tux/easy/four-kart, 1920x1080 pair on the same frozen integration binary
2f26d7b4051f, feature absent/present, gave:tools/trace_digest.pysummaries of that historical integration pair:Both arms held the legacy #6759/#1191/#6820 integration, local ownership and TheAggie's shader revision
45944fbbe0dcconstant. These changes are outside this PR, and their effects are not credited here. The preserved complete runtime and trace hashes were verified locally.One 12-second pair does not establish repeatability; the user also observed later drops toward 10 FPS. These are not current-main versus PR measurements, nor a claim that this standalone branch delivers the same FPS. Required acceptance-length comparisons, cross-title coverage and public trace evidence remain unavailable; traces/evidence bundles are intentionally not uploaded. This PR requests review of the mechanism and trade-offs, not performance acceptance or default enablement. Pacing may reduce useful CPU/GPU overlap in other workloads.
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)