From 995149ad01d7c74e7c97bfc76d098ac23f029fa5 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Wed, 3 Feb 2021 17:57:03 -0800 Subject: [PATCH] fix(core): Adds ability to store null value in ordered hash set (#447) - [X] Adds the ability to store a null value in an ordered hash set TODO: Optimized the OrderedHashSet. See this: * https://github.com/dotnet/runtime/issues/10050 Looks like the OrderedDictionary that I based this structure from was not updated. See the source for diffing: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs Difference between a HashSet and an OrderedHashSet is updating the indexes of the entries after fixing the chain to preserve insertion order. --- .../Tests/Collections/OrderedHashSetTests.cs | 24 ++++++++- Projects/Server/Collections/HashHelpers.cs | 23 +++++++++ Projects/Server/Collections/OrderedHashSet.cs | 49 +++++++++++-------- 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/Projects/Server.Tests/Tests/Collections/OrderedHashSetTests.cs b/Projects/Server.Tests/Tests/Collections/OrderedHashSetTests.cs index 133746876..916a5c389 100644 --- a/Projects/Server.Tests/Tests/Collections/OrderedHashSetTests.cs +++ b/Projects/Server.Tests/Tests/Collections/OrderedHashSetTests.cs @@ -9,7 +9,10 @@ namespace Server.Tests [Fact] public void TestOrderedHashSet() { - var set = new OrderedHashSet(1) { "random string1", "another random string1", "another random string2", "another random string3" }; + var set = new OrderedHashSet(1) + { + "random string1", "another random string1", "another random string2", "another random string3" + }; set.Remove("another random string1"); var list = set.ToList(); @@ -20,5 +23,24 @@ namespace Server.Tests Assert.Equal(entry, arr[i++]); } } + + [Fact] + public void TestOrderedHashWithNull() + { + var set = new OrderedHashSet(1) + { + "random string1", null, "another random string2", "another random string3" + }; + + set.Remove("another random string1"); + var list = set.ToList(); + + string[] arr = { "random string1", null, "another random string2", "another random string3" }; + int i = 0; + foreach (var entry in list) + { + Assert.Equal(entry, arr[i++]); + } + } } } diff --git a/Projects/Server/Collections/HashHelpers.cs b/Projects/Server/Collections/HashHelpers.cs index 98046027c..ca58de66b 100644 --- a/Projects/Server/Collections/HashHelpers.cs +++ b/Projects/Server/Collections/HashHelpers.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; +using System.Runtime.CompilerServices; namespace Microsoft.Collections.Extensions { @@ -98,5 +99,27 @@ namespace Microsoft.Collections.Extensions return GetPrime(newSize); } + + /// Returns approximate reciprocal of the divisor: ceil(2**64 / divisor). + /// This should only be used on 64-bit. + public static ulong GetFastModMultiplier(uint divisor) => + ulong.MaxValue / divisor + 1; + + /// Performs a mod operation using the multiplier pre-computed with . + /// This should only be used on 64-bit. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static uint FastMod(uint value, uint divisor, ulong multiplier) + { + // We use modified Daniel Lemire's fastmod algorithm (https://github.com/dotnet/runtime/pull/406), + // which allows to avoid the long multiplication if the divisor is less than 2**31. + Debug.Assert(divisor <= int.MaxValue); + + // This is equivalent of (uint)Math.BigMul(multiplier * value, divisor, out _). This version + // is faster than BigMul currently because we only need the high bits. + uint highbits = (uint)(((((multiplier * value) >> 32) + 1) * divisor) >> 32); + + Debug.Assert(highbits == value % divisor); + return highbits; + } } } diff --git a/Projects/Server/Collections/OrderedHashSet.cs b/Projects/Server/Collections/OrderedHashSet.cs index 9781688c9..c1b1813e7 100644 --- a/Projects/Server/Collections/OrderedHashSet.cs +++ b/Projects/Server/Collections/OrderedHashSet.cs @@ -56,6 +56,7 @@ namespace Server.Collections private static readonly Entry[] InitialEntries = new Entry[1]; private int[] _buckets = HashHelpers.SizeOneIntArray; private Entry[] _entries = InitialEntries; + private ulong _fastModMultiplier; private int _count; private int _version; private readonly IEqualityComparer _comparer; @@ -81,11 +82,13 @@ namespace Server.Collections { throw new ArgumentOutOfRangeException(nameof(capacity)); } + if (capacity > 0) { int newSize = HashHelpers.GetPrime(capacity); _buckets = new int[newSize]; _entries = new Entry[newSize]; + _fastModMultiplier = HashHelpers.GetFastModMultiplier((uint)newSize); } if (comparer != EqualityComparer.Default) @@ -300,6 +303,13 @@ namespace Server.Collections ArrayPool.Shared.Return(entriesToMove); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private ref int GetBucketRef(uint hashCode) + { + int[] buckets = _buckets!; + return ref buckets[HashHelpers.FastMod(hashCode, (uint)buckets.Length, _fastModMultiplier)]; + } + public bool Remove(TValue value) { int index = IndexOf(value); @@ -447,6 +457,9 @@ namespace Server.Collections int count = Count; Array.Copy(_entries, newEntries, count); + + _fastModMultiplier = HashHelpers.GetFastModMultiplier((uint)newSize); + for (int i = 0; i < count; ++i) { AddEntryToBucket(ref newEntries[i], i, newBuckets); @@ -459,27 +472,23 @@ namespace Server.Collections private int IndexOf(TValue value, out uint hashCode) { - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } - IEqualityComparer comparer = _comparer; - hashCode = (uint)(comparer?.GetHashCode(value) ?? value.GetHashCode()); - int index = _buckets[(int)(hashCode % (uint)_buckets.Length)] - 1; - if (index >= 0) + hashCode = (uint)(comparer?.GetHashCode(value) ?? value?.GetHashCode() ?? 0); + ref int bucket = ref GetBucketRef(hashCode); + int i = bucket - 1; + if (i >= 0) { comparer ??= EqualityComparer.Default; Entry[] entries = _entries; int collisionCount = 0; do { - Entry entry = entries[index]; + Entry entry = entries[i]; if (entry.HashCode == hashCode && comparer.Equals(entry.Value, value)) { break; } - index = entry.Next; + i = entry.Next; if (collisionCount >= entries.Length) { // The chain of entries forms a loop; which means a concurrent update has happened. @@ -487,9 +496,9 @@ namespace Server.Collections throw new InvalidOperationException(InvalidOperation_ConcurrentOperationsNotSupported); } ++collisionCount; - } while (index >= 0); + } while (i >= 0); } - return index; + return i; } #nullable enable @@ -541,16 +550,16 @@ namespace Server.Collections { Entry[] entries = _entries; Entry entry = entries[entryIndex]; - ref int b = ref _buckets[(int)(entry.HashCode % (uint)_buckets.Length)]; + ref int bucket = ref GetBucketRef(entry.HashCode); // Bucket was pointing to removed entry. Update it to point to the next in the chain - if (b == entryIndex + 1) + if (bucket == entryIndex + 1) { - b = entry.Next + 1; + bucket = entry.Next + 1; } else { // Start at the entry the bucket points to, and walk the chain until we find the entry with the index we want to remove, then fix the chain - int i = b - 1; + int i = bucket - 1; int collisionCount = 0; while (true) { @@ -576,16 +585,16 @@ namespace Server.Collections { Entry[] entries = _entries; Entry entry = entries[entryIndex]; - ref int b = ref _buckets[(int)(entry.HashCode % (uint)_buckets.Length)]; + ref int bucket = ref GetBucketRef(entry.HashCode); // Bucket was pointing to entry. Increment the index by incrementAmount. - if (b == entryIndex + 1) + if (bucket == entryIndex + 1) { - b += incrementAmount; + bucket += incrementAmount; } else { // Start at the entry the bucket points to, and walk the chain until we find the entry with the index we want to increment. - int i = b - 1; + int i = bucket - 1; int collisionCount = 0; while (true) {