From e7cf4e6eaf528aa72e27f6ba86259c00813bc776 Mon Sep 17 00:00:00 2001 From: riperiperi Date: Tue, 24 Jan 2023 16:32:56 +0000 Subject: [PATCH] Vulkan: Reset queries on same command buffer (#4329) * Reset queries on same command buffer Vulkan seems to complain when the queries are reset on another command buffer. No idea why, the spec really could be written better in this regard. This fixes complaints, and hopefully any implementations that care extensively about them. This change _guesses_ how many queries need to be reset and resets as many as possible at the same time to avoid splitting render passes. If it resets too many queries, we didn't waste too much time - if it runs out of resets it will batch reset 10 more. The number of queries reset is the maximum number of queries in the last 3 frames. This has been worked into the AutoFlushCounter so that it only resets up to 32 if it is yet to force a command buffer submission in this attachment. This is only done for samples passed queries right now, as they have by far the most resets. * Address Feedback --- Ryujinx.Graphics.Vulkan/AutoFlushCounter.cs | 32 +++++++++++++++++++ Ryujinx.Graphics.Vulkan/PipelineBase.cs | 2 +- Ryujinx.Graphics.Vulkan/PipelineFull.cs | 30 ++++------------- .../Queries/BufferedQuery.cs | 16 ++++++---- .../Queries/CounterQueue.cs | 23 ++++++++++++- .../Queries/CounterQueueEvent.cs | 2 +- Ryujinx.Graphics.Vulkan/Queries/Counters.cs | 13 ++++++++ Ryujinx.Graphics.Vulkan/VulkanRenderer.cs | 10 ++++++ Ryujinx.Graphics.Vulkan/Window.cs | 2 ++ 9 files changed, 97 insertions(+), 33 deletions(-) diff --git a/Ryujinx.Graphics.Vulkan/AutoFlushCounter.cs b/Ryujinx.Graphics.Vulkan/AutoFlushCounter.cs index 4ba689ccb4..2a4cbd52e6 100644 --- a/Ryujinx.Graphics.Vulkan/AutoFlushCounter.cs +++ b/Ryujinx.Graphics.Vulkan/AutoFlushCounter.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics; +using System.Linq; namespace Ryujinx.Graphics.Vulkan { @@ -16,6 +17,10 @@ namespace Ryujinx.Graphics.Vulkan private bool _hasPendingQuery; private int _queryCount; + private int[] _queryCountHistory = new int[3]; + private int _queryCountHistoryIndex; + private int _remainingQueries; + public void RegisterFlush(ulong drawCount) { _lastFlush = Stopwatch.GetTimestamp(); @@ -27,6 +32,9 @@ namespace Ryujinx.Graphics.Vulkan public bool RegisterPendingQuery() { _hasPendingQuery = true; + _remainingQueries--; + + _queryCountHistory[_queryCountHistoryIndex]++; // Interrupt render passes to flush queries, so that early results arrive sooner. if (++_queryCount == InitialQueryCountForFlush) @@ -37,6 +45,21 @@ namespace Ryujinx.Graphics.Vulkan return false; } + public int GetRemainingQueries() + { + if (_remainingQueries <= 0) + { + _remainingQueries = 16; + } + + if (_queryCount < InitialQueryCountForFlush) + { + return Math.Min(InitialQueryCountForFlush - _queryCount, _remainingQueries); + } + + return _remainingQueries; + } + public bool ShouldFlushQuery() { return _hasPendingQuery; @@ -69,5 +92,14 @@ namespace Ryujinx.Graphics.Vulkan return now > _lastFlush + flushTimeout; } + + public void Present() + { + _queryCountHistoryIndex = (_queryCountHistoryIndex + 1) % 3; + + _remainingQueries = _queryCountHistory.Max() + 10; + + _queryCountHistory[_queryCountHistoryIndex] = 0; + } } } diff --git a/Ryujinx.Graphics.Vulkan/PipelineBase.cs b/Ryujinx.Graphics.Vulkan/PipelineBase.cs index 1c0c836bba..c53acfe104 100644 --- a/Ryujinx.Graphics.Vulkan/PipelineBase.cs +++ b/Ryujinx.Graphics.Vulkan/PipelineBase.cs @@ -22,7 +22,7 @@ namespace Ryujinx.Graphics.Vulkan protected readonly Device Device; public readonly PipelineCache PipelineCache; - protected readonly AutoFlushCounter AutoFlush; + public readonly AutoFlushCounter AutoFlush; protected PipelineDynamicState DynamicState; private PipelineState _newState; diff --git a/Ryujinx.Graphics.Vulkan/PipelineFull.cs b/Ryujinx.Graphics.Vulkan/PipelineFull.cs index 5b4f4a6e9f..94fd24416d 100644 --- a/Ryujinx.Graphics.Vulkan/PipelineFull.cs +++ b/Ryujinx.Graphics.Vulkan/PipelineFull.cs @@ -14,7 +14,6 @@ namespace Ryujinx.Graphics.Vulkan private CounterQueueEvent _activeConditionalRender; private readonly List _pendingQueryCopies; - private readonly List _pendingQueryResets; private ulong _byteWeight; @@ -22,7 +21,6 @@ namespace Ryujinx.Graphics.Vulkan { _activeQueries = new List(); _pendingQueryCopies = new(); - _pendingQueryResets = new List(); CommandBuffer = (Cbs = gd.CommandBufferPool.Rent()).CommandBuffer; } @@ -34,16 +32,6 @@ namespace Ryujinx.Graphics.Vulkan query.PoolCopy(Cbs); } - lock (_pendingQueryResets) - { - foreach (var query in _pendingQueryResets) - { - query.PoolReset(CommandBuffer); - } - - _pendingQueryResets.Clear(); - } - _pendingQueryCopies.Clear(); } @@ -238,10 +226,12 @@ namespace Ryujinx.Graphics.Vulkan Gd.Api.CmdBeginQuery(CommandBuffer, queryPool, 0, Gd.Capabilities.SupportsPreciseOcclusionQueries ? QueryControlFlags.PreciseBit : 0); } + Gd.ResetCounterPool(); + Restore(); } - public void BeginQuery(BufferedQuery query, QueryPool pool, bool needsReset) + public void BeginQuery(BufferedQuery query, QueryPool pool, bool needsReset, bool fromSamplePool) { if (needsReset) { @@ -249,9 +239,11 @@ namespace Ryujinx.Graphics.Vulkan Gd.Api.CmdResetQueryPool(CommandBuffer, pool, 0, 1); - lock (_pendingQueryResets) + if (fromSamplePool) { - _pendingQueryResets.Remove(query); // Might be present on here. + // Try reset some additional queries in advance. + + Gd.ResetFutureCounters(CommandBuffer, AutoFlush.GetRemainingQueries()); } } @@ -267,14 +259,6 @@ namespace Ryujinx.Graphics.Vulkan _activeQueries.Remove(pool); } - public void ResetQuery(BufferedQuery query) - { - lock (_pendingQueryResets) - { - _pendingQueryResets.Add(query); - } - } - public void CopyQueryResults(BufferedQuery query) { _pendingQueryCopies.Add(query); diff --git a/Ryujinx.Graphics.Vulkan/Queries/BufferedQuery.cs b/Ryujinx.Graphics.Vulkan/Queries/BufferedQuery.cs index 4cf258eb78..a1a5eb2794 100644 --- a/Ryujinx.Graphics.Vulkan/Queries/BufferedQuery.cs +++ b/Ryujinx.Graphics.Vulkan/Queries/BufferedQuery.cs @@ -18,7 +18,6 @@ namespace Ryujinx.Graphics.Vulkan.Queries private readonly PipelineFull _pipeline; private QueryPool _queryPool; - private bool _isReset; private readonly BufferHolder _buffer; private readonly IntPtr _bufferMap; @@ -27,6 +26,7 @@ namespace Ryujinx.Graphics.Vulkan.Queries private bool _isSupported; private long _defaultValue; + private int? _resetSequence; public unsafe BufferedQuery(VulkanRenderer gd, Device device, PipelineFull pipeline, CounterType type, bool result32Bit) { @@ -92,16 +92,17 @@ namespace Ryujinx.Graphics.Vulkan.Queries public void Reset() { End(false); - Begin(); + Begin(null); } - public void Begin() + public void Begin(int? resetSequence) { if (_isSupported) { - _pipeline.BeginQuery(this, _queryPool, !_isReset); + bool needsReset = resetSequence == null || _resetSequence == null || resetSequence.Value != _resetSequence.Value; + _pipeline.BeginQuery(this, _queryPool, needsReset, _type == CounterType.SamplesPassed && resetSequence != null); } - _isReset = false; + _resetSequence = null; } public unsafe void End(bool withResult) @@ -162,13 +163,14 @@ namespace Ryujinx.Graphics.Vulkan.Queries return data; } - public void PoolReset(CommandBuffer cmd) + public void PoolReset(CommandBuffer cmd, int resetSequence) { if (_isSupported) { _api.CmdResetQueryPool(cmd, _queryPool, 0, 1); } - _isReset = true; + + _resetSequence = resetSequence; } public void PoolCopy(CommandBufferScoped cbs) diff --git a/Ryujinx.Graphics.Vulkan/Queries/CounterQueue.cs b/Ryujinx.Graphics.Vulkan/Queries/CounterQueue.cs index 7ee3c15aec..c47f95eab8 100644 --- a/Ryujinx.Graphics.Vulkan/Queries/CounterQueue.cs +++ b/Ryujinx.Graphics.Vulkan/Queries/CounterQueue.cs @@ -3,6 +3,7 @@ using Silk.NET.Vulkan; using System; using System.Collections.Generic; using System.Threading; +using System.Linq; namespace Ryujinx.Graphics.Vulkan.Queries { @@ -32,6 +33,8 @@ namespace Ryujinx.Graphics.Vulkan.Queries private Thread _consumerThread; + public int ResetSequence { get; private set; } + internal CounterQueue(VulkanRenderer gd, Device device, PipelineFull pipeline, CounterType type) { _gd = gd; @@ -53,6 +56,24 @@ namespace Ryujinx.Graphics.Vulkan.Queries _consumerThread.Start(); } + public void ResetCounterPool() + { + ResetSequence++; + } + + public void ResetFutureCounters(CommandBuffer cmd, int count) + { + // Pre-emptively reset queries to avoid render pass splitting. + lock (_queryPool) + { + count = Math.Min(count, _queryPool.Count); + for (int i = 0; i < count; i++) + { + _queryPool.ElementAt(i).PoolReset(cmd, ResetSequence); + } + } + } + private void EventConsumer() { while (!Disposed) @@ -106,7 +127,7 @@ namespace Ryujinx.Graphics.Vulkan.Queries { lock (_lock) { - _pipeline.ResetQuery(query); + // The query will be reset when it dequeues. _queryPool.Enqueue(query); } } diff --git a/Ryujinx.Graphics.Vulkan/Queries/CounterQueueEvent.cs b/Ryujinx.Graphics.Vulkan/Queries/CounterQueueEvent.cs index 241fe1ee08..6b780ba34a 100644 --- a/Ryujinx.Graphics.Vulkan/Queries/CounterQueueEvent.cs +++ b/Ryujinx.Graphics.Vulkan/Queries/CounterQueueEvent.cs @@ -34,7 +34,7 @@ namespace Ryujinx.Graphics.Vulkan.Queries DrawIndex = drawIndex; - _counter.Begin(); + _counter.Begin(_queue.ResetSequence); } public Auto GetBuffer() diff --git a/Ryujinx.Graphics.Vulkan/Queries/Counters.cs b/Ryujinx.Graphics.Vulkan/Queries/Counters.cs index 63581e4271..7113d0601a 100644 --- a/Ryujinx.Graphics.Vulkan/Queries/Counters.cs +++ b/Ryujinx.Graphics.Vulkan/Queries/Counters.cs @@ -24,6 +24,19 @@ namespace Ryujinx.Graphics.Vulkan.Queries } } + public void ResetCounterPool() + { + foreach (var queue in _counterQueues) + { + queue.ResetCounterPool(); + } + } + + public void ResetFutureCounters(CommandBuffer cmd, int count) + { + _counterQueues[(int)CounterType.SamplesPassed].ResetFutureCounters(cmd, count); + } + public CounterQueueEvent QueueReport(CounterType type, EventHandler resultHandler, bool hostReserved) { return _counterQueues[(int)type].QueueReport(resultHandler, _pipeline.DrawCount, hostReserved); diff --git a/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs b/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs index bec7b84755..f53f794009 100644 --- a/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs +++ b/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs @@ -679,6 +679,16 @@ namespace Ryujinx.Graphics.Vulkan _counters.Update(); } + public void ResetCounterPool() + { + _counters.ResetCounterPool(); + } + + public void ResetFutureCounters(CommandBuffer cmd, int count) + { + _counters?.ResetFutureCounters(cmd, count); + } + public void BackgroundContextAction(Action action, bool alwaysBackground = false) { action(); diff --git a/Ryujinx.Graphics.Vulkan/Window.cs b/Ryujinx.Graphics.Vulkan/Window.cs index edc7d7160d..7412ad90d9 100644 --- a/Ryujinx.Graphics.Vulkan/Window.cs +++ b/Ryujinx.Graphics.Vulkan/Window.cs @@ -225,6 +225,8 @@ namespace Ryujinx.Graphics.Vulkan public unsafe override void Present(ITexture texture, ImageCrop crop, Action swapBuffersCallback) { + _gd.PipelineInternal.AutoFlush.Present(); + uint nextImage = 0; while (true)