Back to synchronized version, since CAS approach demonstrates high contention

Author: sakno

src/DotNext.Threading/Threading/AsyncExchanger.cs

@@ -83,17 +83,20 @@ private ExchangePoint RentExchangePoint(T value)
 
     private void OnCompleted(ExchangePoint point)
     {
-        Monitor.Enter(SyncRoot);
-        if (ReferenceEquals(this.point, point))
+        lock (SyncRoot)
         {
-            this.point = null;
+            if (ReferenceEquals(this.point, point))
+            {
+                this.point = null;
+            }
         }
 
-        Monitor.Exit(SyncRoot);
-
         if (point.TryReset(out _))
         {
-            pool.Return(point);
+            lock (SyncRoot)
+            {
+                pool.Return(point);
+            }
         }
     }
 

src/DotNext.Threading/Threading/QueuedSynchronizer.Queue.cs

@@ -51,7 +51,10 @@ private void ReturnNode(WaitNode node)
         // the node is removed for sure, it can be returned back to the pool
         if (node.TryReset(out _) && !IsDisposingOrDisposed)
         {
-            pool.Return(node);
+            lock (SyncRoot)
+            {
+                pool.Return(node);
+            }
         }
     }
 

src/DotNext.Threading/Threading/Tasks/Pooling/ValueTaskPool.cs

@@ -1,4 +1,3 @@
-using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using Debug = System.Diagnostics.Debug;
 
@@ -7,13 +6,13 @@ namespace DotNext.Threading.Tasks.Pooling;
 /*
  * Represents a pool without any allocations. Assuming that wait queues are organized using
  * linked nodes where each node is a completion source. If so, the node pointers (next/previous)
- * can be used to keep completion sources in the pool. The implementation is thread-safe.
+ * can be used to keep completion sources in the pool. The access must be synchronized.
  */
 [StructLayout(LayoutKind.Auto)]
 internal struct ValueTaskPool<T>(long maximumRetained)
 {
-    private volatile LinkedValueTaskCompletionSource<T>? first;
-    private long count; // volatile
+    private LinkedValueTaskCompletionSource<T>? first;
+    private long count;
 
     public ValueTaskPool()
         : this(long.MaxValue)
@@ -27,54 +26,33 @@ public void Return(LinkedValueTaskCompletionSource<T> node)
         Debug.Assert(node is { Status: ManualResetCompletionSourceStatus.WaitForActivation });
         Debug.Assert(node is { Next: null, Previous: null });
 
-        // try to increment the counter
-        for (long current = Atomic.Read(in count), tmp; current < maximumRetained; current = tmp)
+        if (count < maximumRetained)
         {
-            tmp = Interlocked.CompareExchange(ref count, current + 1L, current);
-            if (tmp == current)
-            {
-                ReturnCore(node);
-                break;
-            }
-        }
-    }
-
-    private void ReturnCore(LinkedValueTaskCompletionSource<T> node)
-    {
-        for (LinkedValueTaskCompletionSource<T>? current = first, tmp;; current = tmp)
-        {
-            node.Next = current;
-
-            tmp = Interlocked.CompareExchange(ref first, node, current);
-            if (ReferenceEquals(tmp, current))
-                break;
+            node.Next = first;
+            first = node;
+            count++;
         }
     }
 
     public TNode Rent<TNode>()
         where TNode : LinkedValueTaskCompletionSource<T>, new()
     {
-        var current = first;
-        for (LinkedValueTaskCompletionSource<T>? tmp;; current = Unsafe.As<TNode>(tmp))
+        if (first is TNode result)
         {
-            if (current is null)
-            {
-                current = new TNode();
-                break;
-            }
+            first = result.Next;
+            result.Next = null;
+            count--;
 
-            tmp = Interlocked.CompareExchange(ref first, current.Next, current);
-            if (!ReferenceEquals(tmp, current))
-                continue;
+            Debug.Assert(count >= 0L);
+            Debug.Assert(count is 0L || first is not null);
+        }
+        else
+        {
+            Debug.Assert(count is 0L);
 
-            current.Next = null;
-            var actualCount = Interlocked.Decrement(ref count);
-            Debug.Assert(actualCount >= 0L);
-            break;
+            result = new();
         }
 
-        Debug.Assert(current is TNode);
-        Debug.Assert(current is { Next: null, Previous: null });
-        return Unsafe.As<TNode>(current);
+        return result;
     }
 }

src/DotNext.Threading/Threading/Tasks/TaskCompletionPipe.cs

@@ -56,7 +56,10 @@ private void OnCompleted(Signal signal)
 
         if (signal.TryReset(out _))
         {
-            pool.Return(signal);
+            lock (SyncRoot)
+            {
+                pool.Return(signal);
+            }
         }
     }