Repository navigation
Conversation
github-actions
Bot
added
unblocks title
|
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.
|
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Static review plus a small arm64 repro on macOS (Apple M2 Max, macOS 27.2, Apple clang 21), head e6a2bcbbf. I did not build the AGC driver: main does not build it on macOS. The macOS series in #7332 does build it, and there GuestMemory.cpp reads regions with mach_vm_region instead of /proc/self/maps.
The capability check now runs after a /proc read, so the refusal message changes on hosts without procfs.
GuestBufferMemory.cpp:2406callsGuestNonzeroBlocksbeforemake_shared<ResidentBuffer>at line 2410, which is whereResidentMirror.cpp:342-343rejects a device withoutsparseBindingorsparseResidencyAliased.GuestNonzeroBlocks→possiblyNonzero→mappings()opens/proc/self/mapsunder#ifndef _WIN32(ResidentMirror.cpp:217-218).populatedPagesdoes the same with/proc/self/pagemap(line 256).- On macOS, the first heap mirror therefore throws
AGC graphics: cannot read /proc/self/maps for a heap mirror's pages. That error is raised outside thetrythat wraps it inheapMirrorError, so it carries neitherheap mirror of 0xnorsparse binding. heapMirrorsRefused(BdaResources.cpp:1218-1222) requires both strings, so on MoltenVK, which the PR body lists as lacking the sparse features, itsRequirefails instead of reporting "refused ... mirror behaviour not tested".
Repro: I extracted mappings(), Require, the ResidentBuffer capability throw and the heapMirrorsRefused predicate with sparseBinding = false:
PR order (GuestNonzeroBlocks first):
error: AGC graphics: cannot read /proc/self/maps for a heap mirror's pages
heapMirrorsRefused predicate: false -> Require fails
capability checked first:
error: AGC graphics: heap mirror of 0x10000+0x10000: the device has no queue with sparse binding for a resident heap mirror; ...
heapMirrorsRefused predicate: true
Suggestion:
- Check
context.sparseBinding && context.sparseResidencyAliasedbeforeGuestNonzeroBlocksat line 2406, for example with a staticResidentBuffer::Supported(context)that throws throughheapMirrorError. This keeps the refusal the same on every host. - Gate the
mappings()/populatedPages()path on__linux__rather than!_WIN32, and give other hosts the conservative_WIN32branch (maybe.push_back({begin, end, true})). A sparse-capable non-Linux device then still gets a working mirror instead of a procfs error.
AI-assisted: yes (Claude Code)
|
AMD on Windows run. Radeon 860M (RDNA 3.5, driver 32.0.13058.8001), Windows 11 26300, WinLibs GCC 15.2.0 r7, Release,
AI-assisted |
|
Windows data: with this PR the 15 tests built from Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
The test files don't build on I also ran full CTest with this PR and the 14 other open AGC PRs I tested in the same batch merged together (#7641, #7630, #7627, #7603, #7600, #7591, #7579, #7535, #7533, #7474, #7462, #7050, #7048, #7650). Neither GPU showed a failure beyond AI-assisted: yes (Claude Code) |
e6a2bcb to
71ca66c
Compare
|
@jotitadev thanks, fixed in 71ca66c. A device without sparse binding, buffer residency or aliased residency now keeps the whole-range copy |
|
This is another reproduction of the #7047 pattern, this time on a discrete AMD GPU with most of its VRAM free. On this title the mirrored ranges are almost all untouched guest memory, which is the case this PR targets. Setup:
Stop: about 10 s after the overworld starts loading, the run fails with Memory at the stop (
What is mirrored: the title makes four 2432 MiB (
I have not run this PR on the title. It conflicts with current AI-assisted: yes (Claude Code) |
71ca66c to
0670cec
Compare
0670cec to
7472e23
Compare
|
I tested this head ( I prepared the small test-only correction below:
On your unmodified head, the resident-mirror test reports Skipped. With only this fixture correction, 4/4 focused checks pass without skips: Machine: Windows 11 build 26200, Intel i7-13700, NVIDIA RTX 4070, driver 610.88; GCC 15.2, Release, SPIR-V Tools enabled. Both main and candidate checks used This validates the focused test behavior on this machine, not a full Astro gameplay run or an FPS/RAM improvement. The runtime implementation is yours; this patch only enables the existing regression on Windows. Would you like to include it here? Test-fixture correctiondiff --git a/core/libs/prx/libSceAgcDriver/tests/execution/ResidentHeapMirror.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/ResidentHeapMirror.cpp
index 7657a227d..44cd382a4 100644
--- a/core/libs/prx/libSceAgcDriver/tests/execution/ResidentHeapMirror.cpp
+++ b/core/libs/prx/libSceAgcDriver/tests/execution/ResidentHeapMirror.cpp
@@ -3,6 +3,7 @@
#include "prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp"
#include "prx/libSceAgcDriver/Graphics/include/ResidentMirror.hpp"
#include "prx/libc/include/GuestAllocations.hpp"
+#include "prx/libc/include/GuestArena.hpp"
#include "prx/libc/include/GuestWriteWatch.hpp"
#include "Recompiler.hpp"
#include "VulkanTestDevice.hpp"
@@ -49,7 +50,8 @@ class OutputBlock {
public:
OutputBlock() {
#ifdef _WIN32
- block = static_cast<std::uint32_t*>(VirtualAlloc(nullptr, Block, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE));
+ block = static_cast<std::uint32_t*>(GuestArena::GuestArenaAllocate_nid_postfix(Block, Block));
+ if (block != nullptr) GuestArena::GuestArenaCommit_nid_postfix(block, Block, PAGE_READWRITE, Block);
#else
block = static_cast<std::uint32_t*>(std::aligned_alloc(Block, Block));
#endif
@@ -60,7 +62,8 @@ public:
~OutputBlock() {
GuestAllocations::Mutation().Remove(block);
#ifdef _WIN32
- VirtualFree(block, 0, MEM_RELEASE);
+ GuestArena::GuestArenaReset_nid_postfix(block, Block);
+ GuestArena::GuestArenaRelease_nid_postfix(block, Block);
#else
std::free(block);
#endif
@@ -80,8 +83,9 @@ public:
explicit GuestHeap(bool writable, std::uint64_t ranges = 1, std::uint64_t rangeBytes = HeapBytes, void* at = nullptr) : ranges(ranges), rangeBytes(rangeBytes) {
const auto bytes = rangeBytes * ranges;
#ifdef _WIN32
- raw = VirtualAlloc(at, bytes, MEM_RESERVE | MEM_COMMIT | MEM_WRITE_WATCH, PAGE_READWRITE);
+ raw = at != nullptr ? GuestArena::GuestArenaAllocateFromHint_nid_postfix(reinterpret_cast<std::uintptr_t>(at), bytes, Block) : GuestArena::GuestArenaAllocate_nid_postfix(bytes, Block);
Require(raw != nullptr && (at == nullptr || raw == at), "resident heap mirror: cannot allocate the guest heap");
+ GuestArena::GuestArenaCommit_nid_postfix(raw, bytes, PAGE_READWRITE, Block);
base = static_cast<std::uint8_t*>(raw);
#else
raw = mmap(at, bytes + Block, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE | (at != nullptr ? MAP_FIXED_NOREPLACE : 0), -1, 0);
@@ -100,7 +104,8 @@ public:
}
GuestWriteWatch::GuestWriteWatchUnregister_nid_postfix(base, rangeBytes * ranges);
#ifdef _WIN32
- VirtualFree(raw, 0, MEM_RELEASE);
+ GuestArena::GuestArenaReset_nid_postfix(raw, rangeBytes * ranges);
+ GuestArena::GuestArenaRelease_nid_postfix(raw, rangeBytes * ranges);
#else
munmap(raw, rangeBytes * ranges + Block);
#endif
@@ -325,7 +330,12 @@ int main() {
try {
const auto device = OpenVulkanTestDevice();
if (!device) return VulkanTestSkipped;
- if (!GuestWriteWatch::GuestWriteWatchAvailable_nid_postfix()) {
+#ifdef _WIN32
+ const bool watched = GuestArena::GuestArenaWriteWatched_nid_postfix();
+#else
+ const bool watched = GuestWriteWatch::GuestWriteWatchAvailable_nid_postfix();
+#endif
+ if (!watched) {
std::puts("skipped, no guest write watch: heap mirrors need one");
return VulkanTestSkipped;
} |
A heap mirror is now a sparse aliased buffer of 64 KiB blocks: blocks that hold nonzero guest bytes are bound to device-local memory with a host copy, and every other block aliases one zero block, bound as the guest writes it. A device without sparse binding, buffer residency or aliased residency, or without the sparse address space for a mirror, keeps the whole-range copy of main. Changed bytes reach the device by copies recorded before the build's work, and bytes the GPU wrote, also into the mirrors a cached address space holds, are copied back after it, so no mirror block needs a host-visible memory type. A device read of a mirror inside a GPU completion throws. Sparse binds take a free queue only. Main and settle queue selection is unchanged and the settle queue is kept: when no free queue binds, binds go to the main queue under its lock. With dma-buf import on Linux, the settle queue that main dropped when the sparse family was a one-queue settle family is now kept. A new joined host import bumps the import epoch, so the next build releases heap mirrors over its ranges instead of refreshing a range that is no longer write-watched.
7472e23 to
ed8f15b
Compare
|
Windows / Radeon 780M (Ryzen Z1 Extreme) run of this PR with SILENT HILL: Townfall (PPSA33286), the title from #7047. Setup
Results (360 s each)
Observations
Logs and the sampled CSV are available if useful. |
|
Every new throw path in |
|
You're right, that catch shouldn't have gone. Restored in 0b9b519: a sparse heap whose mirror can't be made prints the same New test |
|
A Windows / AMD APU data point: Radeon 860M ( Driver support.
Other tests. The other 43 tests matching At runtime it didn't run. I ran four alternating SuperTuxKart races (PPSA69421), two on |
|
@fma-rc I prepared the Windows test fixes in fma-rc#4, targeting your It adds the missing DLL-path setup and 30-second timeout for Both reported issues reproduced before the fix. Afterward, 12 unique focused tests pass with zero skips on Windows 11 / RTX 4070, driver 610.88; the resident test actually executes and passes. Matched Vulkan controls on main and the fix show identical known older-layer diagnostics, with no new validation-error signatures or missing-layer messages. Full details are in the fix PR. AI-assisted: yes, OpenAI Codex. |
|
Thanks @Zenodeon, cherry-picked as 2d50c08 with you as author. It adds On this head the Windows VM build passes 1760 of 1761 tests; the failing one doesn't compile on the merge base either (#8310). The VM has no GPU, so @LeandroLP23, if you have time, a run of |
|
@fma-rc ran
The build used |
|
@fma-rc here is the Bugsnax run I promised, on head Setup: RX 6700 XT (gfx1031, 12 GiB VRAM), 16 GB RAM (GTT 7934 MiB), Linux 7.2, RADV Mesa 26.2.5, Bugsnax (PPSA01503) 02.000.000,
|
|
Follow-up: a longer manual session on the same build, about 22 minutes of play in the overworld and cutscenes, with the window at 2560x1440 and the title rendering at 3840x2160.
|
|
@fma-rc a data point for this PR from #9514's thread: parl0v ran Sword of the Sea (PPSA25344) 01.510.000 on Windows 10, with an RTX 4060 8 GB and 16 GB of RAM (comment). Once #9514 gets the New Game load past the copy cap, it stops on |
|
@fma-rc a coordination note, since #9551 edits the same heap mirror code. What #9551 does:
How it meets this PR:
Neither depends on the other: #9551 changes which range a mirror is built over and when it is released, and this PR changes how a mirror's memory is allocated and bound. Whichever merges second needs the two resolutions above, and I'm happy to rebase #9551 onto this one if it lands first. |
|
@fma-rc #9670 adds a presentation queue in the device-creation block that this PR moves into Whichever lands second has to keep this:
|
|
Windows notes from GTA V (PPSA04264, RTX 4090 + Intel UHD 770). They come from my own mirror experiment on an older Device-local vs import. On a BDA bench with 64 MiB nodes, dependent reads took 210 ms from a host import and 0.78 ms from device-local memory on the 4090 (x271). On the UHD 770 (UMA) the ratio was 1.0. So on APUs a mirror only costs memory, and binding only written blocks is a pure win. On discrete GPUs mirrors are a big speed lever, and keeping them current costs the most. What sank my version: GPU writes. I used budgeted, chunked mirrors. In GTA, GPU writes into mirrored blocks made about 2.4 GB of copies per 10 s, and FPS fell from 2.0 to about 1.5. This PR copies GPU-written bytes back after each build. A copy-back bytes per 10 s count in the Windows nonzero scan. On Windows I can run GTA on your head and AI-assisted: yes (Claude Code) |
This branch has not been deployed
13 participants
What
A heap mirror copied its whole registered range into device memory, so a title with large GPU heaps paid for every page it had reserved, not just the ones it had written. Playroom (PPSA01325) on a Radeon 680M APU loses the device on current
mainat about 359 s:Its three heap mirrors hold 10126 MiB. Heap mirrors are made wherever a range's host import is refused, which happens once registered memory exceeds the import budget, so this is not specific to one GPU or OS. This is the heap mirror part of the device loss in #7047; that report is a Radeon 780M on Windows, which I could not run, and host imports are not changed here.
A heap mirror is now a sparse, aliased buffer of 64 KiB blocks. Each block that holds a nonzero guest byte is bound to device-local memory and keeps a host copy; every other block is bound to one shared zero block per device. Holes of the guest's shared memory are found with SEEK_DATA and never read. A block the guest or the GPU writes later is bound when the next build sees the change. Changed bytes reach the device by copies recorded before the build's work, and bytes the GPU wrote, including into the mirrors a cached address space holds, are copied back after it, so no block needs a host-visible memory type (the RTX 5080 has none that a sparse aliased buffer accepts). A device read of a mirror inside a GPU completion throws.
A device without sparseBinding, sparseResidencyBuffer and sparseResidencyAliased, or without enough sparse address space left for a mirror, keeps the whole-range copy
mainmakes. The RTX 5080, Radeon 610M and 680M (RADV) and lavapipe report all three; lavapipe has 2 GiB of sparse address space, so a larger heap range is copied whole there. From the driver sources, Intel before Gen12 on i915, Mali (panvk), Raspberry Pi (v3dv) and MoltenVK lack them. Device creation no longer limits sparse binding to dma-buf import.A sparse queue takes only a queue that is free. Main and settle queue selection is unchanged. When no free queue binds, binds go to the main queue under its lock. One change on Linux with dma-buf import:
maindropped the settle queue when the sparse family was the settle family with a single queue. That settle queue is now kept.A new joined host import bumps the import epoch, so the next build releases heap mirrors over its ranges. Without that, the next refresh throws
a heap mirror's range is no longer write-watched. PR 6546 and PR 6646 handle that throw by refreshing or re-planning the mirror. This PR removes the case where a joined import leaves the mirrors behind. It does not change what they do in other cases.Memory use is not lower overall: on the 680M the GTT peak drops and the process RSS peak is about the same (table). Texture snapshot memory (
CachedTexture.bytes) is left to PR 6982.Tested
Arch Linux,
mainc4c2b3a with and without this PR (the branch is rebased onto bf2c63f since, after #6646 and the import requester counts changed the same code).mainOver all runs on this machine, including earlier heads of both:
mainlost the device in 9 of 9 (342-372 s), this branch ran to the end in 12 of 12.New test
agc_driver_resident_heap_mirror:main: the heap mirrors of two 8192 KiB heaps hold 16384 KiB where the guest and the GPU wrote 192 KiB.the GPU store of thread 1 into a never-written heap block reached the guest as 0x0, expected 0x4.a heap mirror's range is no longer write-watched).SetResidentAddressSpaceLimit), the two heaps are mirrored whole and the reads and GPU stores still match. With that path replaced by a throw it fails 3 of 3 on the 680M and on lavapipe, and passes 3 of 3 on each with it.New test
agc_driver_device_queue_plancovers queue selection for 9 queue family layouts.On this head (rebased onto bf2c63f), all 96 tests that match the changed areas (
agc_import_watch,agc_*host_import*,agc_driver_bda*,agc_driver_address*,agc_driver_image_*,agc_driver_graphics, the heap mirror unwatch tests from #6646, the two new tests and others) pass on the NVIDIA RTX 5080 and lavapipe. On the RADV 610M 94 pass;agc_driver_image_table_vouched_lookupsandagc_driver_guest_memory_lifetimefail there and also fail onmain674c253 on the same GPU (3 of 3 runs each).agc_driver_native_depth_bda_clearis skipped on RADV and lavapipe as onmain.Driver suite (
agc_driver|recompiler) with this PR: lavapipe 541/541; RADV 680M 539/541, the two failures also fail onmain(agc_driver_tessellation, a Mesa 26.2 crash, andagc_driver_bda_device, see #7584); NVIDIAagc_driver|bda|mirror|import555/555. On the rebased head the full suite passes on lavapipe (1654/1654) and on the RADV 680M all but the four that also fail onmainthere (agc_driver_tessellation,agc_driver_bda_device,agc_driver_image_table_vouched_lookups,agc_driver_guest_memory_lifetime); On 2d50c08 (the Windows test fixes from @Zenodeon) the Windows VM (WinLibs GCC 15.2, no GPU) builds the PR and passes 1760 of 1761 tests; the one failure,agc_driver_image_array_window_past_layer_limit, does not compile there and fails the same way on the merge base (#8310 fixes it). The VM has no GPU; @LeandroLP23 ran the heap mirror tests on this head on a Radeon 860M (Windows 11, AMD driver 32.0.13058.8001): 8 of 8 pass in 4 runs, andagc_driver_resident_heap_mirrorruns there instead of skipping (comment).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)