Repository navigation
refactor(agc): split prepareDraw's read-only plan from its record-time work - #14051
vingelvatten wants to merge 1 commit into
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — changes requested
The checklist leaves this PR's overlap with open #10678 at Depends on: none, and neither PR's discussion records an agreed landing order.
What I ran
Reviewed head aa0880e4ebc0b1a705118028eb68fa93d69fd7a8. GitHub reports base 474030e58424fefdb65e5df9f3820810fbb4553c; I also checked the merge preview against the newer live main tip 0f302813cebfc09e3e0b6f8fced15262d186879e, which merged cleanly. Environment: Debian 12, Linux 6.1, Python 3.11.2, GCC 12.2.0. No local build or C++ tests were run.
python3 tools/pr_precheck.py --base upstream/main --head aa0880e4ebc0b1a705118028eb68fa93d69fd7a8 --title "refactor(agc): split prepareDraw's read-only plan from its record-time work" --body-file /tmp/anyps5-14051-review-current/pr-body.md --output-dir /tmp/anyps5-14051-review-final --no-negative-controls
git merge-tree --write-tree upstream/main aa0880e4ebc0b1a705118028eb68fa93d69fd7a8
gh run view 41222879876 -R boykopovar/AnyPS5 --job 124600944041 --log
grep -c 'plan_verify.*\*\*\*Skipped' /tmp/ci-14051-windows.logResults
| Check | Result |
|---|---|
Diff whitespace (git diff --check) |
Passed against live main 0f302813cebfc09e3e0b6f8fced15262d186879e |
| Repository conventions and PR template | Passed against live main |
| Merge preview | Clean against live main; preview tree 4678f46d088f119e2d4da4131c8f24e070d70972 |
| Hosted Windows build and CTest | Passed: 0 failed of 2,545 listed; all 22 new _plan_verify variants were skipped |
| Precheck's optional ABI/test-quality helpers | Unavailable in the extracted PR tree; no exports or Python tests changed, so these do not indicate a PR defect |
Unedited check output
All conventions checks passed.
4678f46d088f119e2d4da4131c8f24e070d70972
windows Build and run changed guest core/libs tests 2026-10-31T17:16:14.2462242Z 100% tests passed, 0 tests failed out of 2545
22
/usr/bin/python3: can't open file '/tmp/anyps5-14051-review-latest-main/head-aa0880e4ebc0-37cc6df8/tools/pr_abi_audit.py': [Errno 2] No such file or directory
/usr/bin/python3: can't open file '/tmp/anyps5-14051-review-latest-main/head-aa0880e4ebc0-37cc6df8/tools/pr_test_quality.py': [Errno 2] No such file or directory
Findings
Blocking
- Coordinate the shared implementation order with #10678. This PR moves the
DrawResourceKeyconstruction intoplanDrawand passes it throughresolveDrawResources; #10678 changes that same resource-resolution path to accept a finisher-builtcontentKey. The PR body itself says whichever lands second must rebase, but its checklist saysDepends on: none, and I found no agreement with #10678's author in either PR's discussion. The repository requires authors of overlapping PRs to agree which lands first and declare the dependency. Please agree on the order, then update the later PR'sDepends onentry accordingly.
Non-blocking
- Clarify the “read-only plan” contract.
planDrawcallsVertexInputCache::TryGet. That call can move an LRU entry, insert or evict cache entries, or set a bypass bit; the cache mutex protects those updates, so this is not by itself evidence of a data race. However, the description says the plan uses “no shared cache,” and the repeat-plan verifier compares plan outputs without checking those cache mutations. Please describe this synchronized cache interaction in the plan contract and verifier rationale, or move the cache operation outside the read-only stage if that property is required for the preparer boundary.
Sources
- Contribution rule for overlapping PRs, pinned to
0f30281 planDrawcache lookup, pinned to this headVertexInputCache::TryGetand mutatingget, pinned to livemain- #10678 resource-key handoff at its current head
Not verified
- The hosted Windows job skipped the 22 new
_plan_verifyvariants; Linux and specialized hosted jobs were skipped. I did not reproduce the author-reported RTX 4070 Ti SUPER, RDNA2 iGPU, or lavapipe runs. - No local C++ build, CTest, console run, or preparer-thread execution was performed.
To get this to a good state
Please agree with the #10678 author on landing order and update the dependency checklist on the later PR. Please also clarify the cache interaction in the plan contract. I will rerun the same checks on the updated head.
✅ NVIDIA GPU test: passedPR head
Failing on main too (4, not counted against this PR)Validated but not compared (22, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 428 tests)Skipped (50)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
|
@vingelvatten heads-up: #12862 split the AGC test registrations into The reason is that the split removed the function(agc_add_draw_pipeline_test agcTestName)
add_test(NAME ${agcTestName}_draw_pipeline COMMAND ${agcTestName}_tests)
set_tests_properties(${agcTestName}_draw_pipeline PROPERTIES ENVIRONMENT "ANYPS5_NO_SHADER_CACHE=1;APS5_DRAW_PIPELINE=1;APS5_PIPELINED_DRAWS=32;APS5_PIPELINE_DISPATCH=1" SKIP_RETURN_CODE 77 WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}")
if(WIN32)
set_tests_properties(${agcTestName}_draw_pipeline PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
endif()
add_test(NAME ${agcTestName}_plan_verify COMMAND ${agcTestName}_tests)
set_tests_properties(${agcTestName}_plan_verify PROPERTIES ENVIRONMENT "ANYPS5_NO_SHADER_CACHE=1;APS5_DRAW_PIPELINE=1;APS5_PIPELINED_DRAWS=32;APS5_PIPELINE_DISPATCH=1;APS5_VERIFY_DRAW_PLAN=1" SKIP_RETURN_CODE 77 WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}")
if(WIN32)
set_tests_properties(${agcTestName}_plan_verify PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
endif()
endfunction()So during the rebase, keep Sorry the tool's error message doesn't point you to this. |
…and main Merge johandavidcm's feat/agc-draw-prepare-ahead (4c1af4f, with boykopovar#10678) for the Windows A/B measurement. The prepared-ahead fragment outputs, stages and vertex layout and the early resource key feed planDraw. An adopted pipeline takes precedence over main's shape memo. Main's shape parameter is kept ahead of contentKey and ahead in every Draw signature. The pipelined summary keeps main's separate read-range field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011mLh2dBi2gkidDMAs7CY9u
…rison Fold the boykopovar#7895 thread digest, the boykopovar#14051 review state, the Amdahl and shared-state analysis, both merges, the exit crash fix and the lavapipe test comparison against main dc9dc7e into one report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011mLh2dBi2gkidDMAs7CY9u
|
@vingelvatten Main-menu A/B of this PR on 007 First Light, on an NVIDIA discrete GPU under Linux, as a second title next to your Crash Site pairs. Each series was declared before its own runs. I stopped the first by hand after five runs, when a validity rule (the desktop's GPU memory, below) had left too few valid PR runs for three pairs; it decides nothing under the declaration, and its per-run FPS are below for the record. The second was declared after that stop, with the first's per-run figures already recorded. All six of its runs are valid: the PR's mean FPS is 0.9821 of main's (range ends 0.9621 and 1.0001) and its mean frame p50 1.0149 of main's (0.9913 and 1.0437) (details below). The declaration fixed what is reported (each run, the ranges, the ratio of the means and of the range ends), not a faster/slower rule. Setup
Result (second series, one window per run)
{"commit":"73f4f7e06e4f","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1529,"ms":180722.566},"presents":189,"frame_ms":{"p50":952.701,"p90":1055.911,"p99":1164.663,"max":1228.102},"gpu_busy_ms_per_present":144.541,"worker_busy_pct":{"queue worker 0x0":99.949,"queue worker 0x20":99.488,"queue worker 0x28":99.714,"queue worker 0x30":99.713,"queue worker 0x38":99.696},"top_phases_ms_per_frame":{"Driver.Packet":4753.506,"Driver.Packet.memory_wait":3327.255,"Resources.Complete":798.024,"GpuMutex.wait.hook":795.035,"Driver.Packet.dispatch":767.105,"Driver.Dispatch":723.133,"Driver.Packet.draw":610.587,"Graphics.Draw":610.583},"trace_dropped":false,"trace":"aps5-trace-73f4f7e06e4f-2.json.gz"}
{"commit":"aa0880e4ebc0","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1503,"ms":180711.264},"presents":186,"frame_ms":{"p50":962.412,"p90":1097.310,"p99":1214.673,"max":1226.959},"gpu_busy_ms_per_present":147.746,"worker_busy_pct":{"queue worker 0x0":99.930,"queue worker 0x20":99.579,"queue worker 0x28":99.744,"queue worker 0x30":99.744,"queue worker 0x38":99.739},"top_phases_ms_per_frame":{"Driver.Packet":4831.336,"Driver.Packet.memory_wait":3396.620,"Resources.Complete":812.809,"GpuMutex.wait.hook":796.653,"Driver.Packet.dispatch":777.855,"Driver.Dispatch":734.865,"Graphics.Draw":611.660,"Driver.Packet.draw":610.241},"trace_dropped":false,"trace":"aps5-trace-aa0880e4ebc0-2.json.gz"}
{"commit":"73f4f7e06e4f","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1508,"ms":180676.425},"presents":188,"frame_ms":{"p50":962.389,"p90":1075.899,"p99":1222.941,"max":1271.359},"gpu_busy_ms_per_present":147.400,"worker_busy_pct":{"queue worker 0x0":99.951,"queue worker 0x20":99.457,"queue worker 0x28":99.699,"queue worker 0x30":99.697,"queue worker 0x38":99.588},"top_phases_ms_per_frame":{"Driver.Packet":4776.285,"Driver.Packet.memory_wait":3350.958,"Resources.Complete":799.964,"GpuMutex.wait.hook":780.211,"Driver.Packet.dispatch":771.339,"Driver.Dispatch":728.784,"Driver.Packet.draw":605.151,"Driver.Draw":604.898},"trace_dropped":false,"trace":"aps5-trace-73f4f7e06e4f-2.json.gz"}
{"commit":"aa0880e4ebc0","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1514,"ms":180657.061},"presents":188,"frame_ms":{"p50":953.993,"p90":1068.382,"p99":1196.979,"max":1202.609},"gpu_busy_ms_per_present":146.763,"worker_busy_pct":{"queue worker 0x0":99.951,"queue worker 0x20":99.552,"queue worker 0x28":99.736,"queue worker 0x30":99.736,"queue worker 0x38":99.712},"top_phases_ms_per_frame":{"Driver.Packet":4780.893,"Driver.Packet.memory_wait":3340.072,"GpuMutex.wait.hook":833.248,"Resources.Complete":797.166,"Driver.Packet.dispatch":777.838,"Driver.Dispatch":734.256,"Graphics.Draw":616.409,"Driver.Packet.draw":615.043},"trace_dropped":false,"trace":"aps5-trace-aa0880e4ebc0-2.json.gz"}
{"commit":"73f4f7e06e4f","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1526,"ms":180112.308},"presents":189,"frame_ms":{"p50":943.705,"p90":1067.736,"p99":1168.333,"max":1304.805},"gpu_busy_ms_per_present":148.516,"worker_busy_pct":{"queue worker 0x0":99.949,"queue worker 0x20":99.999,"queue worker 0x28":99.997,"queue worker 0x30":99.999,"queue worker 0x38":99.995},"top_phases_ms_per_frame":{"Driver.Packet":4751.606,"Driver.Packet.memory_wait":3339.526,"Resources.Complete":794.029,"GpuMutex.wait.hook":782.633,"Driver.Packet.dispatch":761.799,"Driver.Dispatch":716.878,"Driver.Packet.draw":603.375,"Driver.Draw":603.122},"trace_dropped":false,"trace":"aps5-trace-73f4f7e06e4f-2.json.gz"}
{"commit":"aa0880e4ebc0","dirty":false,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"2097152","APS5_TRACE_SECONDS":"1320:180"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.78.08","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 9 7950X3D 16-Core Processor","cpu_threads":32,"memory_mib":63428,"title_id":"PPSA11386","title":"007 First Light","content_version":"01.018.000","resolution":"3840x2160","window":{"spec":"seconds 1320:180","first_present":1497,"ms":180280.769},"presents":182,"frame_ms":{"p50":984.974,"p90":1104.881,"p99":1262.459,"max":1287.959},"gpu_busy_ms_per_present":151.940,"worker_busy_pct":{"queue worker 0x0":99.953,"queue worker 0x20":99.534,"queue worker 0x28":99.722,"queue worker 0x38":99.713,"queue worker 0x30":99.722},"top_phases_ms_per_frame":{"Driver.Packet":4927.081,"Driver.Packet.memory_wait":3455.790,"GpuMutex.wait.hook":886.080,"Resources.Complete":827.735,"Driver.Packet.dispatch":805.734,"Driver.Dispatch":763.176,"Driver.Packet.draw":617.961,"Driver.Draw":617.724},"trace_dropped":false,"trace":"aps5-trace-aa0880e4ebc0-2.json.gz"}AI-assisted: yes (Claude Code) |
aa0880e to
e2d277a
Compare
|
@MarcZX10 thanks for the exact-head review. Both points, in order. Landing order with #10678. Agreed that the checklist should name it. I have asked @WallisSkill on #10678 whether this PR can land first: it is a refactor with no behaviour change, and once The plan contract. You are right that "no shared cache" was too strong. Rebase. The head is now |
|
@janzezno thank you, that is exactly what the rebase needed: |
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Validated but not compared (12, not in the main baseline's validation pass)Validation IDs already present on main (5 IDs in 484 tests)Skipped (46)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
…e work Step 2 of the draw-side series in boykopovar#7895. planDraw builds a DrawPlan from the state, the draw parameters and the compiled stages alone: the validation and nothing-to-draw exits, the fragment outputs and pipeline stages, the mesh and tessellation checks, the viewport, depth-bounds and sample-mask checks, the vertex input layout and the resource-cache key. prepareDrawInputs starts from the plan and keeps the index range and multisample-target checks, the occlusion-sampling check and every capture. resolveDrawResources takes the key from the plan. The same calls run in the same order relative to every record-time effect; a draw that fails a plan check now throws before its captures. APS5_VERIFY_DRAW_PLAN=1 builds the plan again after the record-time work and requires it to equal the plan the draw used. The draw-pipeline test set gets _plan_verify variants with that switch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
e2d277a to
4aa5f51
Compare
|
Rebased onto |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (5, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 533 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (9, not counted against this PR)Validated but not compared (5, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 533 tests)Skipped (59)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
adriadam10
left a comment
There was a problem hiding this comment.
main moved under this head and it now conflicts in core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp (three hunks, all around resolveDrawResources and its call in prepareDraw). It needs a design decision, not a mechanical rebase:
resolveDrawResources:mainnow computesconst bool candidate = recordable && !noDrawResourceCache && !noTextureCache && all PipelineVariantId() != 0and thenresolved.cacheable = candidate && !ShaderResources::NeverReusable(shaders).address(addressStages,AddressDrawReuse::NotReadOnly/NotSpaceOnly) is derived fromcandidate, so it also applies to stages that areNeverReusable.plan.keyedfoldsNeverReusableinto the same flag, so the address-reuse classification would change for those draws.DrawPlanneeds to carry the candidate condition and theNeverReusablecondition separately (and thenoDrawResourceCache/noTextureCachestatics thatmain's code still reads inresolveDrawResources, which this PR removes).prepareDraw: keepmain'sstopAfterResolveblock andNoteDepthSurfaceWriteafterresolveDrawResourceswhen passingplan.main's new helpers (AddressStages,snapshotsInTable,verifyServedSpace,drawStagingEnabled) sit right aboveresolveDrawResources, which is where the conflict markers land.
After the rebase, please rerun the focused draw tests with the _plan_verify variants on the new head, since the key's inputs changed on main.
The landing order with #10678 is still only agreed in comments; Depends on stays none as you wrote it, so that is fine once it is confirmed there.
What
Step 2 of the draw-side series in #7895 (the outline of 21 October, endorsed by @oneandonlydean): make the read-only part of
prepareDrawexplicit and movable, still on the committer and still in the same order relative to every record-time effect.planDraw→DrawPlanCachedFragmentOutputs,PipelineStages), the mesh group count and limits, the tessellation patch check, the viewport, depth-bounds and sample-mask checks, the vertex input layout, and the resource-cache key (DrawResourceKey). It reads the state, the draw parameters and the compiled stages only: no guest memory, no recorder, no registry, nothing another draw left behind. It does touch three memoizations, each synchronized and order-insensitive:CachedFragmentOutputs(its own mutex, the memo of validated outputs),VertexInputCache::TryGet(its own mutex; a lookup can move an LRU entry, insert, evict or set a bypass bit) and the resource-key memoDrawResourceKeypublishes on the program (atomic, idempotent). A plan built on any thread, in any order, gets the same values from them; what they change is cache state, never the plan's output.prepareDrawInputsDrawInputsand everything downstream (recordDraw,recordDrawCommands, the recipe path) are unchanged.resolveDrawResourcescacheableis still decided here because it needsrecordable, which needs the resident targets.prepareDrawbuilds the plan right after the timer starts (afterBoundKeptBytes, as #8992 arranged) and then runs the same sequence as today.DrawWithRecipebuilds the plan from the recipe's fragment outputs, stages and vertex layout asprepareDrawInputsdid for it before.drawTessellationGeometryis untouched: each of its two draws gets its own plan throughDraw.What changes in behaviour, deliberately:
DrawResourceKeyis built for every draw whose stages allow a cache key, not only for recordable ones (a synchronous draw used to skip it). The key is idempotent on the program and cheap;cacheablestill gates the cache lookup.validate, the layout tovertexand the key build tolookup(DrawTimer::chargemoves the key's time out of the phase it would otherwise land in), so[draws]andAPS5_TRACE_DRAWSkeep their meaning.Debug aid:
APS5_VERIFY_DRAW_PLAN=1(off by default) builds the plan a second time at the end ofprepareDraw, after every record-time effect, and requires it to equal the plan the draw used (the content of the layout, the key and the scalar fields; not the memo hit flags, which a second lookup may legitimately change). It is a consistency check that the plan reads nothing the record-time work changes; it is not a thread-safety proof, and it does not cover the three memoizations' internal state (the LRU position or a bypass bit), which the plan's output does not depend on. The draw-pipeline test set gets_plan_verifyvariants with that switch on top of the pipelined settings.No speed claim: the plan is a small share of the committer (the
validateandpipelinephases together are a few percent at Astro Bot's Crash Site). The point is the seam: step 3 and the preparer thread onfeat/agc-draw-prepare-aheadcan takeplanDrawas the work that leaves the committer, and the adopt-time check sits whereprepareDrawInputsbegins.Tested
Windows 11 Pro 26300, Ryzen 7 7800X3D, 31 GB; WinLibs GCC 15.2.0 r7, Release,
BUILD_TESTING=ON. Based onmaind1ae628 (3 November; rebased twice: on 1 November after #12862 split the test registrations and #13894 changed the mesh group limit, on 3 November after #14857 moved theGraphics.Drawtrace marks intoprepareDrawInputs); head 4aa5f51. Thevalidatetrace mark now sits inprepareDrawInputsafter the three checks that stay there, so the slice still covers all of a draw's validation;index_uploadandvertex_uploadare where #14857 put them. Themainarm of every comparison is a build of that same commit from a second worktree, so the two trees differ only by this change.NVIDIA RTX 4070 Ti SUPER (617.42), focused draw tests (
agc_driver_.*(draw|stencil|indirect|multisample|msaa|target|pass|pipelined|finisher|recipe|plan_verify)|^agc_driver_graphics) withANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:main175 tests, 100% passed, 4 skipped (agc_driver_msaa_dcc_compared_import_{2,4,8},agc_driver_stencil_ref_export); branch 197 tests (the same 175 plus the 22_plan_verifyvariants), 100% passed, the same 4 skipped. Neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS; both hold the same 4 lines ofVUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605fromagc_driver_srgb8_color_target_tests; no new VUID.AMD Radeon Graphics (Ryzen 7 7800X3D iGPU, RDNA2 gfx1036, AMD driver 32.0.21045.16010 / 2.0.353), same set and switches:
main175 tests, 100% passed, 4 skipped (agc_driver_msaa_dcc_compared_import_{2,4,8},agc_driver_mesh_draw_relocation); branch 197 tests, 100% passed, the same 4 skipped. 0 layer-missing lines; both logs hold the same 6 lines of the sRGB8 blend VUID; no new VUID.lavapipe (llvmpipe, LLVM 21.1.8, Mesa 25.x, WSL Ubuntu 26.04 x86-64), same focused set and switches,
ctest -j 4:main176 tests, 100% passed, 6 skipped; branch 198 tests, 100% passed, the same 6 skipped (agc_driver_msaa_dcc_{srgb_clear,clear,compared_import}_2,agc_driver_linear_target_direct_write_completion,agc_driver_mesh_draw_relocation,agc_driver_sample_order_recipe_depth_locations). 0 layer-missing lines; both logs hold the same 6 lines of the sRGB8 blend VUID; no new VUID.ASTRO BOT PPSA21567 01.018.000,
app.exe -lvl hub_crashsite, no input, warm shader cache,APS5_HOST_IMPORT_MIB=13312(without it the title aborts about 110 s in on this machine with a pinned guest allocation, see fix(libc): wait only for imports over the freed span #12157),APS5_PROFILE_DRAW=1 APS5_TRACE_SUMMARY_EVERY=10, guest 3840x2160 in a 2226x1252 window, RTX 4070 Ti SUPER, one title on the GPU. Two run folders that differ only inlibs/:mainab6f5d3 and this head. Three alternated pairs in the order main, branch, branch, main, main, branch, 200 s each, the last eight 10 s windows of each run averaged. The first run (main) is discarded: the newmainbuild has a new shader-cache key, so that run compiled pipelines for its whole 200 s and never left the menus (371 presents per 10 s, 89k commits at 4.5 ms); the five later runs are steady.[draw] pipelinedcommits per 10 smainab6f5d3Three earlier pair sets on the previous base (
main73f4f7e, heads before the rebase) gave 74.4 vs 75.1, 74.0 vs 73.0 and 74.5 vs 74.3 presents per 10 s, so the branch lands on both sides ofmain: run-to-run noise. The[draws]phase totals per 10 s are the same on both arms forvertex(899 vs 903 ms),lookup(1474 vs 1473 ms),pipeline(135 vs 128 ms) andrecord(988 vs 974 ms).validatereads 109 ms onmainand 135 ms on the branch: the difference is the three extrasteady_clock::now()reads per draw thatDrawTimer::chargeand the secondPhaseValidatemark cost underAPS5_PROFILE_DRAW(about 0.1 µs at 245k draws per 10 s); without the switch the plan reads no clock at all.python tools/check_conventions.py --base origin/main: all checks passed.python tools/env_reference.py --check: 0 undocumented (the staleAPS5_PAD_SPEAKER_FALLBACK_DBrow ismain's)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)planDraw), pending its author's confirmation on perf(agc): build a pipelined draw's resource-cache key before it reaches the committer #10678Overlap
#10678 (
perf(agc): build a pipelined draw's resource-cache key before it reaches the committer) adds acontentKeyparameter throughDrawandresolveDrawResources. With this PR the key lives inDrawPlan. Proposed order, asked of its author on #10678: this PR lands first, because it is a refactor with no behaviour change and it gives the early key a home; #10678 then shrinks to passing the finisher's key intoplanDrawand taking it whenKeyForDevicematches (about ten lines instead of its current plumbing). If #10678 lands first instead, this PR rebases:planDrawtakes the given key whenKeyForDevicematches and builds it otherwise. johandavidcm'sfeat/agc-draw-prepare-ahead(not a PR yet) prepares the fragment outputs, stages, vertex layout and set layouts on a preparer thread; with this PR that work isplanDraw.🤖 Generated with Claude Code