From 30321142cb2e37b67b26ca2d3ff5d5ab47cce8d4 Mon Sep 17 00:00:00 2001 From: kuba Date: Wed, 7 Oct 2026 10:51:48 +0200 Subject: [PATCH] [GPU/Vk] Keep guest readbacks of GPU-written memory off the command worker (#1032) * [Vk] Run async readbacks on a second general queue family when no dedicated one exists The async readback queue was only taken from a family without graphics, so drivers that expose several general families (each its own hardware queue) kept every guest readback on the main queue and the command worker. Any other family with transfer support now qualifies after the dedicated transfer and compute families. (cherry picked from commit 7b04d6185dfc1711f0fbdf5c75c842a065acd1fa) * [GPU] Let guest reads of GPU-written memory wait on the main queue off the worker A guest thread reading memory the GPU wrote hands the download to the command worker, which recorded the copies, submitted and then waited for the whole GPU before handing the bytes back - unless a dedicated readback queue existed, where the download ran asynchronously and only the guest thread waited. MoltenVK exposes one queue family in-process, so on macOS every such read idled the worker for tens of milliseconds, several times per frame in Silent Hill: The Short Message's menu (a quarter of the worker's time). Without a readback queue the worker now copies the pieces into a buffer of their own, submits without waiting and returns a pending download carrying the submission tick; the guest thread waits for that tick on the timeline (safe from any thread) and completes the read back on the worker, as the readback-queue path does. The worker's GPU wait time drops to zero; SHARPEMU_MAIN_QUEUE_PENDING_READS=0 restores the synchronous path. (cherry picked from commit 6c9ad44bcea7f9396a7564621fcbda3841834052) * [GPU] Re-issue a stale pending readback instead of reading synchronously on the worker A guest read of GPU-written memory records a copy and waits for its tick off the queue. When another GPU write lands first, the completion fell back to a synchronous read on the command worker, which stalled it until the GPU drained (up to half of SH's readbacks). The guest thread now re-issues the pending read and waits again; only the fourth attempt falls back to the synchronous read. (cherry picked from commit 17bf5bb958ebfd4dd980dbf241cbe9e77fffd938) --- .../Gpu/Buffers/GuestBufferCache.cs | 180 +++++++++++++++++- .../Gpu/Scheduling/SubmissionScheduler.cs | 12 ++ .../VulkanVideoPresenter.Device.Setup.cs | 10 +- .../Buffers/GuestBufferAsyncReadbackTests.cs | 27 +++ 4 files changed, 220 insertions(+), 9 deletions(-) diff --git a/src/SharpEmu.Libs/Gpu/Buffers/GuestBufferCache.cs b/src/SharpEmu.Libs/Gpu/Buffers/GuestBufferCache.cs index 183f607..aa2c6fb 100644 --- a/src/SharpEmu.Libs/Gpu/Buffers/GuestBufferCache.cs +++ b/src/SharpEmu.Libs/Gpu/Buffers/GuestBufferCache.cs @@ -896,24 +896,45 @@ public sealed unsafe class GuestBufferCache : IGuestBufferStore, IDisposable $"unsupported buffer readback from an asynchronous GPU completion, addr=0x{guestAddress:X16} size=0x{size:X16}"); } - if (!_relay.IsGpuQueueThread && AsyncReadback is { } readback && GuestReadsAwaitOffQueue) + if (!_relay.IsGpuQueueThread && GuestReadsAwaitOffQueue && (AsyncReadback is not null || MainQueuePendingReads)) { - PendingDownload? pending = null; - if (_relay.TryRunOnGpuQueue(() => pending = BeginReadMemoryOnGpu(guestAddress, size, isWrite, source, allowPending: true))) + // A GPU write that lands after the copy was recorded makes the read stale. Retrying + // synchronously stalls the command worker until the GPU drains, so re-issue the read and + // wait here again; only the last attempt falls back to the synchronous read. + for (var attempt = 1; ; attempt++) { + PendingDownload? pending = null; + if (!_relay.TryRunOnGpuQueue(() => pending = BeginReadMemoryOnGpu(guestAddress, size, isWrite, source, allowPending: true))) + { + return AwaitShutdown(); + } + if (pending is null) { return true; } - readback.Wait(pending.Ticket); - if (_relay.TryRunOnGpuQueue(() => CompleteReadMemoryOnGpu(pending, retrySynchronously: true))) + if (pending.Ticket is { } ticket) + { + (AsyncReadback ?? throw SubmissionScheduler.Fatal("The pending readback lost its queue.")).Wait(ticket); + } + else + { + _scheduler.WaitForSubmittedTick(pending.MainQueueTick); + } + + var finalAttempt = attempt >= PendingReadAttempts; + var applied = false; + if (!_relay.TryRunOnGpuQueue(() => applied = CompleteReadMemoryOnGpu(pending, retrySynchronously: finalAttempt))) + { + return AwaitShutdown(); + } + + if (applied || finalAttempt) { return true; } } - - return AwaitShutdown(); } var onQueue = _relay.IsGpuQueueThread; @@ -1144,9 +1165,14 @@ public sealed unsafe class GuestBufferCache : IGuestBufferStore, IDisposable public static string TakeAsyncReadbackReport() => FormattableString.Invariant( $"[PERF][ASYNC_READBACK] applied={Interlocked.Exchange(ref _reportedApplied, 0)} retried={Interlocked.Exchange(ref _reportedRetried, 0)} eager_started={Interlocked.Exchange(ref _reportedEagerStarted, 0)} eager_used={Interlocked.Exchange(ref _reportedEagerUsed, 0)} eager_stale={Interlocked.Exchange(ref _reportedEagerStale, 0)}"); + // Ticket is set when the readback queue carries the download; otherwise the main queue + // does, into MainQueueBuffer, and MainQueueTick is the submission the guest thread waits for. private sealed record PendingDownload { - public required VulkanAsyncReadback.Ticket Ticket { get; init; } + public VulkanAsyncReadback.Ticket? Ticket { get; init; } + public ulong MainQueueTick { get; init; } + public GpuBuffer? MainQueueBuffer { get; init; } + public ulong[] MainQueueOffsets { get; init; } = []; public required List Copies { get; init; } public required ulong[] WriteTicks { get; init; } public required ulong WindowBegin { get; init; } @@ -1201,6 +1227,31 @@ public sealed unsafe class GuestBufferCache : IGuestBufferStore, IDisposable }; } + if (copies.Count != 0 && allowPending && + TryBeginDownloadOnMainQueue(copies, out var mainTick, out var mainBuffer, out var mainOffsets, out var mainWriteTicks)) + { + foreach (var copy in copies) + { + copy.Buffer.RetainForeignRead(); + } + + return new PendingDownload + { + MainQueueTick = mainTick, + MainQueueBuffer = mainBuffer, + MainQueueOffsets = mainOffsets, + Copies = copies, + WriteTicks = mainWriteTicks, + WindowBegin = windowBegin, + WindowEnd = windowEnd, + GuestAddress = guestAddress, + Size = size, + IsWrite = isWrite, + Source = source, + Started = readbackStarted, + }; + } + if (copies.Count != 0) { DownloadBufferMemory(copies); @@ -1216,9 +1267,70 @@ public sealed unsafe class GuestBufferCache : IGuestBufferStore, IDisposable return null; } + // Without a readback queue the main queue copies the pieces into a buffer of their own and + // submits at once; the guest thread then waits for that submission instead of the worker + // waiting for the whole GPU (a guest read of GPU-written memory otherwise idled the worker + // for tens of milliseconds, several times per frame in Silent Hill: The Short Message). + private bool TryBeginDownloadOnMainQueue(List copies, out ulong tick, out GpuBuffer buffer, out ulong[] offsets, out ulong[] writeTicks) + { + tick = 0; + buffer = null!; + offsets = []; + writeTicks = []; + if (!MainQueuePendingReads || copies.Count == 0) + { + return false; + } + + var ticks = new ulong[copies.Count]; + var placements = new ulong[copies.Count]; + var total = 0UL; + for (var index = 0; index < copies.Count; index++) + { + var written = copies[index].Buffer.LastGpuWriteTick; + if (written == 0) + { + return false; + } + + ticks[index] = written; + placements[index] = total; + total += (copies[index].Size + BufferDownloadBatchPlanner.Alignment - 1) & ~(BufferDownloadBatchPlanner.Alignment - 1); + } + + if (total > AsyncReadbackLimit) + { + return false; + } + + buffer = new GpuBuffer(_device, _scheduler, GpuBufferUsage.Download, 0, BufferUsageFlags.TransferDstBit, total); + for (var index = 0; index < copies.Count; index++) + { + buffer.CopyFrom( + _scheduler.Current, copies[index].Buffer, copies[index].SourceOffset, placements[index], copies[index].Size, + AccessFlags.MemoryWriteBit, AccessFlags.None, AccessFlags.MemoryReadBit | AccessFlags.MemoryWriteBit, AccessFlags.HostReadBit); + } + + tick = _scheduler.Flush(); + offsets = placements; + writeTicks = ticks; + return true; + } + + private static readonly bool MainQueuePendingReads = + Environment.GetEnvironmentVariable("SHARPEMU_MAIN_QUEUE_PENDING_READS") != "0"; + + // Pending reads a guest thread re-issues before it lets the worker read synchronously. + private const int PendingReadAttempts = 4; + private bool CompleteReadMemoryOnGpu(PendingDownload pending, bool retrySynchronously) { using var readbackScope = GuestMemoryProfile.Measure(GuestMemoryProfile.Operation.BufferReadback); + if (pending.Ticket is null) + { + return CompleteMainQueueRead(pending, retrySynchronously); + } + var readback = AsyncReadback ?? throw SubmissionScheduler.Fatal("The pending readback lost its queue."); try { @@ -1269,6 +1381,58 @@ public sealed unsafe class GuestBufferCache : IGuestBufferStore, IDisposable } } + private bool CompleteMainQueueRead(PendingDownload pending, bool retrySynchronously) + { + var buffer = pending.MainQueueBuffer ?? throw SubmissionScheduler.Fatal("The pending readback lost its buffer."); + try + { + if (!IsPendingDownloadCurrent(pending)) + { + if (retrySynchronously) + { + PendingReadbacksRetried++; + Interlocked.Increment(ref _reportedRetried); + ReadMemoryOnGpu(pending.GuestAddress, pending.Size, pending.IsWrite, pending.Source); + } + + return false; + } + + PendingReadbacksApplied++; + Interlocked.Increment(ref _reportedApplied); + var copies = pending.Copies; + buffer.Invalidate(0, buffer.Size); + for (var index = 0; index < copies.Count; index++) + { + var bytes = buffer.Mapped.Slice((int)pending.MainQueueOffsets[index], (int)copies[index].Size); + if (!_backing.TryWriteBacking(copies[index].Address, bytes)) + { + throw SubmissionScheduler.Fatal($"Could not write the required direct backing: addr=0x{copies[index].Address:X16} size=0x{copies[index].Size:X16}"); + } + + _gpuModifiedRanges.Remove(copies[index].Address, copies[index].Size); + } + + _tracker.ClearGpuDirtyPages(pending.WindowBegin, pending.WindowEnd - pending.WindowBegin); + if (pending.IsWrite) + { + _tracker.MarkCpuDirtyPages(pending.GuestAddress, pending.Size); + } + + RecordReadback(pending.WindowBegin, pending.WindowEnd, pending.IsWrite, copies, pending.Started, pending.Source); + return true; + } + finally + { + // The guest thread waited for the submission, so the GPU is done with the buffer. + buffer.Dispose(); + foreach (var copy in pending.Copies) + { + copy.Buffer.ReleaseForeignRead(); + } + } + } + private bool IsPendingDownloadCurrent(PendingDownload pending) { var total = 0UL; diff --git a/src/SharpEmu.Libs/Gpu/Scheduling/SubmissionScheduler.cs b/src/SharpEmu.Libs/Gpu/Scheduling/SubmissionScheduler.cs index 7999a4d..d7e6a8e 100644 --- a/src/SharpEmu.Libs/Gpu/Scheduling/SubmissionScheduler.cs +++ b/src/SharpEmu.Libs/Gpu/Scheduling/SubmissionScheduler.cs @@ -272,6 +272,18 @@ public sealed class SubmissionScheduler : IGpuTickScheduler, IDisposable _lastMemoryDrainTick = nextTick; } + // Waits for a tick that has already been submitted; safe from any thread, since it only + // waits on the timeline and records nothing. + public void WaitForSubmittedTick(ulong tick) + { + if (tick >= CurrentTick) + { + throw Fatal($"Cannot wait off the queue for a tick that is still recording: {tick} >= {CurrentTick}."); + } + + _timeline.Wait(tick); + } + public void Wait(ulong tick) { if (tick > CurrentTick) diff --git a/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.Device.Setup.cs b/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.Device.Setup.cs index 7f5333f..da40015 100644 --- a/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.Device.Setup.cs +++ b/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.Device.Setup.cs @@ -684,6 +684,7 @@ internal static unsafe partial class VulkanVideoPresenter } uint? compute = null; + uint? general = null; for (uint index = 0; index < count; index++) { var flags = families[index].QueueFlags; @@ -701,9 +702,16 @@ internal static unsafe partial class VulkanVideoPresenter { compute ??= index; } + + // MoltenVK exposes only general families, one queue each, every one its own + // Metal command queue: a second one still runs readbacks beside the main queue. + if ((flags & (QueueFlags.TransferBit | QueueFlags.GraphicsBit | QueueFlags.ComputeBit)) != 0) + { + general ??= index; + } } - return compute; + return compute ?? general; } private bool _supportsFragmentShaderBarycentric; diff --git a/tests/SharpEmu.Libs.Tests/Gpu/Buffers/GuestBufferAsyncReadbackTests.cs b/tests/SharpEmu.Libs.Tests/Gpu/Buffers/GuestBufferAsyncReadbackTests.cs index 015ec32..52aae00 100644 --- a/tests/SharpEmu.Libs.Tests/Gpu/Buffers/GuestBufferAsyncReadbackTests.cs +++ b/tests/SharpEmu.Libs.Tests/Gpu/Buffers/GuestBufferAsyncReadbackTests.cs @@ -48,6 +48,33 @@ public sealed class GuestBufferAsyncReadbackTests(HeadlessVulkanFixture fixture) harness.Shutdown(); } + // Without a readback queue the main queue carries the download and the guest thread waits + // for that submission; the worker records the copies and moves on. + [Fact] + public void AGuestReadWithoutAReadbackQueueStillWaitsOffTheQueueThread() + { + if (_vulkan is null || !GatePrerequisites.Ready(_vulkan)) return; + using var harness = new CacheHarness(_vulkan); + using var fatal = new FatalScope(); + Assert.Null(harness.Cache.AsyncReadback); + var address = harness.MapBacked(0x10000, ReadWrite); + GpuWrite(harness, address, 0x55667788); + Assert.True(harness.Cache.HasGpuDirtyBytes(address, 0x100)); + + Assert.True(harness.Cache.DownloadToCpu(address + 8, 4)); + + Assert.False(harness.Cache.HasGpuDirtyBytes(address, 0x100)); + Assert.Equal((1L, 0L), (harness.Cache.PendingReadbacksApplied, harness.Cache.PendingReadbacksRetried)); + Assert.Equal(0x55667788u, BitConverter.ToUInt32(harness.Read(address + 0xF0, 4))); + + // A second write after the first read is picked up by the next read, not served stale. + GpuWrite(harness, address, 0x99AABBCCu); + Assert.True(harness.Cache.DownloadToCpu(address, 4)); + Assert.Equal(0x99AABBCCu, BitConverter.ToUInt32(harness.Read(address + 0x40, 4))); + Assert.Equal((2L, 0L), (harness.Cache.PendingReadbacksApplied, harness.Cache.PendingReadbacksRetried)); + harness.Shutdown(); + } + private static void GpuWrite(CacheHarness harness, ulong address, uint value) => harness.Worker.Run(() => { var (buffer, offset) = harness.Cache.ObtainBuffer(address, 0x100, isWritten: true);