Reimplement TripleBuffer as a true flipping buffer

osu.Framework/Allocation/ObjectUsage.cs

@@ -10,10 +10,7 @@ public class ObjectUsage<T> : IDisposable
     {
         public T? Object;
 
-        /// <summary>
-        /// Whether this usage is actively being written to or read from.
-        /// </summary>
-        public UsageType Usage;
+        public UsageType LastUsage;
 
         public readonly int Index;
 
@@ -33,7 +30,6 @@ public void Dispose()
 
     public enum UsageType
     {
-        None,
         Read,
         Write
     }

osu.Framework/Allocation/TripleBuffer.cs

@@ -1,9 +1,7 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 
 namespace osu.Framework.Allocation
@@ -16,28 +14,12 @@ namespace osu.Framework.Allocation
     public class TripleBuffer<T>
         where T : class
     {
+        private const int buffer_count = 3;
         private readonly ObjectUsage<T>[] buffers = new ObjectUsage<T>[buffer_count];
 
-        /// <summary>
-        /// The freshest buffer index which has finished a write, and is waiting to be read.
-        /// Will be set to <c>null</c> after being read once.
-        /// </summary>
-        private int pendingCompletedWriteIndex = -1;
-
-        /// <summary>
-        /// The last buffer index which was obtained for writing.
-        /// </summary>
-        private int lastWriteIndex = -1;
-
-        /// <summary>
-        /// The last buffer index which was obtained for reading.
-        /// Note that this will remain "active" even after a <see cref="GetForRead"/> ends, to give benefit of doubt that the usage may still be accessing it.
-        /// </summary>
-        private int lastReadIndex = -1;
-
-        private readonly ManualResetEventSlim writeCompletedEvent = new ManualResetEventSlim();
-
-        private const int buffer_count = 3;
+        private int frontIndex;
+        private int flipIndex = 1;
+        private int backIndex = 2;
 
         public TripleBuffer()
         {
@@ -47,97 +29,41 @@ public TripleBuffer()
 
         public ObjectUsage<T> GetForWrite()
         {
-            // Only one write should be allowed at once
-            Debug.Assert(buffers.All(b => b.Usage != UsageType.Write));
-
-            ObjectUsage<T> buffer = getNextWriteBuffer();
-
-            return buffer;
+            ObjectUsage<T> usage = buffers[frontIndex];
+            usage.LastUsage = UsageType.Write;
+            return usage;
         }
 
         public ObjectUsage<T>? GetForRead()
         {
-            // Only one read should be allowed at once
-            Debug.Assert(buffers.All(b => b.Usage != UsageType.Read));
-
-            writeCompletedEvent.Reset();
-
-            var buffer = getPendingReadBuffer();
-
-            if (buffer != null)
-                return buffer;
-
-            // A completed write wasn't available, so wait for the next to complete.
-            if (!writeCompletedEvent.Wait(100))
-                // Generally shouldn't happen, but this avoids spinning forever.
-                return null;
-
-            return GetForRead();
-        }
+            Stopwatch sw = Stopwatch.StartNew();
 
-        private ObjectUsage<T>? getPendingReadBuffer()
-        {
-            // Avoid locking to see if there's a pending write.
-            int pendingWrite = Interlocked.Exchange(ref pendingCompletedWriteIndex, -1);
+            do
+            {
+                flip(ref backIndex);
 
-            if (pendingWrite == -1)
-                return null;
+                // This should really never happen, but prevents a potential infinite loop if the usage can never be retrieved.
+                if (sw.ElapsedMilliseconds > 100)
+                    return null;
+            } while (buffers[backIndex].LastUsage == UsageType.Read);
 
-            lock (buffers)
-            {
-                var buffer = buffers[pendingWrite];
+            ObjectUsage<T> usage = buffers[backIndex];
 
-                Debug.Assert(lastReadIndex != buffer.Index);
-                lastReadIndex = buffer.Index;
+            Debug.Assert(usage.LastUsage == UsageType.Write);
+            usage.LastUsage = UsageType.Read;
 
-                Debug.Assert(buffer.Usage == UsageType.None);
-                buffer.Usage = UsageType.Read;
-                return buffer;
-            }
+            return usage;
         }
 
-        private ObjectUsage<T> getNextWriteBuffer()
+        private void finishUsage(ObjectUsage<T> usage)
         {
-            lock (buffers)
-            {
-                for (int i = 0; i < buffer_count; i++)
-                {
-                    // Never write to the last read index.
-                    // We assume there could be some reads still occurring even after the usage is finished.
-                    if (i == lastReadIndex) continue;
-
-                    // Never write to the same buffer twice in a row.
-                    // This would defeat the purpose of having a triple buffer.
-                    if (i == lastWriteIndex) continue;
-
-                    lastWriteIndex = i;
-
-                    var buffer = buffers[i];
-
-                    Debug.Assert(buffer.Usage == UsageType.None);
-                    buffer.Usage = UsageType.Write;
-
-                    return buffer;
-                }
-            }
-
-            throw new InvalidOperationException("No buffer could be obtained. This should never ever happen.");
+            if (usage.LastUsage == UsageType.Write)
+                flip(ref frontIndex);
         }
 
-        private void finishUsage(ObjectUsage<T> obj)
+        private void flip(ref int localIndex)
         {
-            // This implementation is intentionally written this way to avoid requiring locking overhead.
-            bool wasWrite = obj.Usage == UsageType.Write;
-
-            obj.Usage = UsageType.None;
-
-            if (wasWrite)
-            {
-                Debug.Assert(pendingCompletedWriteIndex != obj.Index);
-                Interlocked.Exchange(ref pendingCompletedWriteIndex, obj.Index);
-
-                writeCompletedEvent.Set();
-            }
+            localIndex = Interlocked.Exchange(ref flipIndex, localIndex);
         }
     }
 }