Repository navigation
Conversation
github-actions
Bot
added
perf
|
This pull request is labeled |
|
Summary files (
GitHub does not accept |
github-actions
Bot
removed
the
perf numbers missing
|
Traces of the two windows whose summary lines are in the description: main + #8091 (0c6a5bd) and this PR + #8091 (ceefc5b), |
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
This is a substantial performance investigation. The report includes same-machine SuperTuxKart comparisons, traces, an equivalence check across millions of skipped validations, a large focused test set, and paired Vulkan validation results with matching VUIDs. git diff --check passes; GitHub reports the head mergeable, and eligibility, conventions, intake, labels, and report checks pass.
I’m requesting changes for the remaining contribution-rule items:
- The benchmark and test descriptions use bases
0c6a5bd593andeaff87674c; currentmainis4a17c71757. Please rebase onto currentmain, rerun the focused AGC tests on the same machine and GPU with the required validation-layer settings on both current main and the change, compare VUIDs, and refresh the performance summaries/traces against that base. - Please coordinate the shared refresh-loop behavior with #8926 before merge. Your overlap analysis notes that #8926 re-inserts imports in this loop and that the incremental path must add those imports to
pendingImportsif both changes land. Agree on order with its author and reflect any prerequisite inDepends on:. Also refresh the overlap check and coordinate any actual same-line overlaps found in the import insertion, test registration, and debt entry with their authors; the current body records adjacent conflicts with #7261, #6820/#8104, and #7844, while the current overlap scan did not complete in my shared checkout due concurrent temporary-ref updates.
No local AGC test or GPU run was possible in this review environment. The reported older-base results are useful evidence, but the repository requires current-main validation for this change before it is ready.
refreshImports checked every live host import, failure and refusal again at each guest allocation registry change. A title that maps memory often (SuperTuxKart, ~1650 maps a second) paid a walk that grew with the live imports: ~34 FPS with ~1.1 GiB of imports on RADV. The import table now keeps a weak_ptr snapshot of the ranges it was last checked against and diffs it with the new lease. Only imports whose import window touches a changed span, and imports made since the last refresh, are checked again. Every import is checked when the window is 0 or the window or budget changed. SuperTuxKart's menu goes from ~34 to ~45 FPS with the same imports live.
ceefc5b to
3b7c43e
Compare
|
Summary files of the six SuperTuxKart runs and the traces of the two windows in the description, on |
|
@MarcZX10 rebased onto On the overlaps: I moved the new lines away from the neighbouring edits in |
This branch has not been deployed
2 participants
What
refreshImportsre-checked every live host import, and every remembered failure and refusal, each time the guest allocation registry changed. A title that maps memory often pays that walk constantly. SuperTuxKart (PPSA69421) maps about 1650 times a second at its menu. With #8091's budget fix, about 440 imports (~1.1 GiB) stay live there, and the walk alone costs ~118 ms of CPU per second on the frame's critical path. The frame rate falls as the import budget grows: ~34 FPS with the default budget, ~48 FPS withAPS5_HOST_IMPORT_MIB=128.This PR makes the refresh incremental:
(address, bytes, weak_ptr)of every range in the lease it was last checked against. The snapshot holdsweak_ptrs because a lease pins its ranges: unmap waits whileuse_count() != 1.owner_before, assameRundoes). The result is the address spans whose ranges were added, removed or replaced, whatever mutation did it, including backing renewals.[base / window * window, max(that + window, base + bytes))) touches a changed span are checked again. These are found with alower_boundon the import map plus one predecessor; imports never overlap, aspeekImportalready relies on.importRun,exactRunandjoinedRunread only ranges inside that span. The scan carries a cursor across the changed spans, so each import is checked at most once per refresh even when several separate spans fall in one window.pendingImports); retiring an import removes its base, so the set never holds more than the live imports.sameRun, so this is cleanup only.One behaviour differs, and it is recorded in
docs/dev/debt/libSceAgcDriver.md. An unchanged import whose run could now be larger, because a remembered refusal no longer holds, is kept until its window changes. A request that needs the larger run still retires it (wouldRetireImport). Onmain, any registry change anywhere retired it. Under a 128 MiB budget in SuperTuxKart,mainretired and re-made the same three still-valid single-range imports about 1650 times a second for this reason.New test
agc_windowed_import_refresh(128 KiB window, three windows):It passes on
mainand on this PR. It fails when the window query is narrowed tolower_bound(begin): "an import whose window gained a continuing range was kept".Two local probes (not in this PR) checked the review follow-ups:
Tested
Arch Linux (kernel 7.2.11), Ryzen 5 7500F, AMD Radeon RX 7900 GRE, RADV Mesa 26.2.5, on
mainb7d1d47a5f. SuperTuxKart (PPSA69421 00.001.000, catalog sha256f2240bf0…b712a), relinked with--trap-sdk-syscall-stubs. Main menu with no input, headlessgamescope --backend headless -W 1920 -H 1080,APS5_TRACE_SUMMARY_EVERY=10, 70 s runs. Each side's first run creates the game's configuration and starts in a different first-launch scene, so it only warms the shader caches and is not counted. Then three runs per side in the order main, PR, main, PR, main, PR. The FPS is the presents in the last 8 s of each run (VideoOut telemetry).Why #8091 is applied on both sides. On RADV, plain
mainlowers the import budget after memfd-backed imports are refused (#8091). The menu then randomly settles in a GPU-bound ~3 FPS mode, with GPU ~90% busy and a CPU nearly idle, or a fast mode with ~74 MiB of imports. Both sides do this: on plainmaineaff876, main ran 2.9 / 2.7 / 3.3 FPS and this PR 1.0 / 3.2 / 53.7 FPS. That noise hides the walk. Both builds below therefore carry #8091's one-line production change inimportRunAt. This PR does not depend on #8091.These six runs were built from a work tree with this change applied on
b7d1d47a5f(identical to the pushed commit), so their summary lines showb7d1d47a5f9fdirty on both sides.tools/trace_digest.pyof the first run of each side (summary files of all six runs attached):main + #8091
6 intervals, 0.0 to 60.1 s, 2007 presents
AMD Radeon RX 7900 GRE (RADV NAVI31), radv Mesa 26.2.5-arch1.1, Linux 7.2.11-arch1-1, 1920x1080, commit b7d1d47 (dirty)
this PR + #8091
6 intervals, 0.0 to 60.0 s, 2998 presents
AMD Radeon RX 7900 GRE (RADV NAVI31), radv Mesa 26.2.5-arch1.1, Linux 7.2.11-arch1-1, 1920x1080, commit b7d1d47 (dirty)
Trace windows for the perf check.
APS5_TRACE_SECONDS=40:10, same scene and machine.mainisb7d1d47a5fand this PR is3b7c43eb75, each with #8091's one-line change applied, so both show as dirty. Two windows per side: main 35.9 / 35.6 FPS, p50 27.5 / 27.7 ms, p99 32.8 / 40.9 ms; this PR 45.6 / 51.8 FPS, p50 20.7 / 18.8 ms, p99 36.7 / 29.0 ms. The first PR window ends in a short run of slow frames (presents 2093–2099, the queue worker idle-waiting, as in the slow frames onmain). The first window of each side, with its trace attached:main + #8091:
{"commit":"b7d1d47a5f9f","dirty":true,"env":{"APS5_NO_PAD_INPUT":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"40:10","APS5_TRACE_SUMMARY_EVERY":"10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"AMD Radeon RX 7900 GRE (RADV NAVI31)","driver":"radv Mesa 26.2.5-arch1.1","os":"Linux 7.2.11-arch1-1","cpu":"AMD Ryzen 5 7500F 6-Core Processor","cpu_threads":12,"memory_mib":31744,"title_id":"PPSA69421","title":"SuperTuxKart","content_version":"00.001.000","resolution":"1920x1080","window":{"spec":"seconds 40:10","first_present":1302,"ms":10015.333},"presents":360,"frame_ms":{"p50":27.476,"p90":31.046,"p99":32.762,"max":33.129},"gpu_busy_ms_per_present":1.247,"worker_busy_pct":{"queue worker 0x0":3.687},"top_phases_ms_per_frame":{"Graphics.Draw":2.664,"Vulkan.DrawFromRecipe":2.280,"Graphics.DrawWithRecipe":2.270,"Graphics.DrawWithRecipe.draw_and_resource_release":2.265,"Graphics.Draw.draw_and_resource_release":2.006,"Recorder.Submit":1.802,"Driver.Packet":1.010,"Driver.Packet.draw":0.737},"trace_dropped":false,"trace":"aps5-trace-b7d1d47a5f9f-164176.json.gz"}this PR + #8091:
{"commit":"3b7c43eb752e","dirty":true,"env":{"APS5_NO_PAD_INPUT":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"40:10","APS5_TRACE_SUMMARY_EVERY":"10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"AMD Radeon RX 7900 GRE (RADV NAVI31)","driver":"radv Mesa 26.2.5-arch1.1","os":"Linux 7.2.11-arch1-1","cpu":"AMD Ryzen 5 7500F 6-Core Processor","cpu_threads":12,"memory_mib":31744,"title_id":"PPSA69421","title":"SuperTuxKart","content_version":"00.001.000","resolution":"1920x1080","window":{"spec":"seconds 40:10","first_present":1646,"ms":10012.706},"presents":457,"frame_ms":{"p50":20.681,"p90":27.595,"p99":36.689,"max":38.388},"gpu_busy_ms_per_present":1.242,"worker_busy_pct":{"queue worker 0x0":4.973},"top_phases_ms_per_frame":{"Recorder.Submit":1.919,"Graphics.Draw":1.550,"Graphics.Draw.draw_and_resource_release":1.339,"Vulkan.DrawFromRecipe":1.155,"Graphics.DrawWithRecipe":1.145,"Graphics.DrawWithRecipe.draw_and_resource_release":1.140,"Driver.Packet":1.074,"Driver.Packet.draw":0.780},"trace_dropped":false,"trace":"aps5-trace-3b7c43eb752e-199732.json.gz"}tools/trace_digest.py, main + #8091seconds 40:10 from present 1302: 360 presents, frame p50 27.5 / p90 31.0 / p99 32.8 / max 33.1 ms, GPU busy 1.2 ms per present
AMD Radeon RX 7900 GRE (RADV NAVI31), radv Mesa 26.2.5-arch1.1, Linux 7.2.11-arch1-1, 1920x1080, commit b7d1d47 (dirty), dropped: no
{"commit":"b7d1d47a5f9f","dirty":true,"env":{"APS5_NO_PAD_INPUT":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"40:10","APS5_TRACE_SUMMARY_EVERY":"10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"AMD Radeon RX 7900 GRE (RADV NAVI31)","driver":"radv Mesa 26.2.5-arch1.1","os":"Linux 7.2.11-arch1-1","cpu":"AMD Ryzen 5 7500F 6-Core Processor","cpu_threads":12,"memory_mib":31744,"title_id":"PPSA69421","title":"SuperTuxKart","content_version":"00.001.000","resolution":"1920x1080","window":{"spec":"seconds 40:10","first_present":1302,"ms":10015.333},"presents":360,"frame_ms":{"p50":27.476,"p90":31.046,"p99":32.762,"max":33.129},"gpu_busy_ms_per_present":1.247,"worker_busy_pct":{"queue worker 0x0":3.687},"top_phases_ms_per_frame":{"Graphics.Draw":2.664,"Vulkan.DrawFromRecipe":2.28,"Graphics.DrawWithRecipe":2.27,"Graphics.DrawWithRecipe.draw_and_resource_release":2.265,"Graphics.Draw.draw_and_resource_release":2.006,"Recorder.Submit":1.802,"Driver.Packet":1.01,"Driver.Packet.draw":0.737},"trace_dropped":false,"trace":"aps5-trace-b7d1d47a5f9f-164176.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.WorkerWait 26.7, Driver.WorkerWait.idle_and_completion_poll 26.6, Graphics.Draw 2.7, Vulkan.DrawFromRecipe 2.3, Graphics.DrawWithRecipe 2.3, Graphics.DrawWithRecipe.draw_and_resource_release 2.3, Graphics.Draw.draw_and_resource_release 2.0, Recorder.Submit 1.8, batch 1.2, Graphics.Draw.validate 0.6
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
tools/trace_digest.py, this PR + #8091seconds 40:10 from present 1646: 457 presents, frame p50 20.7 / p90 27.6 / p99 36.7 / max 38.4 ms, GPU busy 1.2 ms per present
AMD Radeon RX 7900 GRE (RADV NAVI31), radv Mesa 26.2.5-arch1.1, Linux 7.2.11-arch1-1, 1920x1080, commit 3b7c43e (dirty), dropped: no
{"commit":"3b7c43eb752e","dirty":true,"env":{"APS5_NO_PAD_INPUT":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"40:10","APS5_TRACE_SUMMARY_EVERY":"10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"AMD Radeon RX 7900 GRE (RADV NAVI31)","driver":"radv Mesa 26.2.5-arch1.1","os":"Linux 7.2.11-arch1-1","cpu":"AMD Ryzen 5 7500F 6-Core Processor","cpu_threads":12,"memory_mib":31744,"title_id":"PPSA69421","title":"SuperTuxKart","content_version":"00.001.000","resolution":"1920x1080","window":{"spec":"seconds 40:10","first_present":1646,"ms":10012.706},"presents":457,"frame_ms":{"p50":20.681,"p90":27.595,"p99":36.689,"max":38.388},"gpu_busy_ms_per_present":1.242,"worker_busy_pct":{"queue worker 0x0":4.973},"top_phases_ms_per_frame":{"Recorder.Submit":1.919,"Graphics.Draw":1.55,"Graphics.Draw.draw_and_resource_release":1.339,"Vulkan.DrawFromRecipe":1.155,"Graphics.DrawWithRecipe":1.145,"Graphics.DrawWithRecipe.draw_and_resource_release":1.14,"Driver.Packet":1.074,"Driver.Packet.draw":0.78},"trace_dropped":false,"trace":"aps5-trace-3b7c43eb752e-199732.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.WorkerWait 20.7, Driver.WorkerWait.idle_and_completion_poll 20.7, Recorder.Submit 1.9, Graphics.Draw 1.5, Graphics.Draw.draw_and_resource_release 1.3, batch 1.2, Vulkan.DrawFromRecipe 1.2, Graphics.DrawWithRecipe 1.1, Graphics.DrawWithRecipe.draw_and_resource_release 1.1, VideoOut.Flip 0.4
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
tools/performance_compare.pyPerformance comparison
Configuration differences
Frame and GPU times (ms)
Top phases (ms per frame)
Other measurements (same machine and scene, menu,
maineaff876 + #8091):APS5_PROFILE_GPU=1shows the same recorded GPU time per present on both sides (2.4 ms in 59 batches), andAPS5_NO_DRAW_INPUTS_IN_PLACE=1changes nothing. So this is not perf: Red Dead Redemption (PPSA30528) G-buffer draws spend ~100x their GPU time reading vertex/index data in place from host imports #5895's in-place read cost.importRun4.3%,_Rb_tree_increment2.3%,leasedIndexContaining1.8% andsameRange1.6% of user samples. With this PR,importRun,leasedIndexContainingandsameRangestay under 0.3%.refreshImportsitself is 1.7%: the lease diff is linear in the ~800 registered ranges, and the remembered failures and refusals are still iterated.sameRangefor every import the incremental refresh skipped. With the default budget it found 0 differences in ~3.4M skipped checks. None of the skippedunwatchedimports were back in the write watch, and no imports overlapped. With a 128 MiB budget, the only differences were the refusal-dependent run shapes described above.AGC, shader recompiler:
ctest --test-dir build -R "agc|shader|recompiler" -V -j6withANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, onmainb7d1d47a5fand on this change, same machine. Neither log containswas not found but was requested by env var VK_INSTANCE_LAYERS.main: 873 of 876 passed. This PR: 874 of 877, with the new test.agc_idle_publish_wait,agc_driver_image_array_window_past_layer_limit(test(agc): match array sample expectations to the view #8361) andagc_driver_tessellation(segfault, Mesa 26.2, agc_driver_tessellation segfaults on RADV with Mesa 26.2+: nir_opt_varyings corrupts the TCS input offset's use list (Mesa bug) #5444).VkImageMemoryBarrier-image-03320×12,VkImageViewCreateInfo-usage-02275×6,VkPipelineColorBlendStateCreateInfo-pAttachments-00605×26 andVkWriteDescriptorSet-descriptorType-00337×32.Overlap with open pull requests, from
tools/pr_overlap.pyonb7d1d47a5f:checkis the only place either walk retires an import, so its two additions go intocheckand after both walks. The order is agreed in its thread: noDepends onin either, and the one that merges second rebases.importRun's run bounds, which lowers the cost of each check. This PR lowers how many checks run.refreshImports,retireImportorimportRunAt:GuestMemory::Unwatchline in the import loop, which this PR moves intocheck, and notes every import it re-unwatches. With this PR only re-checked imports are re-unwatched. Write watching only comes back on a range through a registry change (a new mapping, or a remap, whoseProtectreplaces the ranges) or through a settle that marks its import watched, so a skipped import is never back in the write watch, as the check mode above also found.importRunAtmakes an import, not the refresh.mainwith an empty description.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)