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

ARMeilleure/CodeGen/CompiledFunction.cs

@@ -48,9 +48,21 @@ namespace ARMeilleure.CodeGen
         /// <returns>A delegate of type <typeparamref name="T"/> pointing to the mapped function</returns>
         public T Map<T>()
         {
-            IntPtr codePtr = JitCache.Map(this);
+            return MapWithPointer<T>(out _);
+        }
 
-            return Marshal.GetDelegateForFunctionPointer<T>(codePtr);
+        /// <summary>
+        /// Maps the <see cref="CompiledFunction"/> onto the <see cref="JitCache"/> and returns a delegate of type
+        /// <typeparamref name="T"/> pointing to the mapped function.
+        /// </summary>
+        /// <typeparam name="T">Type of delegate</typeparam>
+        /// <param name="codePointer">Pointer to the function code in memory</param>
+        /// <returns>A delegate of type <typeparamref name="T"/> pointing to the mapped function</returns>
+        public T MapWithPointer<T>(out IntPtr codePointer)
+        {
+            codePointer = JitCache.Map(this);
+
+            return Marshal.GetDelegateForFunctionPointer<T>(codePointer);
         }
     }
 }

ARMeilleure/Instructions/NativeInterface.cs

@@ -191,7 +191,7 @@ namespace ARMeilleure.Instructions
         {
             TranslatedFunction function = Context.Translator.GetOrTranslate(address, GetContext().ExecutionMode);
 
-            return (ulong)function.FuncPtr.ToInt64();
+            return (ulong)function.FuncPointer.ToInt64();
         }
 
         public static void InvalidateCacheLine(ulong address)

ARMeilleure/Translation/PTC/Ptc.cs

@@ -745,9 +745,9 @@ namespace ARMeilleure.Translation.PTC
             bool highCq)
         {
             var cFunc = new CompiledFunction(code, unwindInfo, RelocInfo.Empty);
-            var gFunc = cFunc.Map<GuestFunction>();
+            var gFunc = cFunc.MapWithPointer<GuestFunction>(out IntPtr gFuncPointer);
 
-            return new TranslatedFunction(gFunc, callCounter, guestSize, highCq);
+            return new TranslatedFunction(gFunc, gFuncPointer, callCounter, guestSize, highCq);
         }
 
         private void UpdateInfo(InfoEntry infoEntry)

ARMeilleure/Translation/TranslatedFunction.cs

@@ -1,6 +1,5 @@
 using ARMeilleure.Common;
 using System;
-using System.Runtime.InteropServices;
 
 namespace ARMeilleure.Translation
 {
@@ -8,18 +7,18 @@ namespace ARMeilleure.Translation
     {
         private readonly GuestFunction _func; // Ensure that this delegate will not be garbage collected.
 
+        public IntPtr FuncPointer { get; }
         public Counter<uint> CallCounter { get; }
         public ulong GuestSize { get; }
         public bool HighCq { get; }
-        public IntPtr FuncPtr { get; }
 
-        public TranslatedFunction(GuestFunction func, Counter<uint> callCounter, ulong guestSize, bool highCq)
+        public TranslatedFunction(GuestFunction func, IntPtr funcPointer, Counter<uint> callCounter, ulong guestSize, bool highCq)
         {
             _func = func;
+            FuncPointer = funcPointer;
             CallCounter = callCounter;
             GuestSize = guestSize;
             HighCq = highCq;
-            FuncPtr = Marshal.GetFunctionPointerForDelegate(func);
         }
 
         public ulong Execute(State.ExecutionContext context)

ARMeilleure/Translation/Translator.cs

@@ -211,7 +211,7 @@ namespace ARMeilleure.Translation
 
                 if (oldFunc != func)
                 {
-                    JitCache.Unmap(func.FuncPtr);
+                    JitCache.Unmap(func.FuncPointer);
                     func = oldFunc;
                 }
 
@@ -230,7 +230,7 @@ namespace ARMeilleure.Translation
         {
             if (FunctionTable.IsValid(guestAddress) && (Optimizations.AllowLcqInFunctionTable || func.HighCq))
             {
-                Volatile.Write(ref FunctionTable.GetValue(guestAddress), (ulong)func.FuncPtr);
+                Volatile.Write(ref FunctionTable.GetValue(guestAddress), (ulong)func.FuncPointer);
             }
         }
 
@@ -292,11 +292,11 @@ namespace ARMeilleure.Translation
                 _ptc.WriteCompiledFunction(address, funcSize, hash, highCq, compiledFunc);
             }
 
-            GuestFunction func = compiledFunc.Map<GuestFunction>();
+            GuestFunction func = compiledFunc.MapWithPointer<GuestFunction>(out IntPtr funcPointer);
 
             Allocators.ResetAll();
 
-            return new TranslatedFunction(func, counter, funcSize, highCq);
+            return new TranslatedFunction(func, funcPointer, counter, funcSize, highCq);
         }
 
         private void BackgroundTranslate()
@@ -537,7 +537,7 @@ namespace ARMeilleure.Translation
 
             foreach (var func in functions)
             {
-                JitCache.Unmap(func.FuncPtr);
+                JitCache.Unmap(func.FuncPointer);
 
                 func.CallCounter?.Dispose();
             }
@@ -546,7 +546,7 @@ namespace ARMeilleure.Translation
 
             while (_oldFuncs.TryDequeue(out var kv))
             {
-                JitCache.Unmap(kv.Value.FuncPtr);
+                JitCache.Unmap(kv.Value.FuncPointer);
 
                 kv.Value.CallCounter?.Dispose();
             }

Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs

@@ -22,7 +22,7 @@ namespace Ryujinx.Graphics.Gpu.Shader.DiskCache
         private const ushort FileFormatVersionMajor = 1;
         private const ushort FileFormatVersionMinor = 2;
         private const uint FileFormatVersionPacked = ((uint)FileFormatVersionMajor << 16) | FileFormatVersionMinor;
-        private const uint CodeGenVersion = 4318;
+        private const uint CodeGenVersion = 4336;
 
         private const string SharedTocFileName = "shared.toc";
         private const string SharedDataFileName = "shared.data";

Ryujinx.Graphics.Shader/CodeGen/Spirv/Instructions.cs

@@ -261,17 +261,17 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv
 
         private static OperationResult GenerateBitfieldExtractS32(CodeGenContext context, AstOperation operation)
         {
-            return GenerateTernaryS32(context, operation, context.Delegates.BitFieldSExtract);
+            return GenerateBitfieldExtractS32(context, operation, context.Delegates.BitFieldSExtract);
         }
 
         private static OperationResult GenerateBitfieldExtractU32(CodeGenContext context, AstOperation operation)
         {
-            return GenerateTernaryS32(context, operation, context.Delegates.BitFieldUExtract);
+            return GenerateTernaryU32(context, operation, context.Delegates.BitFieldUExtract);
         }
 
         private static OperationResult GenerateBitfieldInsert(CodeGenContext context, AstOperation operation)
         {
-            return GenerateQuaternaryS32(context, operation, context.Delegates.BitFieldInsert);
+            return GenerateBitfieldInsert(context, operation, context.Delegates.BitFieldInsert);
         }
 
         private static OperationResult GenerateBitfieldReverse(CodeGenContext context, AstOperation operation)
@@ -2290,22 +2290,6 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv
             }
         }
 
-        private static OperationResult GenerateTernaryS32(
-            CodeGenContext context,
-            AstOperation operation,
-            Func<SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction> emitS)
-        {
-            var src1 = operation.GetSource(0);
-            var src2 = operation.GetSource(1);
-            var src3 = operation.GetSource(2);
-
-            return new OperationResult(AggregateType.S32, emitS(
-                context.TypeS32(),
-                context.GetS32(src1),
-                context.GetS32(src2),
-                context.GetS32(src3)));
-        }
-
         private static OperationResult GenerateTernaryU32(
             CodeGenContext context,
             AstOperation operation,
@@ -2322,7 +2306,23 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv
                 context.GetU32(src3)));
         }
 
-        private static OperationResult GenerateQuaternaryS32(
+        private static OperationResult GenerateBitfieldExtractS32(
+            CodeGenContext context,
+            AstOperation operation,
+            Func<SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction> emitS)
+        {
+            var src1 = operation.GetSource(0);
+            var src2 = operation.GetSource(1);
+            var src3 = operation.GetSource(2);
+
+            return new OperationResult(AggregateType.S32, emitS(
+                context.TypeS32(),
+                context.GetS32(src1),
+                context.GetU32(src2),
+                context.GetU32(src3)));
+        }
+
+        private static OperationResult GenerateBitfieldInsert(
             CodeGenContext context,
             AstOperation operation,
             Func<SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction, SpvInstruction> emitS)
@@ -2332,12 +2332,12 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv
             var src3 = operation.GetSource(2);
             var src4 = operation.GetSource(3);
 
-            return new OperationResult(AggregateType.S32, emitS(
-                context.TypeS32(),
-                context.GetS32(src1),
-                context.GetS32(src2),
-                context.GetS32(src3),
-                context.GetS32(src4)));
+            return new OperationResult(AggregateType.U32, emitS(
+                context.TypeU32(),
+                context.GetU32(src1),
+                context.GetU32(src2),
+                context.GetU32(src3),
+                context.GetU32(src4)));
         }
     }
 }

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;
+        }
     }
 }

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;

Ryujinx.Graphics.Vulkan/PipelineFull.cs

@@ -14,7 +14,6 @@ namespace Ryujinx.Graphics.Vulkan
         private CounterQueueEvent _activeConditionalRender;
 
         private readonly List<BufferedQuery> _pendingQueryCopies;
-        private readonly List<BufferedQuery> _pendingQueryResets;
 
         private ulong _byteWeight;
 
@@ -22,7 +21,6 @@ namespace Ryujinx.Graphics.Vulkan
         {
             _activeQueries = new List<QueryPool>();
             _pendingQueryCopies = new();
-            _pendingQueryResets = new List<BufferedQuery>();
 
             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);

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)

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);
             }
         }

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<DisposableBuffer> GetBuffer()

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<ulong> resultHandler, bool hostReserved)
         {
             return _counterQueues[(int)type].QueueReport(resultHandler, _pipeline.DrawCount, hostReserved);

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();

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)