From 9adc9848eaa65a5dcff949d9b4e2fdccdc3dab28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:41:16 +0300 Subject: [PATCH 1/4] Remove unused StructMemoryPool --- Source.Common/Exts.cs | 55 ------------------------------------------- 1 file changed, 55 deletions(-) diff --git a/Source.Common/Exts.cs b/Source.Common/Exts.cs index 09814c212..99d199b09 100644 --- a/Source.Common/Exts.cs +++ b/Source.Common/Exts.cs @@ -282,61 +282,6 @@ void Grow(int num = 0) { } } -public class StructMemoryPool where T : struct -{ - public static readonly StructMemoryPool Shared = new(); - - readonly RefStack instances = new(); - readonly ConcurrentDictionary valueStates = []; - - public ref T Alloc() { - foreach (var kvp in valueStates) { - ref T existing = ref instances[kvp.Key]; - if (kvp.Value == false) { - valueStates[kvp.Key] = true; - return ref existing; - } - } - - lock (instances) { - ref T instance = ref instances.Push(); - valueStates[instances.Count - 1] = true; - return ref instance; - } - } - - public unsafe bool IsMemoryPoolAllocated(ref T value) { - lock (instances) { - for (int i = 0; i < instances.Count; i++) { - ref T instance = ref instances[i]; - if (Unsafe.AreSame(ref value, ref instance)) - return true; - } - } - - return false; - } - - - public void Free(ref T value) { - lock (instances) { - for (int i = 0; i < instances.Count; i++) { - ref T instance = ref instances[i]; - if (Unsafe.AreSame(ref value, ref instance)) { - if (!valueStates.TryGetValue(i, out bool state)) - AssertMsg(false, $"Passed an instance of {typeof(T).Name} to {nameof(Free)}(T value) that was not allocated by {nameof(Alloc)}()"); - else if (state == false) - AssertMsg(false, $"Attempted to free {typeof(T).Name} instance twice in StructPool, please verify\n"); - else { - valueStates[i] = false; - instance = default; // Zero out the instance - } - } - } - } - } -} - public static class StrTools { // We're going to use / everywhere instead of \ on Windows, since C#'s API From 8383024171da9bec11f8d88d5de61799ff174a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:47:16 +0300 Subject: [PATCH 2/4] Move ObjectPool into its own file --- Source.Common/Exts.cs | 47 ---------------------------------- Source.Common/ObjectPool.cs | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 47 deletions(-) create mode 100644 Source.Common/ObjectPool.cs diff --git a/Source.Common/Exts.cs b/Source.Common/Exts.cs index 99d199b09..8d9d9b9e9 100644 --- a/Source.Common/Exts.cs +++ b/Source.Common/Exts.cs @@ -136,53 +136,6 @@ public void Purge() { public V this[ulong index] => dict[index]; } -public class ObjectPool where T : IPoolableObject, new() -{ - public static readonly ObjectPool Shared = new(); - - - readonly ConcurrentDictionary valueStates = []; - - public T Alloc() { - foreach (var kvp in valueStates) { - if (kvp.Value == false) { // We found something free - valueStates[kvp.Key] = true; - kvp.Key.Init(); - return kvp.Key; - } - } - - // Make an new instance of the class - var instance = new T(); - valueStates[instance] = true; - instance.Init(); - return instance; - } - - public int Count() { - int count = 0; - foreach (KeyValuePair kvp in valueStates) - if (kvp.Value) - count++; - - return count; - } - - public bool IsMemoryPoolAllocated(T value) => valueStates.TryGetValue(value, out _); - public void Free(T value) { - if (value == null) - return; - if (!valueStates.TryGetValue(value, out bool state)) - AssertMsg(false, $"Passed an instance of {typeof(T).Name} to {nameof(Free)}(T value) that was not allocated by {nameof(Alloc)}()"); - else if (state == false) - AssertMsg(false, $"Attempted to free {typeof(T).Name} instance twice in ClassPool, please verify\n"); - else { - value.Reset(); - valueStates[value] = false; - } - } -} - public sealed class PooledLinkedList where T : struct { struct Node diff --git a/Source.Common/ObjectPool.cs b/Source.Common/ObjectPool.cs new file mode 100644 index 000000000..94c07a110 --- /dev/null +++ b/Source.Common/ObjectPool.cs @@ -0,0 +1,50 @@ +using System.Collections.Concurrent; + +namespace Source; + +public class ObjectPool where T : IPoolableObject, new() +{ + public static readonly ObjectPool Shared = new(); + + + readonly ConcurrentDictionary valueStates = []; + + public T Alloc() { + foreach (var kvp in valueStates) { + if (kvp.Value == false) { // We found something free + valueStates[kvp.Key] = true; + kvp.Key.Init(); + return kvp.Key; + } + } + + // Make an new instance of the class + var instance = new T(); + valueStates[instance] = true; + instance.Init(); + return instance; + } + + public int Count() { + int count = 0; + foreach (KeyValuePair kvp in valueStates) + if (kvp.Value) + count++; + + return count; + } + + public bool IsMemoryPoolAllocated(T value) => valueStates.TryGetValue(value, out _); + public void Free(T value) { + if (value == null) + return; + if (!valueStates.TryGetValue(value, out bool state)) + AssertMsg(false, $"Passed an instance of {typeof(T).Name} to {nameof(Free)}(T value) that was not allocated by {nameof(Alloc)}()"); + else if (state == false) + AssertMsg(false, $"Attempted to free {typeof(T).Name} instance twice in ClassPool, please verify\n"); + else { + value.Reset(); + valueStates[value] = false; + } + } +} From 9dcdf8da32943fbf41d1d15c1c03e5bf4a08143d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:47:20 +0300 Subject: [PATCH 3/4] Refactor how the ObjectPool works --- Source.Common/ObjectPool.cs | 43 ++++++++++-------------------------- Source.Engine/ClientFrame.cs | 5 +---- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/Source.Common/ObjectPool.cs b/Source.Common/ObjectPool.cs index 94c07a110..f63e7356b 100644 --- a/Source.Common/ObjectPool.cs +++ b/Source.Common/ObjectPool.cs @@ -6,45 +6,26 @@ namespace Source; { public static readonly ObjectPool Shared = new(); - - readonly ConcurrentDictionary valueStates = []; + readonly ConcurrentBag free = []; + int allocated; public T Alloc() { - foreach (var kvp in valueStates) { - if (kvp.Value == false) { // We found something free - valueStates[kvp.Key] = true; - kvp.Key.Init(); - return kvp.Key; - } - } - - // Make an new instance of the class - var instance = new T(); - valueStates[instance] = true; + if (!free.TryTake(out T? instance)) + instance = new T(); + + Interlocked.Increment(ref allocated); instance.Init(); return instance; } - public int Count() { - int count = 0; - foreach (KeyValuePair kvp in valueStates) - if (kvp.Value) - count++; + public int Count() => Volatile.Read(ref allocated); - return count; - } - - public bool IsMemoryPoolAllocated(T value) => valueStates.TryGetValue(value, out _); - public void Free(T value) { + public void Free(T? value) { if (value == null) return; - if (!valueStates.TryGetValue(value, out bool state)) - AssertMsg(false, $"Passed an instance of {typeof(T).Name} to {nameof(Free)}(T value) that was not allocated by {nameof(Alloc)}()"); - else if (state == false) - AssertMsg(false, $"Attempted to free {typeof(T).Name} instance twice in ClassPool, please verify\n"); - else { - value.Reset(); - valueStates[value] = false; - } + + value.Reset(); + Interlocked.Decrement(ref allocated); + free.Add(value); } } diff --git a/Source.Engine/ClientFrame.cs b/Source.Engine/ClientFrame.cs index d484bd73c..947888379 100644 --- a/Source.Engine/ClientFrame.cs +++ b/Source.Engine/ClientFrame.cs @@ -191,8 +191,5 @@ public void DeleteClientFrames(int tick) { public ClientFrame AllocateFrame() => ClientFramePool.Alloc(); - public void FreeFrame(ClientFrame frame) { - if (ClientFramePool.IsMemoryPoolAllocated(frame)) - ClientFramePool.Free(frame); - } + public void FreeFrame(ClientFrame frame) => ClientFramePool.Free(frame); } From d4bc06cb252e44d8ce56b3e24e0a514fa56bd7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:41:16 +0300 Subject: [PATCH 4/4] Add ObjectPool tests --- Source.Common.Tests/ObjectPoolTests.cs | 101 +++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Source.Common.Tests/ObjectPoolTests.cs diff --git a/Source.Common.Tests/ObjectPoolTests.cs b/Source.Common.Tests/ObjectPoolTests.cs new file mode 100644 index 000000000..3377586a3 --- /dev/null +++ b/Source.Common.Tests/ObjectPoolTests.cs @@ -0,0 +1,101 @@ +namespace Source.Common.Tests; + +public class ObjectPoolTests +{ + sealed class TestPoolable : IPoolableObject + { + public int InitCalls; + public int ResetCalls; + public int InUse; + public bool HandedOutTwice; + + public void Init() { + InitCalls++; + if (Interlocked.Exchange(ref InUse, 1) != 0) + HandedOutTwice = true; + } + + public void Reset() { + ResetCalls++; + Interlocked.Exchange(ref InUse, 0); + } + } + + [Fact] + public void AllocInitsAndFreeResets() { + ObjectPool pool = new(); + + TestPoolable obj = pool.Alloc(); + Assert.Equal(1, obj.InitCalls); + Assert.Equal(0, obj.ResetCalls); + + pool.Free(obj); + Assert.Equal(1, obj.ResetCalls); + } + + [Fact] + public void FreedInstanceIsReused() { + ObjectPool pool = new(); + + TestPoolable first = pool.Alloc(); + pool.Free(first); + TestPoolable second = pool.Alloc(); + + Assert.Same(first, second); + Assert.Equal(2, second.InitCalls); + } + + [Fact] + public void LiveInstancesAreDistinct() { + ObjectPool pool = new(); + + TestPoolable a = pool.Alloc(); + TestPoolable b = pool.Alloc(); + + Assert.NotSame(a, b); + } + + [Fact] + public void CountTracksOutstandingInstances() { + ObjectPool pool = new(); + + TestPoolable a = pool.Alloc(); + TestPoolable b = pool.Alloc(); + Assert.Equal(2, pool.Count()); + + pool.Free(a); + Assert.Equal(1, pool.Count()); + + pool.Free(b); + Assert.Equal(0, pool.Count()); + } + + [Fact] + public void FreeNullIsIgnored() { + ObjectPool pool = new(); + + pool.Free(null); + + Assert.Equal(0, pool.Count()); + } + + [Fact] + public void ConcurrentUseNeverHandsOutAnInstanceTwice() { + ObjectPool pool = new(); + const int threads = 8; + const int iterations = 20000; + bool handedOutTwice = false; + + Parallel.For(0, threads, _ => { + for (int i = 0; i < iterations; i++) { + TestPoolable obj = pool.Alloc(); + if (obj.HandedOutTwice) + Volatile.Write(ref handedOutTwice, true); + pool.Free(obj); + } + }); + + Assert.False(handedOutTwice); + Assert.Equal(0, pool.Count()); + } +}