From 7c1b60c5442bcca0c03464dcc61740ab5ea96e2d Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Fri, 2 Oct 2026 14:20:13 +1300 Subject: [PATCH] perf(agc): patch a stale storage snapshot's changed blocks in place A stale storage snapshot was compared whole with the guest bytes and, on any difference, copied whole into a new buffer, although few of the tracker blocks stamped since the copy usually hold different bytes. Only the 64 KiB tracker blocks stamped since the snapshot's generation are compared now. When some differ and the cache holds the only reference (no recorded draw binds the buffer), those blocks are copied into it in place; otherwise the range is copied again as before. agc_driver_recorder_tests drawSnapshotPatchTests: changed bytes in several blocks and a whole changed block patch the snapshot nothing else holds, an unchanged one is reused, and one a recorded draw still holds is copied again with its bytes untouched. --- .../Graphics/include/Recorder.hpp | 1 + .../libSceAgcDriver/Graphics/src/Recorder.cpp | 33 +++++- .../prx/libSceAgcDriver/tests/Recorder.cpp | 110 ++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp index a2d0d1934..de39cf34f 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp @@ -804,6 +804,7 @@ private: std::map drawSnapshots; std::array drawSnapshotPools; void eraseDrawSnapshot(std::map::iterator entry); + bool refreshDrawSnapshot(DrawSnapshot& entry, std::uint64_t address, std::size_t bytes); }; } diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp index c07b311e0..8702aa6a0 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp @@ -2466,8 +2466,7 @@ std::shared_ptr Recorder::ReusableDrawSnapshot(std::uint64_t address, st if (found == drawSnapshots.end() || std::get<0>(found->first) != address || std::get<1>(found->first) != use) return {}; const bool sameRegistry = found->second.registryGeneration == GuestAllocations::GuestAllocationsGeneration_nid_postfix(); if (!sameRegistry || !GuestMemory::UnchangedSince(address, bytes, found->second.generation)) { - const auto held = found->second.buffer->Bytes(); - if (!sameRegistry || generation == 0 || use != SnapshotUse::Storage || held.size() != bytes || std::memcmp(held.data(), reinterpret_cast(address), bytes) != 0) { + if (!sameRegistry || generation == 0 || use != SnapshotUse::Storage || !refreshDrawSnapshot(found->second, address, bytes)) { eraseDrawSnapshot(found); return {}; } @@ -2479,6 +2478,36 @@ std::shared_ptr Recorder::ReusableDrawSnapshot(std::uint64_t address, st return found->second.buffer; } +bool Recorder::refreshDrawSnapshot(DrawSnapshot& entry, std::uint64_t address, std::size_t bytes) { + const auto held = entry.buffer->Bytes(); + if (held.size() != bytes || bytes == 0) return false; + constexpr std::uint64_t block = 65536; + const auto aligned = address / block * block; + const auto end = address + bytes; + const auto count = static_cast((end - 1) / block - address / block + 1); + thread_local std::vector generations; + thread_local std::vector changed; + thread_local std::vector> differing; + generations.assign(count, entry.generation); + changed.assign(count, GuestMemory::BlockWritten); + differing.clear(); + const bool sole = entry.buffer.use_count() == 1; + GuestMemory::ChangedBlocks(address, bytes, generations, changed); + for (std::size_t k = 0; k < count; ++k) { + if (changed[k] == GuestMemory::BlockUnchanged) continue; + const auto from = std::max(address, aligned + k * block); + const auto to = std::min(end, aligned + (k + 1) * block); + const auto offset = static_cast(from - address); + const auto length = static_cast(to - from); + if (std::memcmp(held.data() + offset, reinterpret_cast(from), length) == 0) continue; + if (!sole) return false; + if (!differing.empty() && differing.back().first + differing.back().second == offset) differing.back().second += length; + else differing.emplace_back(offset, length); + } + for (const auto& [offset, length] : differing) std::memcpy(held.data() + offset, reinterpret_cast(address + offset), length); + return true; +} + void Recorder::KeepDrawSnapshot(std::uint64_t address, std::size_t bytes, std::uint64_t generation, std::uint64_t registryGeneration, std::shared_ptr buffer, SnapshotUse use, std::uint32_t derived) { const bool storage = use == SnapshotUse::Storage; const auto budget = storage ? DrawSnapshotBudget : DrawInputBudget; diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 6561d67e4..89e9a521c 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -1405,6 +1405,114 @@ void drawSnapshotReuseTests(const Device& device, Recorder& recorder) { recorder.Activate(); } +void drawSnapshotPatchTests(const Device& device) { + const auto& context = device.GetContext(); + constexpr std::size_t bytes = 4 * 65536; + std::unique_lock gpu(GpuMutex()); + void* block = context.hostImportAlignment != 0 ? AllocateWatched(bytes, 65536) : nullptr; + if (block == nullptr) { + std::cout << "host imports or write watching unavailable: snapshot patching not tested\n"; + return; + } + struct Release { + void* block; + ~Release() { ReleaseWatched(block, bytes); } + } release{block}; + auto* guest = static_cast(block); + for (std::size_t at = 0; at < bytes; ++at) guest[at] = static_cast(at * 13u + 5u); + const auto address = reinterpret_cast(block); + { + GuestAllocations::Mutation mutation; + mutation.Add(block, bytes, true, true, true); + } + struct Unregister { + const Context& context; + void* block; + ~Unregister() { + { + GuestAllocations::Mutation mutation; + mutation.Remove(block); + } + HostImportFor(context, reinterpret_cast(block), bytes); + } + } unregister{context, block}; + if (HostImportFor(context, address, bytes) == nullptr) { + std::cout << "host import of the watched block refused: snapshot patching not tested\n"; + return; + } + if (!AgcDriver::GuestMemory::Watched(address, bytes)) { + std::cout << "host imports are compared, not watched: snapshot patching not tested\n"; + return; + } + constexpr std::size_t elementOffset = 4096; + constexpr std::size_t elementBytes = 3 * 65536; + const auto element = address + elementOffset; + ShaderRecompiler::RecompileResult program; + ShaderRecompiler::DescriptorBinding binding; + binding.kind = ShaderRecompiler::DescriptorKind::StorageBuffer; + binding.role = ShaderRecompiler::DescriptorRole::GuestBuffers; + binding.descriptorSet = 0; + binding.binding = 0; + binding.count = 1; + binding.guestDescriptor = {static_cast(element), static_cast(element >> 32u) & 0xffffu, static_cast(elementBytes), 0x31000000u}; + binding.bufferWritten = {false}; + program.bindings.push_back(binding); + const CompiledShader compute{ShaderRecompiler::ShaderStage::Compute, &program, 0}; + auto snapshotContext = context; + DescriptorCache cache(snapshotContext); + snapshotContext.descriptorCache = &cache; + Recorder snapshotRecorder(snapshotContext); + snapshotRecorder.Activate(); + { + ShaderResources resources(snapshotContext, compute); + const auto snapshot = [&] { + auto bindings = resources.PrepareDrawBindings(snapshotRecorder); + Require(bindings != nullptr && bindings->snapshots.size() == 1, "read-only draw input was not snapshotted"); + auto buffer = bindings->snapshots[0].buffer; + const auto contents = buffer->Bytes(); + Require(contents.size() == elementBytes && std::memcmp(contents.data(), reinterpret_cast(element), elementBytes) == 0, "a draw snapshot does not hold the guest bytes of its draw"); + return buffer; + }; + const auto settle = [&](const std::weak_ptr& buffer) { + snapshotRecorder.Sync(); + gpu.unlock(); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + while (buffer.use_count() > 1 && std::chrono::steady_clock::now() < deadline) std::this_thread::sleep_for(std::chrono::milliseconds(1)); + gpu.lock(); + Require(buffer.use_count() == 1, "a finished draw still holds its snapshot"); + }; + std::weak_ptr first = snapshot(); + settle(first); + guest[elementOffset + 10] ^= 0xffu; + guest[elementOffset + 65536 + 300] ^= 0xffu; + guest[elementOffset + 2 * 65536 + 7] = guest[elementOffset + 2 * 65536 + 7]; + guest[elementOffset + elementBytes - 1] ^= 0x5au; + AgcDriver::GuestMemory::BumpCollectEpoch(); + { + const auto patched = snapshot(); + Require(patched == first.lock(), "a stale snapshot nothing else holds was copied whole instead of patched"); + } + settle(first); + std::memset(guest + elementOffset + 65536, 0x77, 65536); + AgcDriver::GuestMemory::BumpCollectEpoch(); + { + const auto patched = snapshot(); + Require(patched == first.lock(), "a snapshot whose whole middle block changed was not patched"); + } + settle(first); + const auto held = snapshot(); + Require(held == first.lock(), "an unchanged snapshot was not reused"); + const std::vector before(held->Bytes().begin(), held->Bytes().end()); + guest[elementOffset + 2 * 65536 + 100] ^= 0xffu; + AgcDriver::GuestMemory::BumpCollectEpoch(); + const auto fresh = snapshot(); + Require(fresh != held, "a snapshot a recorded draw still holds was patched"); + Require(std::memcmp(held->Bytes().data(), before.data(), before.size()) == 0, "a held snapshot's bytes changed"); + snapshotRecorder.Sync(); + } +} + + void drawSnapshotEvictionTests(const Device& device) { using namespace AgcDriver::GuestMemory; constexpr std::size_t bytes = 65536; @@ -3929,7 +4037,9 @@ int main(int argc, char** argv) { sampleDumpTests(device, recorder); gpu.unlock(); hostImportUnmapTests(device, recorder); + drawSnapshotPatchTests(device); gpu.lock(); + recorder.Activate(); std::cout << "Recorder read tracking and label tests passed\n"; return 0; } catch (const std::exception& error) {