From 9df52b4439ee89929fa6b466e84ab18b3b8eeaef Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 15:34:39 +0100 Subject: [PATCH 1/7] Add a fast path for PairHashSet.ConcurrentAdd --- src/Jitter2/Collision/PairHashSet.cs | 69 +++++++++++++--------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/src/Jitter2/Collision/PairHashSet.cs b/src/Jitter2/Collision/PairHashSet.cs index 918516e9..bbb758cf 100644 --- a/src/Jitter2/Collision/PairHashSet.cs +++ b/src/Jitter2/Collision/PairHashSet.cs @@ -31,14 +31,9 @@ namespace Jitter2.Collision; /// -/// A hash set implementation optimized for thread-safe additions of (int,int)-pairs. +/// A hash set implementation which stores pairs of (int, int) values. +/// The implementation is based on open addressing. /// -/// -/// - The method is thread-safe and can be called concurrently -/// from multiple threads without additional synchronization. -/// - Other operations, such as or enumeration, are NOT thread-safe -/// and require external synchronization if used concurrently. -/// public unsafe class PairHashSet : IEnumerable { [StructLayout(LayoutKind.Explicit, Size = 8)] @@ -106,8 +101,8 @@ public void Reset() } } - public volatile Pair[] Slots = Array.Empty(); - private volatile int count; + public Pair[] Slots = Array.Empty(); + private int count; // 16384*8/1024 KB = 128 KB public const int MinimumSize = 16384; @@ -150,12 +145,11 @@ private void Resize(int size) if (pair.ID != 0) { int hash = pair.GetHash(); - int hash_i = FindSlot(newSlots, hash, pair.ID); - newSlots[hash_i] = pair; + int hashIndex = FindSlot(newSlots, hash, pair.ID); + newSlots[hashIndex] = pair; } } - Interlocked.MemoryBarrier(); Slots = newSlots; } @@ -175,11 +169,11 @@ private int FindSlot(Pair[] slots, int hash, long id) public bool Add(Pair pair) { int hash = pair.GetHash(); - int hash_i = FindSlot(Slots, hash, pair.ID); + int hashIndex = FindSlot(Slots, hash, pair.ID); - if (Slots[hash_i].ID == 0) + if (Slots[hashIndex].ID == 0) { - Slots[hash_i] = pair; + Slots[hashIndex] = pair; Interlocked.Increment(ref count); if (Slots.Length < 2 * count) @@ -195,49 +189,52 @@ public bool Add(Pair pair) private Jitter2.Parallelization.ReaderWriterLock rwLock; - internal void ConcurrentAdd(Pair pair) + internal bool ConcurrentAdd(Pair pair) { - // TODO: implement a better lock-free version - int hash = pair.GetHash(); + // Fast path: This is a *huge* optimization for the case of frequent additions + // of already existing entries. Entirely bypassing any locks or synchronization. + int fpHashIndex = FindSlot(Slots, hash, pair.ID); + if (Slots[fpHashIndex].ID != 0) return false; + rwLock.EnterReadLock(); fixed (Pair* slotsPtr = Slots) { while (true) { - int hash_i = FindSlot(Slots, hash, pair.ID); - - Pair* slotPtr = &slotsPtr[hash_i]; + var hashIndex = FindSlot(Slots, hash, pair.ID); + var slotPtr = &slotsPtr[hashIndex]; if (slotPtr->ID == pair.ID) { rwLock.ExitReadLock(); - return; + return false; } - if (Interlocked.CompareExchange(ref *(long*)slotPtr, - *(long*)&pair, 0) == 0) + if (Interlocked.CompareExchange(ref *(long*)slotPtr, pair.ID, 0) != 0) { - Interlocked.Increment(ref count); + continue; + } - rwLock.ExitReadLock(); + Interlocked.Increment(ref count); + rwLock.ExitReadLock(); + + if (Slots.Length < 2 * count) + { + rwLock.EnterWriteLock(); + // check if another thread already performed a resize. if (Slots.Length < 2 * count) { - rwLock.EnterWriteLock(); - // check if another thread already did the work for us - if (Slots.Length < 2 * count) - { - Resize(PickSize(Slots.Length * 2)); - } - rwLock.ExitWriteLock(); + Resize(PickSize(Slots.Length * 2)); } - return; + rwLock.ExitWriteLock(); } + return true; } // while } // fixed } @@ -287,8 +284,8 @@ public bool Remove(int slot) public bool Remove(Pair pair) { int hash = pair.GetHash(); - int hash_i = FindSlot(Slots, hash, pair.ID); - return Remove(hash_i); + int hashIndex = FindSlot(Slots, hash, pair.ID); + return Remove(hashIndex); } public IEnumerator GetEnumerator() From 756119a6121ab7afb169309eb47589573c2e19eb Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 16:04:42 +0100 Subject: [PATCH 2/7] SlimBag.ConcurrentAdd naive implementation --- src/Jitter2/DataStructures/SlimBag.cs | 39 +++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Jitter2/DataStructures/SlimBag.cs b/src/Jitter2/DataStructures/SlimBag.cs index 50d74485..dd087a5f 100644 --- a/src/Jitter2/DataStructures/SlimBag.cs +++ b/src/Jitter2/DataStructures/SlimBag.cs @@ -24,6 +24,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Threading; namespace Jitter2.DataStructures; @@ -89,6 +90,40 @@ public void Add(T item) nullOut = counter; } + private Jitter2.Parallelization.ReaderWriterLock rwLock; + + /// + /// Adds an element to the . + /// + /// The element to add. + public void ConcurrentAdd(T item) + { + int lc = Interlocked.Increment(ref counter) - 1; + + again: + + rwLock.EnterReadLock(); + + if (lc < array.Length) + { + array[lc] = item; + rwLock.ExitReadLock(); + } + else + { + rwLock.ExitReadLock(); + + rwLock.EnterWriteLock(); + if (lc >= array.Length) + { + Array.Resize(ref array, array.Length * 2); + } + rwLock.ExitWriteLock(); + + goto again; + } + } + /// /// Removes the first occurrence of a specific element from the . /// @@ -160,7 +195,7 @@ public void NullOut() /// public void NullOutOne() { - if (nullOut <= counter) return; - array[--nullOut] = default!; + //if (nullOut <= counter) return; + //array[--nullOut] = default!; } } \ No newline at end of file From abdf6969c54ab367514f6fb31fa7975e5b2a9cf8 Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 16:51:27 +0100 Subject: [PATCH 3/7] Use SlimBag.ConcurrentAdd in DynamicTree --- src/Jitter2/Collision/DynamicTree.cs | 69 +++++-------------- .../Parallelization/ParallelExtensions.cs | 11 +++ 2 files changed, 29 insertions(+), 51 deletions(-) diff --git a/src/Jitter2/Collision/DynamicTree.cs b/src/Jitter2/Collision/DynamicTree.cs index c00b2909..4bc068d2 100644 --- a/src/Jitter2/Collision/DynamicTree.cs +++ b/src/Jitter2/Collision/DynamicTree.cs @@ -74,7 +74,7 @@ public interface IRayCastable /// public partial class DynamicTree { - private volatile SlimBag[] lists = Array.Empty>(); + private SlimBag lists = new(); private readonly ActiveList proxies = new(); @@ -134,7 +134,6 @@ public struct Node /// public int Root => root; - public Func Filter { get; set; } /// @@ -144,7 +143,7 @@ public struct Node public DynamicTree(Func filter) { scanMoved = ScanForMovedProxies; - scanOverlaps = batch => { ScanForOverlaps(batch.BatchIndex); }; + scanOverlaps = ScanForOverlaps; updateBoundingBoxes = UpdateBoundingBoxesCallback; Filter = filter; @@ -161,12 +160,10 @@ public enum Timings public readonly double[] DebugTimings = new double[(int)Timings.Last]; - private int updatedProxies; - /// /// Gets the number of updated proxies. /// - public int UpdatedProxies => updatedProxies; + public int UpdatedProxies => lists.Count; /// /// Updates all entities that are marked as active in the active list. @@ -187,8 +184,6 @@ void SetTime(Timings type) this.step_dt = dt; - CheckBagCount(multiThread); - SetTime(Timings.UpdateBoundingBoxes); if (multiThread) @@ -196,29 +191,18 @@ void SetTime(Timings type) proxies.ParallelForBatch(256, updateBoundingBoxes); SetTime(Timings.UpdateBoundingBoxes); - const int taskThreshold = 24; - int numTasks = Math.Clamp(proxies.ActiveCount / taskThreshold, 1, ThreadPool.Instance.ThreadCount); - Parallel.ForBatch(0, proxies.ActiveCount, numTasks, scanMoved); - + lists.Clear(); + proxies.ParallelForBatch(24, scanMoved); SetTime(Timings.ScanMoved); - updatedProxies = 0; - - for (int ntask = 0; ntask < numTasks; ntask++) + for (int i = 0; i < lists.Count; i++) { - var sl = lists[ntask]; - updatedProxies += sl.Count; - - for (int i = 0; i < sl.Count; i++) - { - var proxy = sl[i]; - InternalAddRemoveProxy(proxy); - } + InternalAddRemoveProxy(lists[i]); } SetTime(Timings.UpdateProxies); - Parallel.ForBatch(0, proxies.ActiveCount, numTasks, scanOverlaps); + lists.ParallelForBatch(24, scanOverlaps); SetTime(Timings.ScanOverlaps); } @@ -228,22 +212,23 @@ void SetTime(Timings type) UpdateBoundingBoxesCallback(batch); SetTime(Timings.UpdateBoundingBoxes); + lists.Clear(); scanMoved(batch); SetTime(Timings.ScanMoved); - var sl = lists[0]; - for (int i = 0; i < sl.Count; i++) + for (int i = 0; i < lists.Count; i++) { - IDynamicTreeProxy proxy = sl[i]; - InternalAddRemoveProxy(proxy); + InternalAddRemoveProxy(lists[i]); } SetTime(Timings.UpdateProxies); - scanOverlaps(new Parallel.Batch(0, proxies.ActiveCount)); + scanOverlaps(new Parallel.Batch(0, lists.Count)); SetTime(Timings.ScanOverlaps); } + + lists.NullOutOne(); } private Real step_dt; @@ -522,19 +507,6 @@ private void FreeNode(int node) freeNodes.Push(node); } - private void CheckBagCount(bool multiThread) - { - int numThreads = multiThread ? ThreadPool.Instance.ThreadCount : 1; - if (lists.Length != numThreads) - { - lists = new SlimBag[numThreads]; - for (int i = 0; i < numThreads; i++) - { - lists[i] = new SlimBag(); - } - } - } - private double Cost(ref Node node) { if (node.IsLeaf) @@ -590,9 +562,6 @@ private void OverlapCheckRemove(int index, int node) private void ScanForMovedProxies(Parallel.Batch batch) { - var list = lists[batch.BatchIndex]; - list.Clear(); - for (int i = batch.Start; i < batch.End; i++) { var proxy = proxies[i]; @@ -602,7 +571,7 @@ private void ScanForMovedProxies(Parallel.Batch batch) if (node.ForceUpdate || !node.ExpandedBox.Encompasses(proxy.WorldBoundingBox)) { node.ForceUpdate = false; - list.Add(proxy); + lists.ConcurrentAdd(proxy); } // else proxy is well contained within the nodes expanded Box: @@ -611,15 +580,13 @@ private void ScanForMovedProxies(Parallel.Batch batch) // Make sure we do not hold too many dangling references // in the internal array of the SlimBag data structure which might // prevent GC. But do only free them one-by-one to prevent overhead. - list.NullOutOne(); } - private void ScanForOverlaps(int fraction) + private void ScanForOverlaps(Parallel.Batch batch) { - var sl = lists[fraction]; - for (int i = 0; i < sl.Count; i++) + for (int i = batch.Start; i < batch.End; i++) { - OverlapCheckAdd(root, sl[i].NodePtr); + OverlapCheckAdd(root, lists[i].NodePtr); } } diff --git a/src/Jitter2/Parallelization/ParallelExtensions.cs b/src/Jitter2/Parallelization/ParallelExtensions.cs index b23bec15..e8492e0d 100644 --- a/src/Jitter2/Parallelization/ParallelExtensions.cs +++ b/src/Jitter2/Parallelization/ParallelExtensions.cs @@ -104,4 +104,15 @@ public static int ParallelForBatch(this ActiveList list, int taskThreshold return numTasks; } + + internal static int ParallelForBatch(this SlimBag list, int taskThreshold, + Action action, bool execute = true) where T : class, IListIndex + { + int numTasks = list.Count / taskThreshold + 1; + numTasks = Math.Min(numTasks, ThreadPool.Instance.ThreadCount); + + Parallel.ForBatch(0, list.Count, numTasks, action, execute); + + return numTasks; + } } \ No newline at end of file From 1a113bb3fadd92a2b9c26f1192f0d3f2dcd1295d Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 17:04:53 +0100 Subject: [PATCH 4/7] Use SlimBag.ConcurrentAdd in UpdateContactsCallback --- src/Jitter2/World.Step.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Jitter2/World.Step.cs b/src/Jitter2/World.Step.cs index 1c0fee5d..c8799ce2 100644 --- a/src/Jitter2/World.Step.cs +++ b/src/Jitter2/World.Step.cs @@ -492,10 +492,7 @@ private void UpdateContactsCallback(Parallel.Batch batch) if ((cq.UsageMask & ContactData.MaskContactAll) == 0) { var h = memContacts.GetHandle(ref cq); - lock (brokenArbiters) - { - brokenArbiters.Add(h); - } + brokenArbiters.ConcurrentAdd(h); } } } From c9c869502d9628810ce7767f7dd3fa8f45a97c4b Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 17:41:29 +0100 Subject: [PATCH 5/7] Refine NullOut in SlimBag.cs --- src/Jitter2/Collision/DynamicTree.cs | 2 +- src/Jitter2/DataStructures/SlimBag.cs | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/Jitter2/Collision/DynamicTree.cs b/src/Jitter2/Collision/DynamicTree.cs index 4bc068d2..384b28a1 100644 --- a/src/Jitter2/Collision/DynamicTree.cs +++ b/src/Jitter2/Collision/DynamicTree.cs @@ -228,7 +228,7 @@ void SetTime(Timings type) SetTime(Timings.ScanOverlaps); } - lists.NullOutOne(); + lists.TrackAndNullOutOne(); } private Real step_dt; diff --git a/src/Jitter2/DataStructures/SlimBag.cs b/src/Jitter2/DataStructures/SlimBag.cs index dd087a5f..0da5cc57 100644 --- a/src/Jitter2/DataStructures/SlimBag.cs +++ b/src/Jitter2/DataStructures/SlimBag.cs @@ -87,7 +87,6 @@ public void Add(T item) } array[counter++] = item; - nullOut = counter; } private Jitter2.Parallelization.ReaderWriterLock rwLock; @@ -182,20 +181,15 @@ public void Clear() } /// - /// Sets unused positions in the internal array to their default values. + /// This should be called after adding entries to the SlimBag in order + /// to keep track of the largest index used within the internal array of + /// this datastructure. It will set this item in the array to its default value + /// to allow for garbage collection. /// - public void NullOut() + public void TrackAndNullOutOne() { - Array.Clear(array, counter, nullOut - counter); - nullOut = counter; - } - - /// - /// Null out a single position in the internal array. - /// - public void NullOutOne() - { - //if (nullOut <= counter) return; - //array[--nullOut] = default!; + nullOut = Math.Max(nullOut, counter); + if (nullOut <= counter) return; + array[--nullOut] = default!; } } \ No newline at end of file From 98900085a686065bb0b475bb3783f35e182c61f8 Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 17:41:54 +0100 Subject: [PATCH 6/7] Rename lists in DynamicTree.cs --- src/Jitter2/Collision/DynamicTree.cs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Jitter2/Collision/DynamicTree.cs b/src/Jitter2/Collision/DynamicTree.cs index 384b28a1..3fe45484 100644 --- a/src/Jitter2/Collision/DynamicTree.cs +++ b/src/Jitter2/Collision/DynamicTree.cs @@ -74,7 +74,7 @@ public interface IRayCastable /// public partial class DynamicTree { - private SlimBag lists = new(); + private readonly SlimBag movedProxies = new(); private readonly ActiveList proxies = new(); @@ -163,7 +163,7 @@ public enum Timings /// /// Gets the number of updated proxies. /// - public int UpdatedProxies => lists.Count; + public int UpdatedProxies => movedProxies.Count; /// /// Updates all entities that are marked as active in the active list. @@ -191,18 +191,18 @@ void SetTime(Timings type) proxies.ParallelForBatch(256, updateBoundingBoxes); SetTime(Timings.UpdateBoundingBoxes); - lists.Clear(); + movedProxies.Clear(); proxies.ParallelForBatch(24, scanMoved); SetTime(Timings.ScanMoved); - for (int i = 0; i < lists.Count; i++) + for (int i = 0; i < movedProxies.Count; i++) { - InternalAddRemoveProxy(lists[i]); + InternalAddRemoveProxy(movedProxies[i]); } SetTime(Timings.UpdateProxies); - lists.ParallelForBatch(24, scanOverlaps); + movedProxies.ParallelForBatch(24, scanOverlaps); SetTime(Timings.ScanOverlaps); } @@ -212,23 +212,23 @@ void SetTime(Timings type) UpdateBoundingBoxesCallback(batch); SetTime(Timings.UpdateBoundingBoxes); - lists.Clear(); + movedProxies.Clear(); scanMoved(batch); SetTime(Timings.ScanMoved); - for (int i = 0; i < lists.Count; i++) + for (int i = 0; i < movedProxies.Count; i++) { - InternalAddRemoveProxy(lists[i]); + InternalAddRemoveProxy(movedProxies[i]); } SetTime(Timings.UpdateProxies); - scanOverlaps(new Parallel.Batch(0, lists.Count)); + scanOverlaps(new Parallel.Batch(0, movedProxies.Count)); SetTime(Timings.ScanOverlaps); } - lists.TrackAndNullOutOne(); + movedProxies.TrackAndNullOutOne(); } private Real step_dt; @@ -571,7 +571,7 @@ private void ScanForMovedProxies(Parallel.Batch batch) if (node.ForceUpdate || !node.ExpandedBox.Encompasses(proxy.WorldBoundingBox)) { node.ForceUpdate = false; - lists.ConcurrentAdd(proxy); + movedProxies.ConcurrentAdd(proxy); } // else proxy is well contained within the nodes expanded Box: @@ -586,7 +586,7 @@ private void ScanForOverlaps(Parallel.Batch batch) { for (int i = batch.Start; i < batch.End; i++) { - OverlapCheckAdd(root, lists[i].NodePtr); + OverlapCheckAdd(root, movedProxies[i].NodePtr); } } From 77aaffa94d5e0c99c8ecc1cea3d31b7adaca2fe6 Mon Sep 17 00:00:00 2001 From: notgiven688 Date: Wed, 1 Jan 2025 17:43:45 +0100 Subject: [PATCH 7/7] Fix Tests --- src/JitterTests/MiscTests.cs | 39 ------------------------------------ 1 file changed, 39 deletions(-) diff --git a/src/JitterTests/MiscTests.cs b/src/JitterTests/MiscTests.cs index ffb1f1cf..3fdfc050 100644 --- a/src/JitterTests/MiscTests.cs +++ b/src/JitterTests/MiscTests.cs @@ -9,45 +9,6 @@ public void Setup() { } - [TestCase] - public static void SlimBagTest() - { - var bag = new SlimBag(); - bag.Add(new object()); - bag.Add(new object()); - bag.Add(new object()); - bag.Clear(); - Assert.That(bag[0], Is.Not.EqualTo(null)); - bag.NullOut(); - Assert.That(bag[0], Is.EqualTo(null)); - bag.Add(new object()); - bag.Add(new object()); - bag.RemoveAt(1); - Assert.That(bag[0], Is.Not.EqualTo(null)); - Assert.That(bag[1], Is.Not.EqualTo(null)); - bag.NullOut(); - Assert.That(bag[0], Is.Not.EqualTo(null)); - Assert.That(bag[1], Is.EqualTo(null)); - bag.Clear(); - bag.NullOut(); - bag.Add(new object()); - bag.Add(new object()); - bag.Add(new object()); - bag.NullOutOne(); - Assert.That(bag[0], Is.Not.EqualTo(null)); - Assert.That(bag[1], Is.Not.EqualTo(null)); - Assert.That(bag[2], Is.Not.EqualTo(null)); - bag.NullOutOne(); - bag.RemoveAt(1); - Assert.That(bag[2], Is.Not.EqualTo(null)); - bag.NullOutOne(); - Assert.That(bag[2], Is.EqualTo(null)); - bag.NullOutOne(); - Assert.That(bag[0], Is.Not.EqualTo(null)); - Assert.That(bag[1], Is.Not.EqualTo(null)); - Assert.That(bag[2], Is.EqualTo(null)); - } - [TestCase] public static void RequestId() {