From e768a54f17b390c3ac10904c7909e3bef020edbd Mon Sep 17 00:00:00 2001 From: sunshineinabox Date: Thu, 12 Oct 2023 09:11:15 -0700 Subject: [PATCH] Replace ReaderWriterLock with ReaderWriterLockSlim (#5785) * Replace ReaderWriterLock with ReaderWriterLockSlim * Resolve Feedback + Correct typo * Revert some unncessary logic --- .../Translation/TranslatorCache.cs | 36 +++++----- src/Ryujinx.Common/ReactiveObject.cs | 10 +-- src/Ryujinx.Graphics.Vulkan/BufferHolder.cs | 68 ++++++++++--------- 3 files changed, 58 insertions(+), 56 deletions(-) diff --git a/src/ARMeilleure/Translation/TranslatorCache.cs b/src/ARMeilleure/Translation/TranslatorCache.cs index 11286381b..99ca58dc6 100644 --- a/src/ARMeilleure/Translation/TranslatorCache.cs +++ b/src/ARMeilleure/Translation/TranslatorCache.cs @@ -7,14 +7,14 @@ namespace ARMeilleure.Translation internal class TranslatorCache { private readonly IntervalTree _tree; - private readonly ReaderWriterLock _treeLock; + private readonly ReaderWriterLockSlim _treeLock; public int Count => _tree.Count; public TranslatorCache() { _tree = new IntervalTree(); - _treeLock = new ReaderWriterLock(); + _treeLock = new ReaderWriterLockSlim(); } public bool TryAdd(ulong address, ulong size, T value) @@ -24,70 +24,70 @@ namespace ARMeilleure.Translation public bool AddOrUpdate(ulong address, ulong size, T value, Func updateFactoryCallback) { - _treeLock.AcquireWriterLock(Timeout.Infinite); + _treeLock.EnterWriteLock(); bool result = _tree.AddOrUpdate(address, address + size, value, updateFactoryCallback); - _treeLock.ReleaseWriterLock(); + _treeLock.ExitWriteLock(); return result; } public T GetOrAdd(ulong address, ulong size, T value) { - _treeLock.AcquireWriterLock(Timeout.Infinite); + _treeLock.EnterWriteLock(); value = _tree.GetOrAdd(address, address + size, value); - _treeLock.ReleaseWriterLock(); + _treeLock.ExitWriteLock(); return value; } public bool Remove(ulong address) { - _treeLock.AcquireWriterLock(Timeout.Infinite); + _treeLock.EnterWriteLock(); bool removed = _tree.Remove(address) != 0; - _treeLock.ReleaseWriterLock(); + _treeLock.ExitWriteLock(); return removed; } public void Clear() { - _treeLock.AcquireWriterLock(Timeout.Infinite); + _treeLock.EnterWriteLock(); _tree.Clear(); - _treeLock.ReleaseWriterLock(); + _treeLock.ExitWriteLock(); } public bool ContainsKey(ulong address) { - _treeLock.AcquireReaderLock(Timeout.Infinite); + _treeLock.EnterReadLock(); bool result = _tree.ContainsKey(address); - _treeLock.ReleaseReaderLock(); + _treeLock.ExitReadLock(); return result; } public bool TryGetValue(ulong address, out T value) { - _treeLock.AcquireReaderLock(Timeout.Infinite); + _treeLock.EnterReadLock(); bool result = _tree.TryGet(address, out value); - _treeLock.ReleaseReaderLock(); + _treeLock.ExitReadLock(); return result; } public int GetOverlaps(ulong address, ulong size, ref ulong[] overlaps) { - _treeLock.AcquireReaderLock(Timeout.Infinite); + _treeLock.EnterReadLock(); int count = _tree.Get(address, address + size, ref overlaps); - _treeLock.ReleaseReaderLock(); + _treeLock.ExitReadLock(); return count; } public List AsList() { - _treeLock.AcquireReaderLock(Timeout.Infinite); + _treeLock.EnterReadLock(); List list = _tree.AsList(); - _treeLock.ReleaseReaderLock(); + _treeLock.ExitReadLock(); return list; } diff --git a/src/Ryujinx.Common/ReactiveObject.cs b/src/Ryujinx.Common/ReactiveObject.cs index d2624c365..ac7d2c4d8 100644 --- a/src/Ryujinx.Common/ReactiveObject.cs +++ b/src/Ryujinx.Common/ReactiveObject.cs @@ -5,7 +5,7 @@ namespace Ryujinx.Common { public class ReactiveObject { - private readonly ReaderWriterLock _readerWriterLock = new(); + private readonly ReaderWriterLockSlim _readerWriterLock = new(); private bool _isInitialized; private T _value; @@ -15,15 +15,15 @@ namespace Ryujinx.Common { get { - _readerWriterLock.AcquireReaderLock(Timeout.Infinite); + _readerWriterLock.EnterReadLock(); T value = _value; - _readerWriterLock.ReleaseReaderLock(); + _readerWriterLock.ExitReadLock(); return value; } set { - _readerWriterLock.AcquireWriterLock(Timeout.Infinite); + _readerWriterLock.EnterWriteLock(); T oldValue = _value; @@ -32,7 +32,7 @@ namespace Ryujinx.Common _isInitialized = true; _value = value; - _readerWriterLock.ReleaseWriterLock(); + _readerWriterLock.ExitWriteLock(); if (!oldIsInitialized || oldValue == null || !oldValue.Equals(_value)) { diff --git a/src/Ryujinx.Graphics.Vulkan/BufferHolder.cs b/src/Ryujinx.Graphics.Vulkan/BufferHolder.cs index a93ced0e5..d3a3cae11 100644 --- a/src/Ryujinx.Graphics.Vulkan/BufferHolder.cs +++ b/src/Ryujinx.Graphics.Vulkan/BufferHolder.cs @@ -58,7 +58,7 @@ namespace Ryujinx.Graphics.Vulkan private int _flushTemp; private int _lastFlushWrite = -1; - private readonly ReaderWriterLock _flushLock; + private readonly ReaderWriterLockSlim _flushLock; private FenceHolder _flushFence; private int _flushWaiting; @@ -85,7 +85,7 @@ namespace Ryujinx.Graphics.Vulkan _currentType = currentType; DesiredType = currentType; - _flushLock = new ReaderWriterLock(); + _flushLock = new ReaderWriterLockSlim(); _useMirrors = gd.IsTBDR; } @@ -106,7 +106,7 @@ namespace Ryujinx.Graphics.Vulkan _currentType = currentType; DesiredType = currentType; - _flushLock = new ReaderWriterLock(); + _flushLock = new ReaderWriterLockSlim(); } public bool TryBackingSwap(ref CommandBufferScoped? cbs) @@ -116,7 +116,7 @@ namespace Ryujinx.Graphics.Vulkan // Only swap if the buffer is not used in any queued command buffer. bool isRented = _buffer.HasRentedCommandBufferDependency(_gd.CommandBufferPool); - if (!isRented && _gd.CommandBufferPool.OwnedByCurrentThread && !_flushLock.IsReaderLockHeld && (_pendingData == null || cbs != null)) + if (!isRented && _gd.CommandBufferPool.OwnedByCurrentThread && !_flushLock.IsReadLockHeld && (_pendingData == null || cbs != null)) { var currentAllocation = _allocationAuto; var currentBuffer = _buffer; @@ -131,7 +131,7 @@ namespace Ryujinx.Graphics.Vulkan ClearMirrors(cbs.Value, 0, Size); } - _flushLock.AcquireWriterLock(Timeout.Infinite); + _flushLock.EnterWriteLock(); ClearFlushFence(); @@ -185,7 +185,7 @@ namespace Ryujinx.Graphics.Vulkan _gd.PipelineInternal.SwapBuffer(currentBuffer, _buffer); - _flushLock.ReleaseWriterLock(); + _flushLock.ExitWriteLock(); } _swapQueued = false; @@ -548,42 +548,44 @@ namespace Ryujinx.Graphics.Vulkan private void WaitForFlushFence() { - // Assumes the _flushLock is held as reader, returns in same state. + if (_flushFence == null) + { + return; + } + + // If storage has changed, make sure the fence has been reached so that the data is in place. + _flushLock.ExitReadLock(); + _flushLock.EnterWriteLock(); if (_flushFence != null) { - // If storage has changed, make sure the fence has been reached so that the data is in place. + var fence = _flushFence; + Interlocked.Increment(ref _flushWaiting); - var cookie = _flushLock.UpgradeToWriterLock(Timeout.Infinite); + // Don't wait in the lock. - if (_flushFence != null) + _flushLock.ExitWriteLock(); + + fence.Wait(); + + _flushLock.EnterWriteLock(); + + if (Interlocked.Decrement(ref _flushWaiting) == 0) { - var fence = _flushFence; - Interlocked.Increment(ref _flushWaiting); - - // Don't wait in the lock. - - var restoreCookie = _flushLock.ReleaseLock(); - - fence.Wait(); - - _flushLock.RestoreLock(ref restoreCookie); - - if (Interlocked.Decrement(ref _flushWaiting) == 0) - { - fence.Put(); - } - - _flushFence = null; + fence.Put(); } - _flushLock.DowngradeFromWriterLock(ref cookie); + _flushFence = null; } + + // Assumes the _flushLock is held as reader, returns in same state. + _flushLock.ExitWriteLock(); + _flushLock.EnterReadLock(); } public PinnedSpan GetData(int offset, int size) { - _flushLock.AcquireReaderLock(Timeout.Infinite); + _flushLock.EnterReadLock(); WaitForFlushFence(); @@ -603,7 +605,7 @@ namespace Ryujinx.Graphics.Vulkan // Need to be careful here, the buffer can't be unmapped while the data is being used. _buffer.IncrementReferenceCount(); - _flushLock.ReleaseReaderLock(); + _flushLock.ExitReadLock(); return PinnedSpan.UnsafeFromSpan(result, _buffer.DecrementReferenceCount); } @@ -621,7 +623,7 @@ namespace Ryujinx.Graphics.Vulkan result = resource.GetFlushBuffer().GetBufferData(resource.GetPool(), this, offset, size); } - _flushLock.ReleaseReaderLock(); + _flushLock.ExitReadLock(); // Flush buffer is pinned until the next GetBufferData on the thread, which is fine for current uses. return PinnedSpan.UnsafeFromSpan(result); @@ -1073,11 +1075,11 @@ namespace Ryujinx.Graphics.Vulkan _allocationAuto.Dispose(); } - _flushLock.AcquireWriterLock(Timeout.Infinite); + _flushLock.EnterWriteLock(); ClearFlushFence(); - _flushLock.ReleaseWriterLock(); + _flushLock.ExitWriteLock(); } } }