Skip to content

test(agc): give the store idle results test the deadline its siblings use - #7992

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
MarrasFederico:fix/agc-store-idle-results-deadline
Oct 24, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
MarrasFederico:fix/agc-store-idle-results-deadline

Conversation

@MarrasFederico

@MarrasFederico MarrasFederico commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What

agc_driver_store_idle_results fails on a loaded host with AGC graphics: the queue-idle store did not bring the result to guest memory without a direct flush or further submission. It waits 2 s after its submission for the stored texels, and with --idle-epoch-wake also 2 s for the pending storage results. That window also holds the driver start and the compute program's compilation at dispatch (ANYPS5_NO_SHADER_CACHE=1), so the time grows with the load on the host, not with a driver timer. Measured from the store to the texels reaching guest memory on an RTX 5060 Ti, with N instances started at once:

Instances Time to the stored texels
1 341 to 375 ms
4 1032 to 1068 ms
8 2001 to 2098 ms
16 4203 to 4463 ms
24 6240 to 6792 ms

The other execution tests that wait on a deadline after Submit give it 10 s (AliasCopyLabel, PipelinedDispatch, PipelinedDma, PreparedShaders, StoreOverUntracked, UnimportedLabelOrder, FlipStoresSmallResults from #7774, among others). Both waits here now use 10 s. The driver is not changed.

Tested

Ubuntu 26.04.1, kernel 7.0.0-38, GCC 15.2.0, Release, full build with BUILD_TESTING=ON.

  • In a full ctest -j12 run on main the test failed once and passed when run alone.
  • 24 instances at once: 24 of 24 fail on main, 24 of 24 pass with this change. 16 at once: 16 of 16 pass with this change, with and without --idle-epoch-wake (on main, --idle-epoch-wake passed 4 of 8 at 8 instances and 0 of 16 at 16, stopping at idle wake fixture did not observe pending storage results).
  • ctest -R '^agc_driver_store_idle_results$' -V on main and on this change with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation on each device below: passed on both, no VUID- in either log, and neither log contains was not found but was requested by env var VK_INSTANCE_LAYERS.
  • tools/check_conventions.py --base origin/main passes.
  • Overlap with fix(agc): wake queue 0 when storage results are pending #7535: tools/pr_overlap.py reports it on StoreIdleResults.cpp L113-122. fix(agc): wake queue 0 when storage results are pending #7535 moves the same wait block under its new --async-queue-wake branch, so the two conflict textually there; the changes themselves are distinct (timeout length here, queue wake fixture there). No merge order is required: whichever lands second keeps fix(agc): wake queue 0 when storage results are pending #7535's block with this PR's 10 s waits. fix(agc): wake queue 0 when storage results are pending #7535's new 2 s wait for queue 0 to go dormant could then take the same 10 s.
  • Combined validation: both heads merged onto main 524398151f with that resolution. On Intel Arc (MTL), Mesa 26.0.8 ANV, Ubuntu 26.04.1:
    • agc_driver_store_idle_results and agc_driver_store_idle_results_async_queue_wake pass with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation;
    • the log has no VUID- and no was not found but was requested by env var VK_INSTANCE_LAYERS;
    • 8 instances at once of each variant pass.

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 under Silent stubs in their library's technical debt file
  • 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
  • Fixes: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 5060 Ti, driver 595.99.02, Ubuntu 26.04.1; NVIDIA GeForce RTX 3060, driver 595.99.02, Ubuntu 26.04.1; lavapipe (llvmpipe, LLVM 21.1.8), Mesa 26.0.8, Ubuntu 26.04.1
  • AI-assisted: yes

… use

agc_driver_store_idle_results waits 2 s after its submission for the
stored texels (and, with --idle-epoch-wake, for the pending storage
results). That window also holds the driver start and the compute
program's compilation at dispatch (ANYPS5_NO_SHADER_CACHE=1), so on a
loaded host the result arrives after the deadline: alone it takes about
350 ms on an RTX 5060 Ti, with 8 concurrent instances about 2.0-2.1 s,
with 16 about 4.3 s and with 24 about 6.4 s.

Wait 10 s, as the other execution tests that wait on a deadline after
Submit do. The driver is not changed.
@github-actions github-actions Bot added the agc libSceAgc* label Oct 19, 2026

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

The current Tested section says tools/pr_overlap.py reports no overlap, but the PR already has an overlap report identifying shared StoreIdleResults.cpp lines 113–122 with open #7535. I compared the current heads against main: their combined tree is clean, and the changes are distinct (timeout length versus the async queue wake fixture), but both touch the same test flow. Please correct the overlap statement and record the coordination/combined validation; add a dependency only if the merge order is actually required. I’m holding a formal approval until the PR description matches the current overlap result.

@MarrasFederico

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks, you're right: the overlap statement was wrong, and I've corrected the Tested section. Merging both heads onto current main 524398151f does give a textual conflict in StoreIdleResults.cpp L113-122, because #7535 moves the same wait block under its new --async-queue-wake branch. The resolution is #7535's block with this PR's 10 s waits.

With that resolution, on Intel Arc (MTL) with Mesa 26.0.8 ANV:

  • both agc_driver_store_idle_results tests pass with the validation layers loaded and no VUID-;
  • 8 instances at once of each variant pass.

No merge order is required, so I left Depends on as none: whichever lands second takes the other's block. #7535's new 2 s wait for queue 0 to go dormant could then use the same 10 s.

@adriadam10 adriadam10 self-assigned this Oct 23, 2026
@adriadam10
adriadam10 merged commit d2a2889 into boykopovar:main Oct 24, 2026
10 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the two 2 s waits in StoreIdleResults.cpp (L115, L130) against the 10 s deadline the sibling execution tests use and the PR body; the earlier overlap with #7535 is gone (it was closed); merged.

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

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants