Vulkan: Simplify MultiFenceHolder and managing them (#4845)

* Vulkan: Simplify waitable add/remove

Removal of unnecessary hashset and dictionary

* Thread safety for GetBufferData in PersistentFlushBuffer

* Fix WaitForFencesImpl thread safety

* Proper methods for risky reference increments

* Wrong type of CB.

* Address feedback

src/Ryujinx.Graphics.Vulkan/Auto.cs

@@ -105,6 +105,23 @@ namespace Ryujinx.Graphics.Vulkan
             }
         }
 
+        public bool TryIncrementReferenceCount()
+        {
+            int lastValue;
+            do
+            {
+                lastValue = _referenceCount;
+
+                if (lastValue == 0)
+                {
+                    return false;
+                }
+            }
+            while (Interlocked.CompareExchange(ref _referenceCount, lastValue + 1, lastValue) != lastValue);
+
+            return true;
+        }
+
         public void IncrementReferenceCount()
         {
             if (Interlocked.Increment(ref _referenceCount) == 1)

src/Ryujinx.Graphics.Vulkan/BufferHolder.cs

@@ -599,9 +599,10 @@ namespace Ryujinx.Graphics.Vulkan
             Auto<DisposableBuffer> dst,
             int srcOffset,
             int dstOffset,
-            int size)
+            int size,
+            bool registerSrcUsage = true)
         {
-            var srcBuffer = src.Get(cbs, srcOffset, size).Value;
+            var srcBuffer = registerSrcUsage ? src.Get(cbs, srcOffset, size).Value : src.GetUnsafe().Value;
             var dstBuffer = dst.Get(cbs, dstOffset, size).Value;
 
             InsertBufferBarrier(

src/Ryujinx.Graphics.Vulkan/CommandBufferPool.cs

@@ -31,7 +31,7 @@ namespace Ryujinx.Graphics.Vulkan
             public SemaphoreHolder Semaphore;
 
             public List<IAuto> Dependants;
-            public HashSet<MultiFenceHolder> Waitables;
+            public List<MultiFenceHolder> Waitables;
             public HashSet<SemaphoreHolder> Dependencies;
 
             public void Initialize(Vk api, Device device, CommandPool pool)
@@ -47,7 +47,7 @@ namespace Ryujinx.Graphics.Vulkan
                 api.AllocateCommandBuffers(device, allocateInfo, out CommandBuffer);
 
                 Dependants = new List<IAuto>();
-                Waitables = new HashSet<MultiFenceHolder>();
+                Waitables = new List<MultiFenceHolder>();
                 Dependencies = new HashSet<SemaphoreHolder>();
             }
         }
@@ -143,8 +143,10 @@ namespace Ryujinx.Graphics.Vulkan
         public void AddWaitable(int cbIndex, MultiFenceHolder waitable)
         {
             ref var entry = ref _commandBuffers[cbIndex];
-            waitable.AddFence(cbIndex, entry.Fence);
+            if (waitable.AddFence(cbIndex, entry.Fence))
-            entry.Waitables.Add(waitable);
+            {
+                entry.Waitables.Add(waitable);
+            }
         }
 
         public bool HasWaitableOnRentedCommandBuffer(MultiFenceHolder waitable, int offset, int size)
@@ -156,7 +158,7 @@ namespace Ryujinx.Graphics.Vulkan
                     ref var entry = ref _commandBuffers[i];
 
                     if (entry.InUse &&
-                        entry.Waitables.Contains(waitable) &&
+                        waitable.HasFence(i) &&
                         waitable.IsBufferRangeInUse(i, offset, size))
                     {
                         return true;
@@ -331,7 +333,7 @@ namespace Ryujinx.Graphics.Vulkan
 
             foreach (var waitable in entry.Waitables)
             {
-                waitable.RemoveFence(cbIndex, entry.Fence);
+                waitable.RemoveFence(cbIndex);
                 waitable.RemoveBufferUses(cbIndex);
             }
 

src/Ryujinx.Graphics.Vulkan/FenceHolder.cs

@@ -32,6 +32,25 @@ namespace Ryujinx.Graphics.Vulkan
             return _fence;
         }
 
+        public bool TryGet(out Fence fence)
+        {
+            int lastValue;
+            do
+            {
+                lastValue = _referenceCount;
+
+                if (lastValue == 0)
+                {
+                    fence = default;
+                    return false;
+                }
+            }
+            while (Interlocked.CompareExchange(ref _referenceCount, lastValue + 1, lastValue) != lastValue);
+
+            fence = _fence;
+            return true;
+        }
+
         public Fence Get()
         {
             Interlocked.Increment(ref _referenceCount);

src/Ryujinx.Graphics.Vulkan/MultiFenceHolder.cs

@@ -1,6 +1,5 @@
 using Silk.NET.Vulkan;
-using System.Collections.Generic;
+using System;
-using System.Linq;
 
 namespace Ryujinx.Graphics.Vulkan
 {
@@ -11,7 +10,7 @@ namespace Ryujinx.Graphics.Vulkan
     {
         private static int BufferUsageTrackingGranularity = 4096;
 
-        private readonly Dictionary<FenceHolder, int> _fences;
+        private readonly FenceHolder[] _fences;
         private BufferUsageBitmap _bufferUsageBitmap;
 
         /// <summary>
@@ -19,7 +18,7 @@ namespace Ryujinx.Graphics.Vulkan
         /// </summary>
         public MultiFenceHolder()
         {
-            _fences = new Dictionary<FenceHolder, int>();
+            _fences = new FenceHolder[CommandBufferPool.MaxCommandBuffers];
         }
 
         /// <summary>
@@ -28,7 +27,7 @@ namespace Ryujinx.Graphics.Vulkan
         /// <param name="size">Size of the buffer</param>
         public MultiFenceHolder(int size)
         {
-            _fences = new Dictionary<FenceHolder, int>();
+            _fences = new FenceHolder[CommandBufferPool.MaxCommandBuffers];
             _bufferUsageBitmap = new BufferUsageBitmap(size, BufferUsageTrackingGranularity);
         }
 
@@ -80,25 +79,37 @@ namespace Ryujinx.Graphics.Vulkan
         /// </summary>
         /// <param name="cbIndex">Command buffer index of the command buffer that owns the fence</param>
         /// <param name="fence">Fence to be added</param>
-        public void AddFence(int cbIndex, FenceHolder fence)
+        /// <returns>True if the command buffer's previous fence value was null</returns>
+        public bool AddFence(int cbIndex, FenceHolder fence)
         {
-            lock (_fences)
+            ref FenceHolder fenceRef = ref _fences[cbIndex];
+
+            if (fenceRef == null)
             {
-                _fences.TryAdd(fence, cbIndex);
+                fenceRef = fence;
+                return true;
             }
+
+            return false;
         }
 
         /// <summary>
         /// Removes a fence from the holder.
         /// </summary>
         /// <param name="cbIndex">Command buffer index of the command buffer that owns the fence</param>
-        /// <param name="fence">Fence to be removed</param>
+        public void RemoveFence(int cbIndex)
-        public void RemoveFence(int cbIndex, FenceHolder fence)
         {
-            lock (_fences)
+            _fences[cbIndex] = null;
-            {
+        }
-                _fences.Remove(fence);
+
-            }
+        /// <summary>
+        /// Determines if a fence referenced on the given command buffer.
+        /// </summary>
+        /// <param name="cbIndex">Index of the command buffer to check if it's used</param>
+        /// <returns>True if referenced, false otherwise</returns>
+        public bool HasFence(int cbIndex)
+        {
+            return _fences[cbIndex] != null;
         }
 
         /// <summary>
@@ -147,21 +158,29 @@ namespace Ryujinx.Graphics.Vulkan
         /// <returns>True if all fences were signaled before the timeout expired, false otherwise</returns>
         private bool WaitForFencesImpl(Vk api, Device device, int offset, int size, bool hasTimeout, ulong timeout)
         {
-            FenceHolder[] fenceHolders;
+            Span<FenceHolder> fenceHolders = new FenceHolder[CommandBufferPool.MaxCommandBuffers];
-            Fence[] fences;
 
-            lock (_fences)
+            int count = size != 0 ? GetOverlappingFences(fenceHolders, offset, size) : GetFences(fenceHolders);
+            Span<Fence> fences = stackalloc Fence[count];
+
+            int fenceCount = 0;
+
+            for (int i = 0; i < count; i++)
             {
-                fenceHolders = size != 0 ? GetOverlappingFences(offset, size) : _fences.Keys.ToArray();
+                if (fenceHolders[i].TryGet(out Fence fence))
-                fences = new Fence[fenceHolders.Length];
-
-                for (int i = 0; i < fenceHolders.Length; i++)
                 {
-                    fences[i] = fenceHolders[i].Get();
+                    fences[fenceCount] = fence;
+
+                    if (fenceCount < i)
+                    {
+                        fenceHolders[fenceCount] = fenceHolders[i];
+                    }
+
+                    fenceCount++;
                 }
             }
 
-            if (fences.Length == 0)
+            if (fenceCount == 0)
             {
                 return true;
             }
@@ -170,14 +189,14 @@ namespace Ryujinx.Graphics.Vulkan
 
             if (hasTimeout)
             {
-                signaled = FenceHelper.AllSignaled(api, device, fences, timeout);
+                signaled = FenceHelper.AllSignaled(api, device, fences.Slice(0, fenceCount), timeout);
             }
             else
             {
-                FenceHelper.WaitAllIndefinitely(api, device, fences);
+                FenceHelper.WaitAllIndefinitely(api, device, fences.Slice(0, fenceCount));
             }
 
-            for (int i = 0; i < fenceHolders.Length; i++)
+            for (int i = 0; i < fenceCount; i++)
             {
                 fenceHolders[i].Put();
             }
@@ -186,27 +205,49 @@ namespace Ryujinx.Graphics.Vulkan
         }
 
         /// <summary>
-        /// Gets fences to wait for use of a given buffer region.
+        /// Gets fences to wait for.
         /// </summary>
-        /// <param name="offset">Offset of the range</param>
+        /// <param name="storage">Span to store fences in</param>
-        /// <param name="size">Size of the range in bytes</param>
+        /// <returns>Number of fences placed in storage</returns>
-        /// <returns>Fences for the specified region</returns>
+        private int GetFences(Span<FenceHolder> storage)
-        private FenceHolder[] GetOverlappingFences(int offset, int size)
         {
-            List<FenceHolder> overlapping = new List<FenceHolder>();
+            int count = 0;
 
-            foreach (var kv in _fences)
+            for (int i = 0; i < _fences.Length; i++)
             {
-                var fence = kv.Key;
+                var fence = _fences[i];
-                var ownerCbIndex = kv.Value;
 
-                if (_bufferUsageBitmap.OverlapsWith(ownerCbIndex, offset, size))
+                if (fence != null)
                 {
-                    overlapping.Add(fence);
+                    storage[count++] = fence;
                 }
             }
 
-            return overlapping.ToArray();
+            return count;
+        }
+
+        /// <summary>
+        /// Gets fences to wait for use of a given buffer region.
+        /// </summary>
+        /// <param name="storage">Span to store overlapping fences in</param>
+        /// <param name="offset">Offset of the range</param>
+        /// <param name="size">Size of the range in bytes</param>
+        /// <returns>Number of fences for the specified region placed in storage</returns>
+        private int GetOverlappingFences(Span<FenceHolder> storage, int offset, int size)
+        {
+            int count = 0;
+
+            for (int i = 0; i < _fences.Length; i++)
+            {
+                var fence = _fences[i];
+
+                if (fence != null && _bufferUsageBitmap.OverlapsWith(i, offset, size))
+                {
+                    storage[count++] = fence;
+                }
+            }
+
+            return count;
         }
     }
 }

src/Ryujinx.Graphics.Vulkan/PersistentFlushBuffer.cs

@@ -34,16 +34,26 @@ namespace Ryujinx.Graphics.Vulkan
         public Span<byte> GetBufferData(CommandBufferPool cbp, BufferHolder buffer, int offset, int size)
         {
             var flushStorage = ResizeIfNeeded(size);
+            Auto<DisposableBuffer> srcBuffer;
 
             using (var cbs = cbp.Rent())
             {
-                var srcBuffer = buffer.GetBuffer(cbs.CommandBuffer);
+                srcBuffer = buffer.GetBuffer(cbs.CommandBuffer);
                 var dstBuffer = flushStorage.GetBuffer(cbs.CommandBuffer);
 
-                BufferHolder.Copy(_gd, cbs, srcBuffer, dstBuffer, offset, 0, size);
+                if (srcBuffer.TryIncrementReferenceCount())
+                {
+                    BufferHolder.Copy(_gd, cbs, srcBuffer, dstBuffer, offset, 0, size, registerSrcUsage: false);
+                }
+                else
+                {
+                    // Source buffer is no longer alive, don't copy anything to flush storage.
+                    srcBuffer = null;
+                }
             }
 
             flushStorage.WaitForFences();
+            srcBuffer?.DecrementReferenceCount();
             return flushStorage.GetDataStorage(0, size);
         }
 

src/Ryujinx.Graphics.Vulkan/PipelineBase.cs

@@ -66,6 +66,8 @@ namespace Ryujinx.Graphics.Vulkan
         private ulong _vertexBuffersDirty;
         protected Rectangle<int> ClearScissor;
 
+        private readonly VertexBufferUpdater _vertexBufferUpdater;
+
         public SupportBufferUpdater SupportBufferUpdater;
         public IndexBufferPattern QuadsToTrisPattern;
         public IndexBufferPattern TriFanToTrisPattern;
@@ -96,6 +98,7 @@ namespace Ryujinx.Graphics.Vulkan
             gd.Api.CreatePipelineCache(device, pipelineCacheCreateInfo, null, out PipelineCache).ThrowOnError();
 
             _descriptorSetUpdater = new DescriptorSetUpdater(gd, this);
+            _vertexBufferUpdater = new VertexBufferUpdater(gd);
 
             _transformFeedbackBuffers = new BufferState[Constants.MaxTransformFeedbackBuffers];
             _vertexBuffers = new VertexBufferState[Constants.MaxVertexBuffers + 1];
@@ -1185,6 +1188,9 @@ namespace Ryujinx.Graphics.Vulkan
 
             int validCount = 1;
 
+            BufferHandle lastHandle = default;
+            Auto<DisposableBuffer> lastBuffer = default;
+
             for (int i = 0; i < count; i++)
             {
                 var vertexBuffer = vertexBuffers[i];
@@ -1194,7 +1200,12 @@ namespace Ryujinx.Graphics.Vulkan
 
                 if (vertexBuffer.Buffer.Handle != BufferHandle.Null)
                 {
-                    var vb = Gd.BufferManager.GetBuffer(CommandBuffer, vertexBuffer.Buffer.Handle, false);
+                    Auto<DisposableBuffer> vb = (vertexBuffer.Buffer.Handle == lastHandle) ? lastBuffer :
+                        Gd.BufferManager.GetBuffer(CommandBuffer, vertexBuffer.Buffer.Handle, false);
+
+                    lastHandle = vertexBuffer.Buffer.Handle;
+                    lastBuffer = vb;
+
                     if (vb != null)
                     {
                         int binding = i + 1;
@@ -1222,24 +1233,29 @@ namespace Ryujinx.Graphics.Vulkan
                         ref var buffer = ref _vertexBuffers[binding];
                         int oldScalarAlign = buffer.AttributeScalarAlignment;
 
-                        buffer.Dispose();
-
                         if (Gd.Capabilities.VertexBufferAlignment < 2 &&
                             (vertexBuffer.Stride % FormatExtensions.MaxBufferFormatScalarSize) == 0)
                         {
-                            buffer = new VertexBufferState(
+                            if (!buffer.Matches(vb, descriptorIndex, vertexBuffer.Buffer.Offset, vbSize, vertexBuffer.Stride))
-                                vb,
+                            {
-                                descriptorIndex,
+                                buffer.Dispose();
-                                vertexBuffer.Buffer.Offset,
-                                vbSize,
-                                vertexBuffer.Stride);
 
-                            buffer.BindVertexBuffer(Gd, Cbs, (uint)binding, ref _newState);
+                                buffer = new VertexBufferState(
+                                    vb,
+                                    descriptorIndex,
+                                    vertexBuffer.Buffer.Offset,
+                                    vbSize,
+                                    vertexBuffer.Stride);
+
+                                buffer.BindVertexBuffer(Gd, Cbs, (uint)binding, ref _newState, _vertexBufferUpdater);
+                            }
                         }
                         else
                         {
                             // May need to be rewritten. Bind this buffer before draw.
 
+                            buffer.Dispose();
+
                             buffer = new VertexBufferState(
                                 vertexBuffer.Buffer.Handle,
                                 descriptorIndex,
@@ -1255,6 +1271,8 @@ namespace Ryujinx.Graphics.Vulkan
                 }
             }
 
+            _vertexBufferUpdater.Commit(Cbs);
+
             _newState.VertexBindingDescriptionsCount = (uint)validCount;
             SignalStateChange();
         }
@@ -1596,10 +1614,12 @@ namespace Ryujinx.Graphics.Vulkan
                 {
                     int i = BitOperations.TrailingZeroCount(_vertexBuffersDirty);
 
-                    _vertexBuffers[i].BindVertexBuffer(Gd, Cbs, (uint)i, ref _newState);
+                    _vertexBuffers[i].BindVertexBuffer(Gd, Cbs, (uint)i, ref _newState, _vertexBufferUpdater);
 
                     _vertexBuffersDirty &= ~(1UL << i);
                 }
+
+                _vertexBufferUpdater.Commit(Cbs);
             }
 
             if (_stateDirty || Pbp != pbp)
@@ -1712,6 +1732,7 @@ namespace Ryujinx.Graphics.Vulkan
                 _framebuffer?.Dispose();
                 _newState.Dispose();
                 _descriptorSetUpdater.Dispose();
+                _vertexBufferUpdater.Dispose();
 
                 for (int i = 0; i < _vertexBuffers.Length; i++)
                 {

src/Ryujinx.Graphics.Vulkan/VertexBufferState.cs

@@ -46,7 +46,7 @@ namespace Ryujinx.Graphics.Vulkan
             AttributeScalarAlignment = 1;
         }
 
-        public void BindVertexBuffer(VulkanRenderer gd, CommandBufferScoped cbs, uint binding, ref PipelineState state)
+        public void BindVertexBuffer(VulkanRenderer gd, CommandBufferScoped cbs, uint binding, ref PipelineState state, VertexBufferUpdater updater)
         {
             var autoBuffer = _buffer;
 
@@ -65,21 +65,7 @@ namespace Ryujinx.Graphics.Vulkan
 
                         var buffer = autoBuffer.Get(cbs, 0, newSize).Value;
 
-                        if (gd.Capabilities.SupportsExtendedDynamicState)
+                        updater.BindVertexBuffer(cbs, binding, buffer, 0, (ulong)newSize, (ulong)stride);
-                        {
-                            gd.ExtendedDynamicStateApi.CmdBindVertexBuffers2(
-                                cbs.CommandBuffer,
-                                binding,
-                                1,
-                                buffer,
-                                0,
-                                (ulong)newSize,
-                                (ulong)stride);
-                        }
-                        else
-                        {
-                            gd.Api.CmdBindVertexBuffers(cbs.CommandBuffer, binding, 1, buffer, 0);
-                        }
 
                         _buffer = autoBuffer;
 
@@ -106,21 +92,7 @@ namespace Ryujinx.Graphics.Vulkan
             {
                 var buffer = autoBuffer.Get(cbs, _offset, _size).Value;
 
-                if (gd.Capabilities.SupportsExtendedDynamicState)
+                updater.BindVertexBuffer(cbs, binding, buffer, (ulong)_offset, (ulong)_size, (ulong)_stride);
-                {
-                    gd.ExtendedDynamicStateApi.CmdBindVertexBuffers2(
-                        cbs.CommandBuffer,
-                        binding,
-                        1,
-                        buffer,
-                        (ulong)_offset,
-                        (ulong)_size,
-                        (ulong)_stride);
-                }
-                else
-                {
-                    gd.Api.CmdBindVertexBuffers(cbs.CommandBuffer, binding, 1, buffer, (ulong)_offset);
-                }
             }
         }
 
@@ -129,6 +101,11 @@ namespace Ryujinx.Graphics.Vulkan
             return _buffer == buffer;
         }
 
+        public bool Matches(Auto<DisposableBuffer> buffer, int descriptorIndex, int offset, int size, int stride = 0)
+        {
+            return _buffer == buffer && DescriptorIndex == descriptorIndex && _offset == offset && _size == size && _stride == stride;
+        }
+
         public void Swap(Auto<DisposableBuffer> from, Auto<DisposableBuffer> to)
         {
             if (_buffer == from)

src/Ryujinx.Graphics.Vulkan/VertexBufferUpdater.cs

@@ -0,0 +1,84 @@
+using Silk.NET.Vulkan;
+using System;
+
+using VkBuffer = Silk.NET.Vulkan.Buffer;
+
+namespace Ryujinx.Graphics.Vulkan
+{
+    internal class VertexBufferUpdater : IDisposable
+    {
+        private VulkanRenderer _gd;
+
+        private uint _baseBinding;
+        private uint _count;
+
+        private NativeArray<VkBuffer> _buffers;
+        private NativeArray<ulong> _offsets;
+        private NativeArray<ulong> _sizes;
+        private NativeArray<ulong> _strides;
+
+        public VertexBufferUpdater(VulkanRenderer gd)
+        {
+            _gd = gd;
+
+            _buffers = new NativeArray<VkBuffer>(Constants.MaxVertexBuffers);
+            _offsets = new NativeArray<ulong>(Constants.MaxVertexBuffers);
+            _sizes = new NativeArray<ulong>(Constants.MaxVertexBuffers);
+            _strides = new NativeArray<ulong>(Constants.MaxVertexBuffers);
+        }
+
+        public void BindVertexBuffer(CommandBufferScoped cbs, uint binding, VkBuffer buffer, ulong offset, ulong size, ulong stride)
+        {
+            if (_count == 0)
+            {
+                _baseBinding = binding;
+            }
+            else if (_baseBinding + _count != binding)
+            {
+                Commit(cbs);
+                _baseBinding = binding;
+            }
+
+            int index = (int)_count;
+
+            _buffers[index] = buffer;
+            _offsets[index] = offset;
+            _sizes[index] = size;
+            _strides[index] = stride;
+
+            _count++;
+        }
+
+        public unsafe void Commit(CommandBufferScoped cbs)
+        {
+            if (_count != 0)
+            {
+                if (_gd.Capabilities.SupportsExtendedDynamicState)
+                {
+                    _gd.ExtendedDynamicStateApi.CmdBindVertexBuffers2(
+                        cbs.CommandBuffer,
+                        _baseBinding,
+                        _count,
+                        _buffers.Pointer,
+                        _offsets.Pointer,
+                        _sizes.Pointer,
+                        _strides.Pointer);
+                }
+                else
+                {
+                    _gd.Api.CmdBindVertexBuffers(cbs.CommandBuffer, _baseBinding, _count, _buffers.Pointer, _offsets.Pointer);
+                }
+
+                _count = 0;
+            }
+        }
+
+        public void Dispose()
+        {
+            _buffers.Dispose();
+            _offsets.Dispose();
+            _sizes.Dispose();
+            _strides.Dispose();
+        }
+    }
+}