Repository navigation
fix(agc): keep at most 4 GiB of the host heap from the host import budget - #7844
oneandonlydean merged 3 commits into
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.
|
|
Windows and AMD's own driver. AMD Radeon 860M (RDNA 3.5,
AI-assisted: yes (Claude Code) |
fogydes
left a comment
There was a problem hiding this comment.
Reviewed the change:
- Capping the host heap reserve at 4 GiB (
std::min<std::uint64_t>(hostHeap / 2, 4096ull << 20u)) prevents 32 GiB hosts with 16 GiB host heaps from clamping the import budget below the 10,466 MiB working set required by memory-intensive titles (PPSA24191), while preserving the 2 GiB minimum floor for smaller 16 GiB configurations. - The unit test cases in
Graphics.cppexplicitly guard both 16 GiB, 32 GiB, and 15,911 MiB heap scenarios against regression. - Technical debt in
docs/dev/debt/libSceAgcDriver.mdupdated accurately. - Confirmed independent hardware test results posted by
@LeandroLP23(AMD Radeon 860M) verifyingagc_driver_graphicspasses 3/3 on this head. - Zero comments added in code; Conventional Commits title; all CI checks pass.
Ready to merge.
AI-assisted: yes
|
AMD and Intel Windows data: with this PR's test changes, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
AI-assisted: yes (Claude Code) |
096de7f to
ac03b41
Compare
|
I ran into the same thing with Ghost of Yōtei (PPSA26344) on a 32 GiB Windows laptop with an NVIDIA GPU, so here's a second data point from another title and vendor. With #6472's rule the budget there comes out at 8,117 MiB. In a warm run from a save into the open world, imports started being refused as soon as the world began loading, 171,828 of them by the end of the run, and the environment only appeared in the frame dump at 492 s. The same build with the old 2 GiB reserve put back (about 14 GiB here) refused no import in a 1,408 s run, and the same frame showed the environment at 412 s. I haven't run Yōtei with this PR's 4 GiB cap yet; it would give about 12 GiB on this machine. Our older notes put Yōtei's import working set at around 13 GiB, so it may still refuse some here, though far fewer than at 8 GiB. If it's useful, I can run your branch and post the refusal count. One caveat: these runs are from our local tree (a Details
|
|
Performance data from another title: on a 32 GiB Linux machine, #6472's budget makes ASTRO BOT about 2.5× slower, and this PR brings it back. Setup: ASTRO BOT PPSA21567 01.019.000,
What happens on How I found it: Crash Site went from p50 124 ms on AI-assisted: yes (Claude Code) |
ac03b41 to
2af7b9b
Compare
|
Follow-up on the first level, Sky Garden (
Tested and written by Claude (AI assistant) on a contributor's machine. |
|
Windows data point for
|
|
A data point from Linux with PRAGMATA (PPSA02530 01.200.000), where the default budget fills on a 32 GB machine. Setup: Arch Linux, kernel 7.2.11, Ryzen 7 9800X3D, 32 GB RAM, Radeon RX 9070 XT 16 GB, Mesa 26.2.5 RADV. Host heap 0 is 15596 MiB, so the default budget is 7798 MiB on the base and 11500 MiB with this PR. Release build. The base is Route: new game from the title menu (Standard difficulty), then the opening cinematic, Cross every 30 s, 1200 s per run,
|
|
@Mouhand-Kaddo #11357 just landed on main and now conflicts with this PR in one place: the "Host import windows and budget" bullet in docs/dev/debt/libSceAgcDriver.md. Could you merge main and keep both edits there (your budget sentence plus the eviction sentences from #11357)? The code changes merge cleanly and are independent of each other. After the merge we'll run fresh CI on the new head. The open point about agc_host_import_sparse being skipped on CI is unchanged. |
…se/pr7844 # Conflicts: # docs/dev/debt/libSceAgcDriver.md
|
@oneandonlydean Merged main in 89d6a7c. The "Host import windows and budget" bullet now has both edits: this PR's budget sentence (half the heap, kept between 2 and 4 GiB) and the frame-based eviction text from #11357. Against main, this PR still changes only the budget sentence in the docs, |
|
Correction (2026-10-30): the Persona 5 Royal pair below is not valid evidence (a harness fault drove both legs with an extra input driver); see the correction. The @oneandonlydean Two notes on this head (89d6a7c).
Persona 5 Royal (PPSA05110, own dump) with this head. Follow-up to our 10-25 comment, same machine, now on plain
Setup: Windows 11 26200, Ryzen 5 5600X, 32 GiB, RX 6700 XT (AMD driver 26.10.1), WinLibs GCC 15.2.0 r7, Release. AI-assisted: yes (Claude Code) |
✅ NVIDIA GPU test: passedPR head
Failing on main too (5, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (4 IDs in 405 tests)Skipped (39)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. |
|
Correction to my comment above: the Persona 5 Royal pair in it is not valid evidence. A fault in our run harness left the input driver of an earlier, aborted run going, and it pressed buttons into both legs: into the AI-assisted: yes (Claude Code) |
|
@oneandonlydean Here is the fresh Persona 5 Royal (PPSA05110) evidence that replaces the withdrawn pair. Three exclusive legs alternate
Neighbouring pairs (this PR minus
The input driver dumps one frame every 20 presents, so 3 to 4 dumps fell inside each 4 s window, on both builds. The set holds only one leg of this PR; I'll add the next pair once it has run. Traces, summary lines and AI-assisted: yes (Claude Code) |
|
@Mouhand-Kaddo heads-up: #12074 just merged and now conflicts with this PR in |
|
@Mouhand-Kaddo in case it saves you time with @oneandonlydean's merge request above: only the debt entry conflicts. Machine for everything below: RX 6700 XT (AMD driver 32.0.21045.16010), Ryzen 5 5600X, 32 GB, Windows 11 26200. Tests on Persona 5 Royal (PPSA05110) on a base with #12074. This is the second pair promised in our 10-29 comment, with the same route and trace form (New Game, opening movie skipped, standing in the first gameplay area;
Not tested: the title on AI-assisted: yes (Claude Code) |
|
Follow-up to my comment above: the Persona 5 Royal (PPSA05110) pair on the merged head itself, which that comment listed as not tested. Four exclusive legs alternate
Not tested: Linux or lavapipe; other GPUs and titles. AI-assisted: yes (Claude Code) |
# Conflicts: # docs/dev/debt/libSceAgcDriver.md
|
Note 🤖 Claude Opus 5.5 responding on behalf of Mouhand-Kaddo @oneandonlydean Merged |
✅ 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 (3, not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)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. |
What
Demon's Souls (PPSA01342) stops on
mainwhile loading into the tutorial level, right after the intro cinematic, on a 32 GiB Linux machine (RX 6900 XT, RADV):#6472 keeps the larger of 2 GiB and half the largest host heap out of the import budget. That fixed the 16 GiB machine it was measured on (heap 8128 MiB, about 4 GiB kept back). The same half also applies to larger heaps, so a 32 GiB machine (RADV heap 15911 MiB) went from 13863 MiB to 7955 MiB. With
APS5_TRACE_HOST_IMPORT, this title holds up to 10466 MiB of live imports at once while it loads the level, so the address-based build's imports are refused and the copy limit aborts.The reserve is now half the heap kept between 2 and 4 GiB:
mainHeaps up to 8 GiB keep
main's budget.APS5_HOST_IMPORT_MIBstill overrides it, and #6479 (lowering the budget aftervkAllocateMemoryfailures) composes with this: it caps the effective budget, this sets the default it starts from. #7808 rewrites theAPS5_HOST_IMPORT_MIBrow ofENVIRONMENT.md; I left that row alone to avoid the conflict. The debt entry is updated.Risk: two-way door, one constant. A machine with a host heap above 8 GiB pins more host memory for imports before the least-recently-used ones are retired. On the run below, total GTT use peaked at 12.6 of 15.9 GiB.
Tested
Linux 7.2.11 (EndeavourOS), i7-13700K, 32 GiB, RX 6900 XT with Mesa 26.2.5 RADV, GCC 16.2.1, Release,
BUILD_TESTING=ON,main36f60df; rebased ontomain98db779 on 2026-10-20 (only the debt entry conflicted:mainhad extended the same paragraph, so its text is kept with this PR's reserve sentence applied). Rebased again ontomain3a2c51a on 2026-10-25: the debt entry conflicted again, becausemaindropped its sentence on address-based builds evicting (now its own "Piece-split ranges" entry), somain's text is kept with this PR's reserve sentence applied. On Windows 11 (WinLibs GCC 15.2.0, RX 6900 XT, driver 32.0.21045.16010),ctest -R agc -j 8on the rebased branch passes 1037 of 1040;agc_host_import_cut_lifetime,agc_driver_depth_plane_partial_storeandagc_driver_sample_order_grid_referencefail the same way onmain3a2c51a there.agc_driver_graphics(hostImportBudgetTests): with only the test change onmainit fails witha 32 GiB machine does not keep 4 GiB of its 16 GiB host heap from imports; with the change it passes. The 8128 MiB and 3 GiB cases are unchanged and still pass. Rerun on 98db779:agc_driver_graphics_testswithmain'sGuestBufferMemory.cppexits 1 with the same message, and with this PR it printsGraphics validation tests passed.agc_host_import_unmap,agc_host_import_retirement,agc_host_import_retire,agc_host_import_sparse,agc_driver_bda_anchor_reads,agc_driver_bda_span_readspass.agc_driver_bda_devicefails the same way onmainand on this branch on this GPU (an anchored BDA read differs from the same read through the lookups at anchor ...); it does not use the budget.python3 tools/check_conventions.py --base upstream/mainpasses.--to-intel, continuing a saved character (main menu → Continue → Continue Offline), 1080p (+r_Wants4K = WantsPerfin a copy of therenderconfig_ps5_*.txtfiles), no environment overrides:maine12c913 (sameHostImportBudget): the stop above while loading the level, exit 134 (one run).mainwithAPS5_HOST_IMPORT_MIB=13862(the old default): reaches the level in both runs, peak 10466 MiB live imports, no refusal.mainrun withAPS5_HOST_IMPORT_MIB=11815(this PR's value) crashed in title code while loading: a guest heap pointer had been overwritten with RGBA16F texel data. No import was refused in it, so it is a separate bug, which I'm investigating.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)