Repository navigation
Conversation
|
Same bug in a second title: Ready or Not (PPSA18605, UE5), Windows 11, RX 7900 XTX. The case. A 768 KiB buffer the title binds on nearly every draw starts at a 640 KiB registered range and runs 128 KiB into the next one, a 256 KiB range ( What it costs, scene, 30 s profile windows, my bring-up build (
With this PR (2 sessions, driver built from your head):
One question. Joins here have no size bound, and a join is re-imported whole each time a bound buffer crosses into a new piece. The host-import debt entry says the 8 MiB window was introduced because runs re-imported on every growth made the NVIDIA driver (610.57.04, 615.78.08) refuse imports or lose the device. Ready or Not's joins stay under 3 MiB, so it doesn't exercise that. Has this run on NVIDIA with a title that grows a heap in pieces? If a cap turns out to be needed, the window-bounded check is one line in |
11fd229 to
1682f90
Compare
1682f90 to
201218a
Compare
|
The full build of this merged with current |
201218a to
eb0e2e6
Compare
|
@oneandonlydean thanks, reproduced. With #290 in, a new piece that completes a window is imported as that window, which isn't a joined import, so @Nesyth on NVIDIA: tested only on an RTX 5070 (driver 610.88, Windows 11). In MCD2 there were no import failures or device loss, with joins up to about 16 MiB. Not tested: Linux NVIDIA, and a title that grows a heap in many small pieces under bound buffers, which is the case that made the 8 MiB window necessary. If you can share the window-bounded cap and its tests, I'm happy to take it into this PR so a join never outgrows the window. |
eb0e2e6 to
2764743
Compare
|
Kena: Bridge of Spirits (PPSA01802) hits the same case. Tested on Linux (Arch, kernel 7.2.9), RX 9070 XT (RADV, Mesa 26.2.4), with the three commits cherry-picked onto a local Kena stack (main from 2026-10-14 plus unmerged GPU PRs and stand-ins Kena needs to reach gameplay; plain main does not reach the title scene, so this is not a plain-main capture). Same save, 120 s runs,
Example lines from one run each: In the slow windows the builds that copied cost 380 to 880 µs each ( |
|
Thanks for the rebase. The
|
2764743 to
68f7a25
Compare
|
@oneandonlydean thanks, both done in 68f7a25 (rebased on
|
|
@alfalfa-star thank you for the Kena runs, that's the first RADV result for this and a third title with the same straddle. I've added it to the description. One note: since your run, the join is capped at the 8 MiB import window (68f7a25, from the review above), so a Kena join larger than that would be copied again. If you get a chance to rerun with the current head, it would show whether any of Kena's joins hit the cap. |
|
@TideOPL I reran Kena: Bridge of Spirits (PPSA01802) with the current head. No join hit the 8 MiB cap. Setup as before: Linux (Arch, kernel 7.2.9), RX 9070 XT (RADV, Mesa 26.2.4), my local Kena stack (main from 2026-10-14 plus unmerged GPU PRs and stand-ins; plain
One current-head run (run 3) aborted during loading. RADV printed AI-assisted: yes (Claude Code) |
github-actions
Bot
added
the
perf numbers missing
# Conflicts: # docs/dev/debt/libSceAgcDriver.md
|
This pull request is labeled |
…t window Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Stellar Blade, playable area, RX 9070 XT (AMD 26.8.1, Windows), 3840x2160. One main-vs-change pair, The change side is this PR's join logic ported onto The joins fire. In queue 0, over the same 90 s window:
Before the change, about 11.5k dispatches per 90 s missed the resource cache, synced and copied a 1–2.5 MiB buffer that started in one registered piece and ended in the next. Older profiled runs show these copies with the reason "window incomplete". Frame rate does not move on this title: 563 presents (6.26 fps) on main, 559 (6.21 fps) with the change. GPU busy is 84.9 vs 84.7 ms per present. Queue 0 stays about 98% busy in both runs, because its draw packets took the time back ( Focused tests with Traces (about 40 MB each) are available if useful. |
|
The numbers show the join doing its job on a Windows AMD device: One scope note for the record: the change side is the join logic ported onto |
|
I found a way for the widened join to bypass the agreed 8 MiB partial-join cap on head The synthetic case registers four adjacent 4 MiB pieces, imports the first and last, then binds 12.5 MiB starting at the first piece. The binding ends inside the fourth piece. A five-line correction retains the original binding bounds and checks the final widened run. The exact-whole-run exception requires the original binding to cover that final run; the exception for ranges each larger than the window is preserved. A rejected widened join does not retry a smaller run or retire the existing imports. I independently rebuilt and ran the final synthetic test directly against this exact PR head, then against the standalone correction based solely on that head. This comparison does not depend on the earlier current-main integration:
The test checks the entire 32 MiB backing allocation, including the unused tail inside the joined import and the surrounding guards. The fallback case checks copying and unchanged initial contents; the two direct-import cases execute GPU fills through the returned buffer view and offset. The same five existing tests pass before and after: Setup: Ryzen 9 3900X, RTX 3080 Ti 12 GB, NVIDIA 610.88, Windows 11 Pro 10.0.26300, supported WinLibs GCC 15.2.0 r7, Release. Both sides used The patch below applies to your exact head and contains only five added production lines and the 120-line synthetic test. Conventions and whitespace checks pass against those exact Git objects. It leaves the default window at 8 MiB. The fixture contains no game data or captures. SF6 speedup, gameplay correctness and prevention of real-game device loss remain unproven; no title benchmark is claimed. AI-assisted code, review and testing. Complete proposed patch, based directly on 0ccc15dFrom 545a8a3f9eed6c27984e760d62bb3a10a6ee44f3 Mon Sep 17 00:00:00 2001
From: embersas
Date: Tue, 20 Oct 2026 18:01:15 +1000
Subject: [PATCH] fix(agc): preserve the join bound before import widening
---
.../Graphics/src/GuestBufferMemory.cpp | 5 +
.../tests/execution/HostImportJoinWindow.cpp | 120 ++++++++++++++++++
2 files changed, 125 insertions(+)
create mode 100644 core/libs/prx/libSceAgcDriver/tests/execution/HostImportJoinWindow.cpp
diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
index e863ee351..070e75c78 100644
--- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
+++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
@@ -1629,10 +1629,15 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s
}
const HostImport* importJoined(const Context& context, HostImports& state, std::uint64_t begin, std::uint64_t end, const GuestAllocations::Lease& lease) {
+ const auto boundBegin = begin;
+ const auto boundEnd = end;
if (const auto* low = peekImport(state, begin, begin + 1); low != nullptr) begin = low->base;
if (const auto* high = peekImport(state, end - 1, end); high != nullptr) end = high->base + high->bytes;
auto run = joinedRun(lease, begin, end);
if (run.ranges.empty()) return nullptr;
+ const auto window = importWindow.load(std::memory_order_relaxed);
+ const bool exact = boundBegin == run.base && boundEnd == run.base + run.bytes;
+ if (!exact && run.bytes > window && !std::all_of(run.ranges.begin(), run.ranges.end(), [window](const auto& range) { return range->bytes > window; })) return nullptr;
if (const auto failed = state.failedJoins.find(run.base); failed != state.failedJoins.end() && sameRun(failed->second, run)) return nullptr;
return importRunAt(context, state, std::move(run), true, begin, lease, end, true, ImportRequester::Join);
}
diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/HostImportJoinWindow.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/HostImportJoinWindow.cpp
new file mode 100644
index 000000000..0afe06fcc
--- /dev/null
+++ b/core/libs/prx/libSceAgcDriver/tests/execution/HostImportJoinWindow.cpp
@@ -0,0 +1,120 @@
+#include "VulkanTestDevice.hpp"
+#include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp"
+#include "prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp"
+#include "prx/libSceAgcDriver/Graphics/include/Resources.hpp"
+#include "prx/libc/include/GuestAllocations.hpp"
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+#include <exception>
+#include <iostream>
+#include <mutex>
+#include <new>
+
+namespace {
+
+namespace Graphics = AgcDriver::Graphics;
+using Graphics::Require;
+
+constexpr std::size_t MiB = 1u << 20u;
+constexpr std::size_t PieceBytes = 4 * MiB;
+constexpr std::size_t AllocationBytes = 8 * PieceBytes;
+constexpr std::size_t BoundBytes = 12 * MiB + MiB / 2;
+
+class Pieces {
+public:
+ explicit Pieces(const Graphics::Context& context) : context(context) {
+ block = static_cast<std::uint32_t*>(::operator new(AllocationBytes, std::align_val_t{4 * PieceBytes}));
+ std::memset(block, 0x11, AllocationBytes);
+ GuestAllocations::Mutation mutation;
+ for (std::size_t index = 1; index <= 4; ++index) mutation.Add(reinterpret_cast<std::uint8_t*>(block) + index * PieceBytes, PieceBytes, true, true, true);
+ }
+
+ ~Pieces() {
+ Graphics::ClearHostImports(context.device);
+ {
+ GuestAllocations::Mutation mutation;
+ for (std::size_t index = 1; index <= 4; ++index) mutation.Remove(reinterpret_cast<std::uint8_t*>(block) + index * PieceBytes);
+ }
+ ::operator delete(block, std::align_val_t{4 * PieceBytes});
+ }
+
+ Pieces(const Pieces&) = delete;
+ Pieces& operator=(const Pieces&) = delete;
+
+ std::uint64_t Address() const { return reinterpret_cast<std::uintptr_t>(block) + PieceBytes; }
+
+ void Check(std::uint32_t pattern, std::size_t boundBytes) const {
+ for (std::size_t index = 0; index < AllocationBytes / sizeof(*block); ++index) {
+ const auto byte = index * sizeof(*block);
+ const auto expected = byte >= PieceBytes && byte < PieceBytes + boundBytes ? pattern : 0x11111111u;
+ Require(block[index] == expected, "a partial joined buffer lost GPU output or changed bytes outside its binding");
+ }
+ }
+
+private:
+ Graphics::Context context;
+ std::uint32_t* block = nullptr;
+};
+
+void CheckWindow(const Graphics::Context& context, std::size_t window, std::size_t boundBytes, bool imported) {
+ Graphics::ClearHostImports(context.device);
+ Graphics::SetHostImportWindow(window);
+ Pieces pieces(context);
+ const auto address = pieces.Address();
+ Require(Graphics::HostImportFor(context, address, 16) != nullptr, "the first registered piece was not imported");
+ Require(Graphics::HostImportFor(context, address + 4 * PieceBytes - 16, 16) != nullptr, "the last registered piece was not imported");
+ Graphics::GuestBufferMemory memory(context);
+ memory.AddWritable(address, boundBytes);
+ memory.Upload(false);
+ const auto copied = memory.CopiedBytes();
+ std::uint32_t adjustment = 0;
+ const auto view = memory.Descriptor(address, boundBytes, adjustment);
+ const auto* direct = Graphics::ExistingHostImportFor(context, address, boundBytes);
+ if (imported) {
+ Require(copied == 0 && direct != nullptr, "an eligible large binding over four pieces was copied");
+ Require(view.buffer == direct->buffer && view.offset + adjustment == address - direct->base, "the partial binding does not refer to its joined import");
+ } else {
+ Require(copied == boundBytes && direct == nullptr, "an 8 MiB window joined a partial binding spanning 16 MiB");
+ memory.WriteBack();
+ pieces.Check(0x11111111u, boundBytes);
+ std::printf("join window %zu MiB, binding %.1f MiB: %zu copied bytes; initial bytes preserved\n", window / MiB, static_cast<double>(boundBytes) / MiB, static_cast<std::size_t>(copied));
+ return;
+ }
+ constexpr std::uint32_t pattern = 0xa5b6c7d8u;
+ Graphics::CommandBatch batch(context);
+ const auto commands = batch.Handle();
+ context.Function<PFN_vkCmdFillBuffer>("vkCmdFillBuffer")(commands, view.buffer, view.offset + adjustment, boundBytes, pattern);
+ VkMemoryBarrier barrier{VK_STRUCTURE_TYPE_MEMORY_BARRIER};
+ barrier.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT;
+ barrier.dstAccessMask = VK_ACCESS_HOST_READ_BIT;
+ context.Function<PFN_vkCmdPipelineBarrier>("vkCmdPipelineBarrier")(commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_HOST_BIT, 0, 1, &barrier, 0, nullptr, 0, nullptr);
+ batch.SubmitAndWait();
+ memory.WriteBack();
+ pieces.Check(pattern, boundBytes);
+ std::printf("join window %zu MiB, binding %.1f MiB: %zu copied bytes; GPU output and surrounding bytes preserved\n", window / MiB, static_cast<double>(boundBytes) / MiB, static_cast<std::size_t>(copied));
+}
+
+}
+
+int main() {
+ try {
+ auto device = OpenVulkanTestDevice();
+ if (!device) return VulkanTestSkipped;
+ const auto context = device->graphicsContext();
+ if (context.hostImportAlignment == 0) {
+ std::puts("skipped, the device does not support host imports");
+ return VulkanTestSkipped;
+ }
+ std::lock_guard lock(AgcDriver::GuestMemory::GpuMutex());
+ CheckWindow(context, 8 * MiB, BoundBytes, false);
+ CheckWindow(context, 8 * MiB, 4 * PieceBytes, true);
+ CheckWindow(context, 16 * MiB, BoundBytes, true);
+ Graphics::SetHostImportWindow(8 * MiB);
+ std::puts("host import join window tests passed");
+ return 0;
+ } catch (const std::exception& error) {
+ std::cerr << error.what() << '\n';
+ return 1;
+ }
+}
--
2.56.0.windows.2 |
|
@TideOPL, @embersas's case checks out against the code on Could you fold in the correction from that comment (keep the binding's original bounds for the exactness check, keep the exception for runs whose pieces are each larger than the window) together with its |
This branch has not been deployed
11 participants
Minecraft Dungeons II (PPSA16064) gameplay: frame p50 92–97 ms in every run, against 100–131 ms when a run lands in the slow mode. That mode copies one buffer on the CPU about 40,000 times in 25 s.
What
Follow-up to #4517, which is now merged. This branch is four commits rebased on
main3aebaa7, thenmainmerged in twice: by a maintainer at 83bf557, and at bf2c63f (0ccc15d).mainnow includes #290 (host import windows), #5249 (sparse descriptor heap runs), #5428 (read-only V# across two imports), #7061, and #6457 and #6479 (aligned power-of-two import blocks). The rebase onto #7061 keeps itspeekImportinimportJoined(an import lookup that does not mark the import as used) and drops only thejoinedcondition, as SleglJan did in their builds below. The merge of bf2c63f conflicted only in the TechnicalDebt entry: it keepsmain's block wording and replaces the join and grown-heap sentences with this PR's. The code merged without conflicts, and the PR's diff tomaininGuestBufferMemory.cppis still the twojoinedconditions and therun.bytes <= windowline.0x305fca0000+0x160000over0x110000 + 0x30000 + part of 0x50000;0x306c410000+0x160000over0x110000 + part of 0x70000.joinedRunrefuses the binding because it covers its pieces only partly. The buffer is then copied on the CPU for every draw that binds it, and each of those draws gets a fresh resource build instead of a cache hit: about 280 builds per frame instead of about 5. Which order a run gets is a race, so about half the runs were slow.joinedRun. It now joins every range the bound region crosses, whole, wherever the binding starts or ends inside them, as long as the joined run is no larger than the import window (8 MiB). perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517's two rules stay as they were: runs of ranges that are each larger than the window, and a buffer bound exactly over its ranges, join at any size.importJoinedwidens the join to the imports the buffer starts or ends in, so overlapping binds merge into one import instead of retiring each other.importJoined(second commit). It widened only into an adjacent import marked joined. With feat(agc): host import windows, least-recent eviction under the budget #290, a newly registered piece that completes a window is imported as that window, which is not a joined import, so a bind across the older boundary joined only the crossed pieces and retired the window, and the next bind across the newer boundary imported the window again: every alternating bind re-imported (zero copies, but a new import each time). The join now widens over any import the bound buffer starts or ends in, and falls back to the crossed ranges alone when the widened run cannot be joined.spaceReadAcrossImportsTestsandspaceReadAcrossImportsWrittenTestsbind a read-only V# across two adjacent read-only ranges. With this PR those two ranges are joined and the V# is bound in place, so the case they cover, a V# copied across two imports, no longer arises there (and the mock device does not back imports with guest bytes). The third range is now registered writable, so the V# crosses two imports that differ in access, which this PR still copies. Both tests pass that way onmain9a3e2ac as well.importJoineddoes not retry with the crossed ranges alone. A failed join copies the buffer and keeps the imports it would have retired, so it cannot retire an import the next bind re-imports. A heap grown in small pieces under bound buffers is re-imported at most once per piece until its run reaches the window, and copied past it: the bound the 8 MiB window was introduced for (NVIDIA 610.57.04, 615.78.08). MCD2's joins stay under 2 MiB and Ready or Not's under 3 MiB.The first version of this PR also fixed
UploadFinishbinding a region past the end of an import that covers only its start. That fix went in with #4517, so it is no longer part of this diff.Tested
Windows 11, WinLibs GCC 15.2, Release.
importJoinTests(agc_driver_graphics):mainbf2c63f (with perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517, feat(agc): host import windows, least-recent eviction under the budget #290, perf(agc): import adjacent host ranges as aligned blocks and stop re-trying budget refusals #6457 and perf(agc): take the host import budget from the device's memory budget #6479 merged;agc_driver_graphics_testsrebuilt withmain'sGuestBufferMemory.cpp) it fails witha buffer bound from one piece into the next, both no larger than the import window, was not imported over both pieces.a bind across a grown heap joined more than the import window or retired the newer import; withmain's import code the test fails earlier, at the piece-crossing check above.Suites: on this head (0ccc15d,
mainbf2c63f),ctest -R "^agc_driver|^guest_memory|^guest_allocations"passes 593 of 597.agc_driver_sparse_dma_buf_importandagc_driver_stencil_ref_exportare skipped, as onmain.agc_driver_msaa_dcc_clear_4andagc_driver_pass_input_hazardfail here withmain'sGuestBufferMemory.cppas well (RTX 5070, Windows), so they aren't from this PR.agc_driver_graphics, which holds this PR's tests, passes. Earlier, onmain3aebaa7, the same selection passed 538/538. Before the first rebase, onmain4eb91ea with every test executable rebuilt, the fullctestpassed 1298/1299. The one left isagc_driver_gpu_indirect_arguments_misaligned_writer_ahead, frommain, which does not build with WinLibs GCC 15.2 (requested alignment '65536' exceeds object file maximum 8192); this PR does not touch it.Conventions:
tools/check_conventions.pypasses.MCD2, Squid Coast spawn:
APS5_TRACE_SECONDS=25:15, opened 25 s after the gameplay HUD appears; interleaved runs with a warm-up boot per side; RTX 5070, driver 610.88.main77b9f72 plus the open PRs MCD2 needs, perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517 among them. "After" is the same build plus the first version of this change.Graphics.Drawdrops from 47–63 to 28–30 ms per frame in the slow runs.joinedRunchange alone gave the same split: before 94.3 / 115.4 / 114.8 / 93.3 ms p50, after 93.3 / 93.6 / 93.9 ms.joined at bindper run, before and after. In the three logged runs with the change there were no import failures and no device loss, and the largest joined import was 16 MiB. MCD2 does not grow a heap piece by piece under bound buffers, so that case has not been run on NVIDIA.APS5_TRACE_BIGBUF=1slows loading and made the slow order happen in 5 of 5 runs before; after, 3 of 3 were fast.dungeons2.exe+0x4cf10b. It happened both before (run 1) and after (a warm-up boot), and on older builds, so it is not from this change.Second title: Nesyth reports the same straddle in Ready or Not (PPSA18605) on an RX 7900 XTX; their numbers are in the comments below.
Third title, AMD: alfalfa-star tested the first three commits in Kena: Bridge of Spirits (PPSA01802) on Linux (Arch, kernel 7.2.9) with an RX 9070 XT (RADV, Mesa 26.2.4), on their Kena stack (not plain
main): over 120 s runs, CPU copies outside imports went from 3,000–72,496 per run (1.8–55 GiB, 17 runs) to 0 (7 runs), the builds that copied went from 380–880 µs to 37–39 µs, and there were no GPU resets. They reran with the capped head (68f7a25) against the previous head: no join hit the 8 MiB cap, 0 copies outside imports on both, frame rates within run-to-run noise, no GPU reset. One capped run aborted while loading with a RADV out-of-memory command submission and device loss; the same kernel error then stopped another title on a build without this PR, so they attribute it to their system that day. Details in the comments below.MCD2 on other machines, this head (e539061):
main636b1b9 + fix(agc): run the unwatch hook when a watched import leaves the write watch #6308: base 4.79 / 4.18 FPS, with this PR 6.08 / 6.34 (+38 % on the means), no aborts. The slowcopyphases (34–44 ms per present) seen in some base rows did not appear with the PR.main5d9dcc4 + fix(agc): run the unwatch hook when a watched import leaves the write watch #6308 + perf(agc): stop pooling released device buffers over the memory budget #4472: 1.95 FPS mean (6 rows) without this PR, 3.65 (8 rows) with it. These ran as consecutive batches, not interleaved, and the second also had perf(agc): retile storage write-back windows straight into their unit-shadow slabs #6490.Not run: Linux and AMD here (alfalfa-star's Kena run above is on RADV); a title that grows a heap in pieces on NVIDIA (the window cap now bounds that case as the window does on
main). The traces are not attached; I can share them on request.Checklist
main; no other open PR implements the same functions (perf(memory): restrict registry work to affected ranges #1324 changes other functions in the same file)NotImplemented_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)