Repository navigation
fix(agc): fail a template's proof on a released texture and audit it with the full walk - #8047
Conversation
|
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.
|
|
Results on a current-main base, with this PR's driver swapped in and nothing else changed. Base:
No crashes in the three runs with the PR, and no storage cache evictions in any of the five, so none of them was rescued by an eviction. Small sample, but the base crawled in both runs that reached the level, and the PR runs were normal from the first level window. AI-assisted: yes |
|
Thanks for the current-main comparison. Two base runs crawled while the three PR runs were normal from the first level window, and none of the five had storage-cache evictions; that is useful evidence for the audit change, with the small sample size clearly stated. The description still says more current-main runs are queued. Please fold this table and its limitation into the Tested section so the PR summary reflects the new result. |
|
Done, the Tested section now has the five-run table result and the small-sample note. AI-assisted: yes |
…with the full walk The fast proof of a cached ShaderResources proves each sampled texture from write stamps, keys and the pending registry. It never asks whether the sampled lookup would still return the bound Texture, so a template whose surface memory is unchanged keeps binding an old object: - After the sampled cache drops or replaces the entry (boykopovar#6492 marks the texture released), Find keeps serving the template until the next Insert, and its fast proof passes. - When the lookup's answer changes without any lookup running (a snapshot whose surface became host-imported is viewed through its storage image from then on), nothing replaces the entry and the template keeps the snapshot until it leaves the resource cache. The fast proof now fails with 'replaced texture' when a bound texture is released. A template whose last full walk is older than APS5_TEMPLATE_AUDIT_MS (default 1000, 0 turns it off) takes the full walk instead of the fast proof ('audit'), so a different lookup answer fails the proof and the template is rebuilt.
…ebuild agc_driver_resource_cache_replaced_texture clears the sampled cache under a template that takes the fast proof and requires the next proof to fail. agc_driver_resource_cache_lookup_changed checks that a proof past the audit interval takes the full walk and keeps an unchanged template, then host-imports the surface (no lookup runs) and requires the template to stop proving current. Both run over a write-watched surface so the template takes the fast proof.
b34b670 to
3c43ce3
Compare
|
Thanks for this. One small change to the new test, please.
Separately, the time-based audit (part 2) changes behaviour for every title, so it is going to the maintainer for a design call. The Released check (part 1) looks good. |
|
Windows runs of the tests (the PR ran them on Linux only), on NVIDIA and on an AMD iGPU. Setup: Windows 11 26300, MinGW GCC 15.2 Release,
No title runs here, so the Demon's Souls result and the 2-3% audit cost are not re-measured. One small point from reading the code: AI-assisted: yes (Claude Code); results above were run on this machine. |
|
I checked exact head |
Co-authored-by: Joey Fisher <fisherjoey@ymail.com>
… skip resource-lookup-changed skipped with its own texts when the device has no host imports or refuses the import of the watched surface. Neither matches DEVICE_LIMITATION in tools/check_skips.py, so a runner without host imports would fail the job instead of recording a device skip. Both now print the existing "skipped, host imports are unavailable".
|
Both skip messages in I meant to rebase onto main, but the branch already had main merged in (1cb6482), so I added the commit on top of that merge instead of force-pushing over it. The branch merges cleanly with current main 286908f. On this head, Thanks @wachi35676 for the Windows runs on NVIDIA and AMD, and for the negative control. AI-assisted: yes |
4 participants
What
A cached
ShaderResources(a dispatch or draw template) is reused whenRevalidateproves it current. The fast proof checks each bound sampled texture's memory, keys and pending writes, but not whether the sampled lookup would still return the boundTexture. A template over an unchanged surface can therefore keep an object the lookup no longer returns (#8040). This PR makes two changes.fastRevalidateandfastRevalidateEachfail, with the new reasonreplaced texture, when a bound texture isReleased(). That is the flag fix(agc): drop resource-cache entries that hold released sampled textures #6492 sets when the sampled cache drops a texture something still holds. fix(agc): drop resource-cache entries that hold released sampled textures #6492 drops such templates at the nextResourceCache::Insert. Until that Insert,Findstill served them and their fast proof passed. With this change the full walk runs, its lookup returns the new entry, the proof fails, and the template is rebuilt. Cost: one atomic load per bound texture per proof.APS5_TEMPLATE_AUDIT_MSold (default 1000 ms,0turns it off) takes the full walk in place of the fast proof, with the new reasonaudit. This covers the case nothing marks: the lookup would now return something else for an unchanged surface, but no lookup has run, so no entry was replaced. For example, a snapshot's surface becomes host-imported and the lookup would view its storage image instead. The walk repeats the real lookups, a different answer fails the proof, the template is rebuilt, and the entry that lookup replaced is released, which then fails every other template binding it. An unchanged template passes the walk and goes back to the fast proof. The interval is a variable so the tests can use 250 ms, and it has a row indocs/dev/ENVIRONMENT.md.Fixes #8040
Tested
agc_driver_resource_cache_replaced_texture: a template over a write-watched snapshot takes the fast proof, the test clears the sampled cache without touching the memory, and the next proof has to fail. With main'sShaderResourcessources it fails on lavapipe witha template binding a texture whose cache entry left still proved current. With this change it passes.agc_driver_resource_cache_lookup_changed(250 ms interval, skips without host imports): past the interval, an unchanged template takes the full walk once, keeps its texture, and the next proof is fast again. Then the test host-imports the surface with no lookup in between. The proof past the interval has to fail, and the rebuild has to bind the storage view. With main's sources it fails on lavapipe witha proof past the audit interval did not take the full walk. With this change it passes.agc_driver_sampled_cache_budget_tests,agc_driver_recorder_tests,agc_driver_image_table_cache_bounds_testsandagc_driver_draw_cache_reuse_testson this branch (main f52509a), Linux, Release, GCC 16. On lavapipe (Mesa llvmpipe): 61 pass and 5 skip for missing capabilities. On an NVIDIA GeForce RTX 5070 Ti (driver 595.91.07,ANYPS5_REQUIRE_VULKAN=1): all 66 pass. The two new tests passed 20 of 20 repeats on lavapipe and 5 of 5 on the RTX 5070 Ti. The Khronos validation layer isn't installed on this machine, so I haven't done the validation-layer comparison against main yet.tools/check_conventions.py --base origin/mainpasses.ImportWatchedpage-digest change from fix(agc): hash the pages of an import window instead of copying them #7981, a guarded unit-shadow publish run in its wait-only mode (APS5_WB_GUARD_PUBLISH=wait, the same idea as fix(agc): wait for the idle flush's unit-shadow publish before returning #7984), and diagnostics that were off in these runs. The fix on that base was this audit plus an in-cache flag in place ofReleased(). That flag was the first version of the first change; fix(agc): drop resource-cache entries that hold released sampled textures #6492's flag marks the same erases.libSceAgcDriver.prxrebuilt with this fix (so the shader cache stayed warm): all three runs were normal from the first level window (median 213 to 214) at 8.84, 9.21 and 9.38 presents per 10 s, with no crash over the full 900 s. None of the five runs had a storage-cache eviction, so no run was rescued by one. Small sample: 2 base runs and 3 with the fix.Related
Released()and the lazy drop atInsert. This PR reuses the flag in the proof and adds the audit for the case where nothing is released.fastRevalidatenear these lines. It doesn't add an identity check, so the two don't overlap in purpose, but whichever lands second may need a rebase.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); this change adds no stubs.mdfiles or images added to the repository (attach them to this PR)AI-assisted: yes