From 302d0f830c903c60b0859cecc9cb304c365ca987 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Fri, 3 Jul 2020 19:22:06 -0300 Subject: [PATCH] Call syncpoint expiration callback outside of the lock (#1349) --- .../Synchronization/Syncpoint.cs | 45 +++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/Ryujinx.Graphics.Gpu/Synchronization/Syncpoint.cs b/Ryujinx.Graphics.Gpu/Synchronization/Syncpoint.cs index abd86b35..30f8cc86 100644 --- a/Ryujinx.Graphics.Gpu/Synchronization/Syncpoint.cs +++ b/Ryujinx.Graphics.Gpu/Synchronization/Syncpoint.cs @@ -13,16 +13,13 @@ namespace Ryujinx.Graphics.Gpu.Synchronization public readonly uint Id; - // TODO: get rid of this lock - private object _listLock = new object(); - /// /// The value of the syncpoint. /// public uint Value => (uint)_storedValue; // TODO: switch to something handling concurrency? - private List _waiters; + private readonly List _waiters; public Syncpoint(uint id) { @@ -39,7 +36,7 @@ namespace Ryujinx.Graphics.Gpu.Synchronization /// The created SyncpointWaiterHandle object or null if already past threshold public SyncpointWaiterHandle RegisterCallback(uint threshold, Action callback) { - lock (_listLock) + lock (_waiters) { if (Value >= threshold) { @@ -64,7 +61,7 @@ namespace Ryujinx.Graphics.Gpu.Synchronization public void UnregisterCallback(SyncpointWaiterHandle waiterInformation) { - lock (_listLock) + lock (_waiters) { _waiters.Remove(waiterInformation); } @@ -78,7 +75,10 @@ namespace Ryujinx.Graphics.Gpu.Synchronization { uint currentValue = (uint)Interlocked.Increment(ref _storedValue); - lock (_listLock) + SyncpointWaiterHandle expired = null; + List expiredList = null; + + lock (_waiters) { _waiters.RemoveAll(item => { @@ -86,13 +86,42 @@ namespace Ryujinx.Graphics.Gpu.Synchronization if (isPastThreshold) { - item.Callback(); + if (expired == null) + { + expired = item; + } + else + { + if (expiredList == null) + { + expiredList = new List(); + } + + expiredList.Add(item); + } } return isPastThreshold; }); } + // Call the callbacks as a separate step. + // As we don't know what the callback will be doing, + // and it could block execution for a indefinite amount of time, + // we can't call it inside the lock. + if (expired != null) + { + expired.Callback(); + + if (expiredList != null) + { + for (int i = 0; i < expiredList.Count; i++) + { + expiredList[i].Callback(); + } + } + } + return currentValue; } }