Repository navigation
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.
|
|
Tested on Minecraft Dungeons II (PPSA16064, UE). Setup
Draw pipeline. It doesn't speed up this title.
Rendering. Nothing renders wrong in either mode. We took screenshots every 3 s over 90 s of gameplay in each run: no missing textures, no flicker, and no red artefacts. Frames match the base build. First boot with an empty shader cache. With Unrelated failure in the base mode. The base mode ( Heap verify lines are in #6947. Summary lines (second run of each) |
CnRJay
left a comment
There was a problem hiding this comment.
Measurements are there and the numbers are good (469 to 807 presents in 10 s, p50 21.1 to 12.2 ms, p99 also lower). Three things before this can be judged.
- One run each, standing still in a menu, on a 4090 at 1080p. The A/A spread of this setup is not shown (compare #5353, where two identical captures differed by 6.4%), and a menu is the best case for draw-cache reuse. Please add a second pair in gameplay, and repeat the pair once to show the spread.
- This is one switch over seven unrelated changes (finisher thread, commit trims, epoch proofs, ordered metadata passes, shape refresh, straight-line walk, capture memory). The description says the finisher alone made the frame rate worse (105 fps on, 111 off) and that "only the combination" helps. That is a reason to measure each part against its predecessor and land them in order, not to bundle them: nobody can tell which part carries the gain or the risk. At minimum, split off the parts the finisher does not need (metadata passes, straight-line walk, capture memory).
- Known limit:
agc_driver_draw_cache_relocationfails with the switch on. A path that breaks an existing test should not become a default. Make the walk fall back to the relocatable capture when the cache has relocation enabled, or document which titles need relocation.
It is also stacked on #5656 and #6632, so the diff shown here includes their commits. Rebase after they land so only this PR's two commits remain.
|
@CnRJay thanks for the review. "Menu". That was a poor word on my side. The scene is not the main menu: it is the item pickup UI in a level, which is the heaviest scene I found in Hades II. The hub holds a steady 120 fps and normal gameplay runs at 100 to 120 fps, so the pickup screen is where the worker is the limit and where the difference shows. A gameplay trace with both switches (combat in a room, 15 s window, same machine, display at 120 Hz): So in combat it sits at the vblank; there is no headroom left to show a difference there on this machine. I have not yet repeated the pair to show the spread. Splitting. The parts depend on each other in Hades II:
So each part could be measured against its predecessor, but the first ones would land as regressions. That is why it is one switch. I will rebase so only the two commits remain once #5656 and #6632 land. LLM assistance used. |
|
@TideOPL's run is the first outside Hades II: on Minecraft Dungeons II at 4K the draw pipeline doesn't help (queue 0 waits about 53 ms per frame on the compute queue) and nothing renders wrong. @CnRJay's asks stand: a gameplay pair with its spread, each part measured against its predecessor so they can land in order, and |
There was a problem hiding this comment.
Linux, RTX 5080 (driver 615.78.08), Ryzen 7 7800X3D. Base is main 2fab88d with 5656 and 6632 as in the body, then 6946 at f8192a6, all with APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1, warm cache, two 10 s traces each.
| frames in 10 s | base | APS5_DRAW_PIPELINE=1 |
|---|---|---|
| Astro's Playroom (PPSA01325), first level gameplay | 109, 119 | 138, 135 |
| Hades II (PPSA36082), first area of a new game | 640, 646 | 1122, 1054 |
Captures match the base in both, and a burst of consecutive Playroom frames shows no glitch. On a Radeon 680M (RADV) Playroom is GPU-bound and the same: 42 base, 43 and 39 with the switch.
One test fails with the switch. In the full ctest (1364 tests) agc_driver_draw_cache_relocation fails 5 of 5 runs with APS5_DRAW_PIPELINE=1 on 6946 and passes 5 of 5 on the base:
with graphics pipeline libraries: the draws that teach the moved pointer and its contents made 0 hits and 1 inserts, expected 0 and 2
Without the switch the suite fails only where the base fails too: agc_driver_store_over_untracked fails 5 of 5 on the base itself, not from this PR. It passes 5 of 5 on current main 0906c64 with 5656 or 6632 merged alone, so it only shows on the older base.
|
007 First Light (PPSA11386 v01.018.000), 4K output, as asked on Discord. The heap runs are on #6947. Setup:
AI-assisted: yes (Claude Code) |
…its render pass recordDraw passed every address-based draw to the pass hazard tracker as both an unknown reader and an unknown writer of guest memory. An unknown writer conflicts with any earlier access in the pass, so no address-based draw ever continued the previous draw's pass: each one ended the pass and began a new one, which loads and stores the attachments again. At Resident Evil Village's menu about half of all draws are address-based, and about 590 render passes begin for about 850 draws per frame. A build whose programs only read through the BDA table now reports unknown reads only. The tracker already orders unknown reads behind any recorded range write in the pass and keeps later range writes behind them, so such a draw continues a pass whose earlier draws wrote no guest ranges. Builds with stores through the table, copied writes, or an address lease without a table keep both flags. APS5_NO_ADDRESS_READ_PASSES=1 restores the old behaviour. Render targets reach guest memory only by explicit write-backs, so a read through the table sees the same bytes whether or not the pass breaks. Raghav314's boykopovar#6947 makes the same change inside its GPU descriptor heap, with the read-only fact from boykopovar#6946's recompiler field; this one keeps it in the driver, without a shader cache format change. The idea also comes from fable-void's 5d57298 (wip/new-gpu-path), which orders address-based builds by their known access ranges; this takes the read-only half without his run-time pre-pass bounds.
|
A repeat on the current head (d63a804, with the Windows fix), next to #6949 which prepares draws on a thread pool. Hades II (PPSA36082), the level's item pickup UI, Linux, RTX 4090, 7800X3D, NVIDIA 615.78.08. All three builds are
The earlier run on the pre-rebase head gave 807 against a base of 469, so the gain repeats (+72% then, +65% now). In this scene #6949's pool keeps no draw in flight ( LLM assistance used. |
|
Besiege (PPSA20313), Windows, RTX 4070 SUPER, Ancient Grounds sandbox (about 1000 draws per frame), head f8192a6 with APS5_DRAW_PIPELINE=1 on top of current main: 17.0 -> 20.0 FPS (three alternating runs each: 17.3/17.2/16.6 before, 20.3/19.8/20.0 after). A 10 minute run in the level on top of other open perf PRs had no errors and the frame looks the same. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
d63a804 to
d0779ad
Compare
d0779ad to
5e45089
Compare
|
We checked 5e45089: only 805774e changed, and it resolves correctly against the committer Stop from #6641. One low-priority follow-up, affecting |
|
A macOS build fix for this PR, as a ready commit in case it's useful. What fails. Apple clang's libc++ has no
The commit: 8a97b049c4 on
Runs on our fork. These use the full Build workflow on Windows and macOS. The fork has no Linux job; libstdc++ takes the
The commit also cherry-picks cleanly onto #6947's head cae12b7, so #6947 gets it through the stack. Take it as is, or fix it your own way. If either head moves again, we can redo it there. |
5e45089 to
b31be5a
Compare
|
@oneandonlydean thanks, you're right. The finisher is only drained at the end of a queue 0 submission, so a ProcessShutdown in the middle skipped it. LLM assistance used. |
A data-only hit (boykopovar#494) reused a cached stage only when every user data word of the stage was unchanged. A stage records the user words it depends on when it is captured: the words its SRT walk reads as values or bases (SrtUserDataUse), the words its binding layout forwards into shader data (userDataRegisterReads), and the vertex fetch table words of its vertex stage info. A cached stage is now a candidate when only words outside that set changed; its captured memory is validated as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… SGPR follows a change of that word The word reaches the data-only mask through userDataRegisterReads (the stage layout's user-data registers). Dropping those bits from the mask makes the draw reuse the old stage (it writes 127, expected 191) and verify mode abort. LLM assistance used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The second half of a pipelined draw (stage materialization, cache insert, enqueue) runs on a finisher thread, overlapping the next draw's packet work; draws whose stages could write memory are held back. Commits skip work whose inputs did not change since the last commit, color metadata passes are ordered through the pipeline, resource and storage-image proofs are reused within one collect epoch until a write breaks them, and a shape entry is refreshed in place at the second sight of the same read user words. APS5_VERIFY_DRAW_PIPELINE checks reused proofs against a full check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Local On Windows a thread_local destructor in a module runs after emulated TLS has freed the object, and prx_thread_local_destructors refuses libSceAgcDriver.prx for it. The draw pipeline of upstream PR 6946 (Raghav314, f8192a6) added such scratch objects: DrawFinisher::Enqueue's and DrawPipeline::Enqueue's reclaim lists, CachedFragmentOutputs' last key and outputs, DecodeVertexStageInfo's attribute and buffer tables, and PrepareDrawBindings' pool size counts. They now live in HostThreadLocal, as the driver's other per-thread scratch does; on other hosts that is the same thread_local.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…+ builds ResultKeyMemo declared std::atomic<std::shared_ptr<...>>. libc++ has no atomic<shared_ptr> specialization, so with Apple clang the recompiler headers stop at std::atomic's trivially-copyable static assertion (request_serializer_tests in the macOS relinker build). The memo now uses AtomicSharedPtr from libc, which is that specialization where the library provides it and a locked shared_ptr with the same load and store otherwise. The header joins the shader disk cache key's sources, and request_serializer_tests gets core/libs on its include path.
… finisher A WRITE_DATA or RELEASE_MEM label the worker queues for the committer drained the draw finisher first, so that it could not reach the committer before the draws the finisher still held. In Hades II nearly every drain of the finisher was one (81,762 of 82,467 per 10 s were WRITE_DATA). While the finisher holds draws, the label's enqueue now runs as a finisher job, so it reaches the committer after them, and its write range and value are a pending write of the finisher until then. A WAIT_REG_MEM that a pending label satisfies, in the finisher or behind it, stays beside the finisher as it did beside the committer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
599bff4 to
fbe1f8b
Compare
|
ASTRO BOT (PPSA21564), Sky Garden, Linux RTX 3080 Ti: a data point for the head before today's rebase,
AI-assisted; the runs are from my machine(s). |
|
Follow-up on the current head
AI-assisted; the runs are from my machine(s). |
|
@mrfelek On the 0 handed to the finisher: |
…povar#6946 onto 4b462ef (programs table, AppendContentKey order, test name clash with the Windows near macro) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean you were right, thanks:
So the hand-off works in this level, almost every draw goes to the finisher, and the switch adds about 1 present per 10 s on top of the rest of the stack here. The +20% over the base is mostly what's active without it. One run without the switch, so that last step is close to noise. AI-assisted; the runs are from my machine(s). |
…povar#6946 onto 4b462ef (programs table, AppendContentKey order, test name clash with the Windows near macro) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
An opt-in draw path,
APS5_DRAW_PIPELINE=1, that takes per-draw work off the queue worker. Stacked on #5656; only the last six commits are this PR.ProgramWritesMemory, from the decoded program) is finished in place.main's second-sight admission is replaced in place when the same read user words are seen twice.WRITE_DATAorRELEASE_MEMlabel the worker queues while the finisher holds draws goes through the finisher's queue instead of draining it, so it reaches the committer after those draws; until then its range and value are a pending write of the finisher, and aWAIT_REG_MEMthat the pending value satisfies stays beside the finisher, as it did beside the committer.Always on, with the same results: the draw resource key is appended in place with a per-result memo and hashed four lanes at a time, a vertex stage's attribute and buffer table words are read in one read, and the pending storage-image check skips ranges the pending set cannot overlap.
The switch is meant with
APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1(both onmain).APS5_VERIFY_DRAW_PIPELINE=1runs the full checks next to the reused proofs and counts differences.Measuring:
APS5_PROFILE_DRAW=1finishes every draw on the worker, so it does not measure this switch.APS5_PIPELINE_STATS(any value;1also turns on Vulkan pipeline statistics) prints[finisher]lines with the draws handed off and kept, and[finisher-held]lines with why draws were held back.PPSA36082
On current
mainthe switch takes work off the worker, and with the labels behind the finisher it rarely drains, but the frame time does not change. Level UI standing still,APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1 APS5_PIPELINE_STATS=2, default import budget, noAPS5_PROFILE_*switch,APS5_TRACE_EVENTS=1048576so the traces are whole, this PR with #8120 onmaina700809 (the run without the switch is from the build before theWAIT_REG_MEMpart of the label commit, which only acts with the switch on):without the switch:
with
APS5_DRAW_PIPELINE=1:plain
main0d8c51a, same spot and environment, without perf(agc): data-only draw hits compare only the user words a stage reads #5656, fix(agc): write-watch the AGC driver memory on Linux #8120 or this PR:Presents in 10 s: 452 on plain
main, 691 with this PR without the switch, 697 with it (single runs here vary by about 2-3%). Shader compile and link per frame is 13.2 ms on plainmainand 6.8 ms on this stack without the switch. I have not split how much of that comes from perf(agc): data-only draw hits compare only the user words a stage reads #5656 and how much from this PR's commits outside the switch, and the plainmainrun is on a newermainthan the other two. Neither run refused the import fix(agc): write-watch the AGC driver memory on Linux #8120 is about.Queue worker busy: 91% without, 89% with. The two switch-on runs before the
WAIT_REG_MEMpart of the label commit had the worker 82.4% and 83.1% busy (708 presents each). The rise to 89% is in draw preparation (Driver.Draw.prepare5.5 to 6.3 ms per frame), not in the packets around the waits (1.71 to 1.83 ms per frame, with the 81,054WAIT_REG_MEMdrains gone). The finisher's draw time (8.2 to 8.9 ms) and the GPU (2.5 to 2.8 ms per present) rose by about as much, and neither runs that code, so I read it as run-to-run variance, from one run each.Drains of the finisher per 10 s: 82,467 before the label commit (81,762 of them
WRITE_DATA), 9,394 after it (allDISPATCH_DIRECTbeside commits). It hands off 775,606 draws per 10 s and finishes none on the worker.The committer is about 75% busy either way (commits times the time per commit on the
[draw] pipelinedline) and the GPU about 2.8 ms per present, so no thread is saturated in this scene. I have not found what sets the frame time here.The earlier pair on this PR (469 to 807 presents) was on a
mainwhere the worker alone limited the frame. This stack now gets most of that without the switch (691 presents above, against 452 on plainmain).Tested
Linux (CachyOS), x86-64, GCC, 7800X3D / RTX 4090, NVIDIA 615.78.08, on
main0d8c51a with #5656.build.ymlrun locally: configure with SPIRV tools on, test registration check, build, libs, the whole ctest and conventions. 1693/1693 pass, 16 expected skips, conventions OK.agc_driver_*tests again withAPS5_DRAW_PIPELINE=1 APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1: 623/623 pass.agc_driver_same_queue_label_waitandagc_driver_unimported_label_order) that check results after CPU writes, BDA reads, pipelined DMA and fills, draw cache reuse and a rejected draw also run with the switch (*_draw_pipeline).agc_driver_pipelined_metadata_pass(fast-clear eliminates sent through the pipeline stay ordered with the queued commits and stores, write the expected texels and leave the CMASK expanded),agc_driver_program_writes_memory(buffer and image stores, GDS writes and indirect jumps count as writes; loads and export-only programs do not; the answer is cached per program),agc_driver_draw_data_hit_user_words_refresh(the shape entry is replaced, not added, at the second sight of the same read words),agc_driver_draw_finisher_trace(Scramblrrrr's: the finisher's CPU work is in the trace),agc_driver_finisher_label_order(eight draws each followed by aWRITE_DATAlabel and aWAIT_REG_MEMon it: the labels hold their values, the draws write the expected color, and with the switch neither packet drains the finisher; without the label commit it fails with0x37 x8, without its wait part with0x3c x8).Since the last review
main0d8c51a with perf(agc): data-only draw hits compare only the user words a stage reads #5656.DrawFinisher::Drain,Settleand the full-queue wait inEnqueuethrow when they run under the GPU lock, asDrawPipelinealready did.draw-error ... finisher=1), instead of being reported as the packet that next drained the finisher.agc_driver_draw_unsupported_buffer_type_draw_pipelinechecks it.[finisher-held]lines also print underAPS5_PIPELINE_STATS, and USAGE.md says thatAPS5_PROFILE_DRAWturns the hand-off off._refreshuser-word test gets libc's directory onPATHon Windows.Known limits
Why this is one PR
Several parts on their own made the frame rate worse in Hades II; only the combination gave the gain (measured on the earlier
mainof the first pair above).main's second-sight admission leaves a shape entry at an old draw, so with the faster worker data-only reuse of vertex stages fell from about 80% to 17%; with the shape refresh it is back to 83% (hub 108 to 117 fps).Testing other titles
Please post:
APS5_TRACE_SECONDS=70:10 APS5_TRACE_EVENTS=1048576) with and withoutAPS5_DRAW_PIPELINE=1, both withAPS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1and nothing else changed (notAPS5_PROFILE_DRAW, see above);APS5_VERIFY_DRAW_PIPELINE=1, the last[epoch-proofs]line in the log (any nonzero "differed" or "failed" count is a bug).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)