From 255f150334389425f174773fb14454467b7f2b74 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 01:32:15 +0000 Subject: [PATCH] fix: build OrderedSet/OrderedCollection/OrderedMap in O(n log n) and stop OrderedSet set operations going quadratic [patch] The bulk constructors filled the backing list one binary-search insert at a time, shifting the list tail on every insert, so building from n unsorted or reverse-sorted items was O(n^2). OrderedSet.ToComparerSet goes through that constructor, which made IsSubsetOf, IsProperSubsetOf, IsProperSupersetOf, SetEquals, IntersectWith and SymmetricExceptWith quadratic in the size of the argument: over a minute for a 1,000,000-element descending array. The constructors now copy the input and run a stable merge sort (new internal StableSort) with the container's comparer: - OrderedSet drops adjacent equal elements, keeping the first, as Add does - OrderedCollection keeps equal elements in their original order, as Add does - OrderedMap throws the same ArgumentException Add throws when two keys compare equal OrderedSet set operations also reuse an OrderedSet argument that has the same comparer instead of copying it, IntersectWith compacts with a single RemoveAll, and SymmetricExceptWith merges the two sorted lists in one linear pass. Fixes ktsu-dev/Containers#82 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K6bDUGMFsAQews3TnA2jXr --- Containers.Test/OrderedCollectionTests.cs | 27 ++++ Containers.Test/OrderedMapTests.cs | 43 +++++++ Containers.Test/OrderedSetTests.cs | 145 ++++++++++++++++++++++ Containers/OrderedCollection.cs | 18 ++- Containers/OrderedMap.cs | 42 +++++-- Containers/OrderedSet.cs | 93 +++++++++----- Containers/StableSort.cs | 96 ++++++++++++++ 7 files changed, 415 insertions(+), 49 deletions(-) create mode 100644 Containers/StableSort.cs diff --git a/Containers.Test/OrderedCollectionTests.cs b/Containers.Test/OrderedCollectionTests.cs index dc084e9..63b3e4b 100644 --- a/Containers.Test/OrderedCollectionTests.cs +++ b/Containers.Test/OrderedCollectionTests.cs @@ -3,6 +3,7 @@ namespace ktsu.Containers.Tests; using System.Collections; +using System.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -741,4 +742,30 @@ public void Constructor_FromEnumerableWithKeyComparer_PreservesInputOrderOfEqual // Assert Assert.AreSequenceEqual(source, collection); } + + [TestMethod] + public void Constructor_FromLargeDescendingSequence_RunsInLinearithmicTime() + { + const int size = 1_000_000; + int[] descending = [.. Enumerable.Range(0, size).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + OrderedCollection collection = new(descending, Comparer.Default); + stopwatch.Stop(); + + Assert.AreSequenceEqual(Enumerable.Range(0, size), collection); + Assert.IsLessThan(5_000, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void Constructor_FromLargeSequenceWithKeyComparer_KeepsEqualElementsInInsertionOrder() + { + // Enough elements that the bulk path merges rather than only insertion sorting + (int Key, string Name)[] source = [.. Enumerable.Range(0, 1_000).Select(i => (i * 7 % 10, i.ToString(System.Globalization.CultureInfo.InvariantCulture)))]; + + OrderedCollection<(int Key, string Name)> collection = new(source, ByKey); + + (int Key, string Name)[] expected = [.. source.OrderBy(item => item.Key)]; + Assert.AreSequenceEqual(expected, collection); + } } diff --git a/Containers.Test/OrderedMapTests.cs b/Containers.Test/OrderedMapTests.cs index f4ff983..39ac8bd 100644 --- a/Containers.Test/OrderedMapTests.cs +++ b/Containers.Test/OrderedMapTests.cs @@ -3,6 +3,7 @@ namespace ktsu.Containers.Test; using System.Collections; +using System.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; /// @@ -529,4 +530,46 @@ public void Enumerate_ChangesMadeThroughEveryMutator_Throw() => [TestMethod] public void Enumerate_Unchanged_VisitsEveryEntry() => Tests.MapEnumerationAssertions.UnchangedEnumerationVisitsEveryEntry(() => new OrderedMap()); + + [TestMethod] + public void Constructor_FromLargeDictionary_RunsInLinearithmicTime() + { + const int size = 1_000_000; + Dictionary dictionary = new(size); + for (int i = size - 1; i >= 0; i--) + { + dictionary.Add(i, -i); + } + + Stopwatch stopwatch = Stopwatch.StartNew(); + OrderedMap map = new(dictionary, Comparer.Default); + stopwatch.Stop(); + + Assert.HasCount(size, map); + Assert.AreSequenceEqual(Enumerable.Range(0, size), map.Keys); + Assert.AreEqual(-123, map[123]); + Assert.IsLessThan(5_000, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void Constructor_FromDictionaryWithKeysEqualUnderComparer_ThrowsLikeAdd() + { + Dictionary dictionary = new() { ["a"] = 1, ["b"] = 2, ["A"] = 3 }; + + Assert.ThrowsExactly(() => _ = new OrderedMap(dictionary, StringComparer.OrdinalIgnoreCase)); + + OrderedMap map = new(StringComparer.OrdinalIgnoreCase) { ["a"] = 1, ["b"] = 2 }; + Assert.ThrowsExactly(() => map.Add("A", 3)); + } + + [TestMethod] + public void Constructor_FromDictionaryWithCustomComparer_HonoursComparer() + { + Dictionary dictionary = new() { [2] = "two", [1] = "one", [3] = "three" }; + + OrderedMap map = new(dictionary, Comparer.Create((x, y) => y.CompareTo(x))); + + Assert.AreSequenceEqual([3, 2, 1], map.Keys); + Assert.AreSequenceEqual(["three", "two", "one"], map.Values); + } } diff --git a/Containers.Test/OrderedSetTests.cs b/Containers.Test/OrderedSetTests.cs index c9bb53b..c25bc4b 100644 --- a/Containers.Test/OrderedSetTests.cs +++ b/Containers.Test/OrderedSetTests.cs @@ -3,6 +3,7 @@ namespace ktsu.Containers.Test; using System.Collections; +using System.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -763,4 +764,148 @@ public void StressTest_MultipleOperations() Assert.IsLessThan(result[i], result[i - 1], "Elements should remain sorted"); } } + + // Filling the backing list one binary-search insert at a time shifts the tail on every insert, so + // building from 1,000,000 descending items takes over a minute; a stable sort takes well under a second. + private const int BulkSize = 1_000_000; + private const int BulkBudgetMilliseconds = 5_000; + + private static readonly IComparer<(int Key, int Index)> ByKey = + Comparer<(int Key, int Index)>.Create((x, y) => x.Key.CompareTo(y.Key)); + + [TestMethod] + public void Constructor_FromLargeDescendingSequence_RunsInLinearithmicTime() + { + int[] descending = [.. Enumerable.Range(0, BulkSize).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + OrderedSet set = new(descending, Comparer.Default); + stopwatch.Stop(); + + Assert.HasCount(BulkSize, set); + Assert.AreSequenceEqual(Enumerable.Range(0, BulkSize), set); + Assert.IsLessThan(BulkBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void IsSubsetOf_LargeDescendingArray_RunsInLinearithmicTime() + { + OrderedSet set = [.. Enumerable.Range(0, BulkSize)]; + int[] descending = [.. Enumerable.Range(0, BulkSize).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + bool isSubset = set.IsSubsetOf(descending); + stopwatch.Stop(); + + Assert.IsTrue(isSubset); + Assert.IsLessThan(BulkBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void SetEquals_LargeDescendingArray_RunsInLinearithmicTime() + { + OrderedSet set = [.. Enumerable.Range(0, BulkSize)]; + int[] descending = [.. Enumerable.Range(0, BulkSize).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + bool equal = set.SetEquals(descending); + stopwatch.Stop(); + + Assert.IsTrue(equal); + Assert.IsLessThan(BulkBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void SymmetricExceptWith_LargeDescendingArray_RunsInLinearithmicTime() + { + OrderedSet set = [.. Enumerable.Range(0, BulkSize)]; + int[] descending = [.. Enumerable.Range(BulkSize / 2, BulkSize).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.SymmetricExceptWith(descending); + stopwatch.Stop(); + + Assert.AreSequenceEqual(Enumerable.Range(0, BulkSize / 2).Concat(Enumerable.Range(BulkSize, BulkSize / 2)), set); + Assert.IsLessThan(BulkBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void IntersectWith_LargeDescendingArray_RunsInLinearithmicTime() + { + OrderedSet set = [.. Enumerable.Range(0, BulkSize)]; + int[] descending = [.. Enumerable.Range(BulkSize / 2, BulkSize).Reverse()]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.IntersectWith(descending); + stopwatch.Stop(); + + Assert.AreSequenceEqual(Enumerable.Range(BulkSize / 2, BulkSize / 2), set); + Assert.IsLessThan(BulkBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void Constructor_FromSequenceWithDuplicates_KeepsFirstOccurrence() + { + // Enough elements that the bulk path merges rather than only insertion sorting + (int Key, int Index)[] source = [.. Enumerable.Range(0, 1_000).Select(i => (i * 7 % 10, i))]; + + OrderedSet<(int Key, int Index)> set = new(source, ByKey); + + (int Key, int Index)[] expected = [.. Enumerable.Range(0, 10).Select(key => source.First(item => item.Key == key))]; + Assert.AreSequenceEqual(expected, set); + } + + [TestMethod] + public void Constructor_FromSequenceWithCustomComparer_HonoursComparer() + { + OrderedSet set = new(["b", "A", "a", "B", "c"], StringComparer.OrdinalIgnoreCase); + + Assert.AreSequenceEqual(["A", "b", "c"], set); + } + + [TestMethod] + public void Constructor_FromSequenceWithReverseComparer_SortsDescending() + { + OrderedSet set = new(Enumerable.Range(0, 100), Comparer.Create((x, y) => y.CompareTo(x))); + + Assert.AreSequenceEqual(Enumerable.Range(0, 100).Reverse(), set); + } + + [TestMethod] + public void Constructor_FromSequence_MatchesRepeatedAdd() + { + Random random = new(42); + int[] source = [.. Enumerable.Range(0, 5_000).Select(_ => random.Next(1_000))]; + OrderedSet added = []; + foreach (int item in source) + { + added.Add(item); + } + + OrderedSet built = new(source, Comparer.Default); + + Assert.AreSequenceEqual(added, built); + } + + [TestMethod] + public void SymmetricExceptWith_Self_EmptiesSet() + { + OrderedSet set = [1, 2, 3]; + + set.SymmetricExceptWith(set); + + Assert.HasCount(0, set); + } + + [TestMethod] + public void SymmetricExceptWith_OrderedSetWithSameComparer_KeepsElementsInOnlyOne() + { + OrderedSet set = [1, 2, 3, 4]; + OrderedSet other = [3, 4, 5, 6]; + + set.SymmetricExceptWith(other); + + Assert.AreSequenceEqual([1, 2, 5, 6], set); + Assert.AreSequenceEqual([3, 4, 5, 6], other); + } } diff --git a/Containers/OrderedCollection.cs b/Containers/OrderedCollection.cs index eb64144..fe22680 100644 --- a/Containers/OrderedCollection.cs +++ b/Containers/OrderedCollection.cs @@ -157,13 +157,12 @@ public OrderedCollection(IEnumerable collection) ); } - items = []; comparer = Comparer.Default; - foreach (T item in collection) - { - Add(item); - } + // Inserting one at a time is O(n^2); a stable sort keeps equal elements in their original order, as Add does + T[] array = [.. collection]; + StableSort.Sort(array, comparer); + items = [.. array]; } /// @@ -177,13 +176,12 @@ public OrderedCollection(IEnumerable collection, IComparer comparer) Ensure.NotNull(collection); Ensure.NotNull(comparer); - items = []; this.comparer = comparer; - foreach (T item in collection) - { - Add(item); - } + // Inserting one at a time is O(n^2); a stable sort keeps equal elements in their original order, as Add does + T[] array = [.. collection]; + StableSort.Sort(array, this.comparer); + items = [.. array]; } /// diff --git a/Containers/OrderedMap.cs b/Containers/OrderedMap.cs index 5f82d39..47316bb 100644 --- a/Containers/OrderedMap.cs +++ b/Containers/OrderedMap.cs @@ -207,13 +207,8 @@ public OrderedMap(IDictionary dictionary) ); } - items = []; comparer = Comparer.Default; - - foreach (KeyValuePair pair in dictionary) - { - Add(pair.Key, pair.Value); - } + items = BuildSorted(dictionary, this.comparer); } /// @@ -228,13 +223,44 @@ public OrderedMap(IDictionary dictionary, IComparer comparer Ensure.NotNull(dictionary); Ensure.NotNull(comparer); - items = []; this.comparer = comparer; + items = BuildSorted(dictionary, this.comparer); + } + /// + /// Builds the backing list for a map from an existing dictionary. + /// + /// The key-value pairs to add. + /// The comparer that orders the keys and decides which are equal. + /// The entries sorted by key. + /// + /// Adding the pairs one at a time costs O(n) per insert, so O(n^2) in all. Sorting a copy costs O(n log n). + /// + /// Thrown when a key is null. + /// Thrown when two keys compare equal under , as would. + private static List BuildSorted(IDictionary dictionary, IComparer comparer) + { + List list = new(dictionary.Count); foreach (KeyValuePair pair in dictionary) { - Add(pair.Key, pair.Value); + Ensure.NotNull((object?)pair.Key); + list.Add(new Entry(pair.Key, pair.Value)); } + + Entry[] entries = [.. list]; + + StableSort.Sort(entries, Comparer.Create((x, y) => comparer.Compare(x.Key, y.Key))); + + for (int i = 1; i < entries.Length; i++) + { + if (comparer.Compare(entries[i - 1].Key, entries[i].Key) == 0) + { + // The stable sort keeps dictionary order, so this is the key Add would have rejected + throw new ArgumentException($"The key '{entries[i].Key}' already exists in the map."); + } + } + + return [.. entries]; } /// diff --git a/Containers/OrderedSet.cs b/Containers/OrderedSet.cs index 3af2841..9ababb2 100644 --- a/Containers/OrderedSet.cs +++ b/Containers/OrderedSet.cs @@ -147,13 +147,8 @@ public OrderedSet(IEnumerable collection) ); } - items = []; Comparer = Comparer.Default; - - foreach (T item in collection) - { - Add(item); - } + items = BuildSortedDistinct(collection, Comparer); } /// @@ -167,13 +162,37 @@ public OrderedSet(IEnumerable collection, IComparer comparer) Ensure.NotNull(collection); Ensure.NotNull(comparer); - items = []; Comparer = comparer; + items = BuildSortedDistinct(collection, comparer); + } - foreach (T item in collection) + /// + /// Builds the backing list for a set from an arbitrary sequence. + /// + /// The elements to add. + /// The comparer that orders the elements and decides which are equal. + /// The distinct elements in sorted order, keeping the first of any that compare equal, as repeated calls to would. + /// + /// Adding the elements one at a time costs O(n) per insert, so O(n^2) in all. Sorting a copy + /// stably and dropping adjacent duplicates costs O(n log n). + /// + private static List BuildSortedDistinct(IEnumerable collection, IComparer comparer) + { + T[] array = [.. collection]; + StableSort.Sort(array, comparer); + + int count = 0; + for (int i = 0; i < array.Length; i++) { - Add(item); + if (count == 0 || comparer.Compare(array[count - 1], array[i]) != 0) + { + array[count++] = array[i]; + } } + + List list = new(count); + list.AddRange(new ArraySegment(array, 0, count)); + return list; } /// @@ -313,7 +332,8 @@ public int BinarySearch(T item) /// which disagrees with whenever a custom comparer was supplied, so the /// temporary set is built with instead. /// - private OrderedSet ToComparerSet(IEnumerable other) => new(other, Comparer); + private OrderedSet ToComparerSet(IEnumerable other) => + other is OrderedSet set && set.Comparer.Equals(Comparer) ? set : new(other, Comparer); /// /// Returns an enumerator that iterates through the set in sorted order. @@ -354,13 +374,8 @@ public void IntersectWith(IEnumerable other) OrderedSet otherSet = ToComparerSet(other); - for (int i = items.Count - 1; i >= 0; i--) - { - if (!otherSet.Contains(items[i])) - { - items.RemoveAt(i); - } - } + // Compacting in one pass keeps this linear, where removing one element at a time would not be + items.RemoveAll(item => !otherSet.Contains(item)); } /// @@ -395,31 +410,47 @@ public void SymmetricExceptWith(IEnumerable other) Ensure.NotNull(other); OrderedSet otherSet = ToComparerSet(other); + if (ReferenceEquals(otherSet, this)) + { + Clear(); + return; + } - // Create a list of items to remove from otherSet as we find them - List toRemoveFromOther = []; - - // Remove items that are in both sets - for (int i = items.Count - 1; i >= 0; i--) + // Both sequences are sorted and distinct, so merge them, keeping the elements found in only one + List otherItems = otherSet.items; + List result = new(items.Count + otherItems.Count); + int i = 0; + int j = 0; + while (i < items.Count && j < otherItems.Count) { - if (otherSet.Contains(items[i])) + int comparison = Comparer.Compare(items[i], otherItems[j]); + if (comparison < 0) + { + result.Add(items[i++]); + } + else if (comparison > 0) + { + result.Add(otherItems[j++]); + } + else { - toRemoveFromOther.Add(items[i]); - items.RemoveAt(i); + i++; + j++; } } - // Remove common items from other set - foreach (T item in toRemoveFromOther) + for (; i < items.Count; i++) { - otherSet.Remove(item); + result.Add(items[i]); } - // Add items that are only in the other set - foreach (T item in otherSet) + for (; j < otherItems.Count; j++) { - Add(item); + result.Add(otherItems[j]); } + + items.Clear(); + items.AddRange(result); } /// diff --git a/Containers/StableSort.cs b/Containers/StableSort.cs new file mode 100644 index 0000000..7146038 --- /dev/null +++ b/Containers/StableSort.cs @@ -0,0 +1,96 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers; + +/// +/// A stable merge sort, used by the sorted containers to build their backing list in bulk. +/// +/// +/// is introspective and so not stable, which would +/// reorder elements that compare equal. The sorted containers rely on equal elements keeping their +/// original order: keeps the first of them, and +/// keeps them in insertion order. +/// +internal static class StableSort +{ + /// + /// Ranges at or below this length are sorted by insertion sort rather than split further. + /// + private const int InsertionSortThreshold = 16; + + /// + /// Sorts in place, keeping elements that compare equal in their original order. + /// + /// The element type. + /// The array to sort. + /// The comparer that defines the order. + /// O(n log n) in the worst case, and O(n) when the input is already sorted. + internal static void Sort(T[] array, IComparer comparer) + { + if (array.Length < 2) + { + return; + } + + T[] buffer = new T[(array.Length / 2) + 1]; + SortRange(array, buffer, 0, array.Length, comparer); + } + + /// + /// Sorts the elements of in [, ). + /// + private static void SortRange(T[] array, T[] buffer, int low, int high, IComparer comparer) + { + if (high - low <= InsertionSortThreshold) + { + InsertionSort(array, low, high, comparer); + return; + } + + int mid = low + ((high - low) / 2); + SortRange(array, buffer, low, mid, comparer); + SortRange(array, buffer, mid, high, comparer); + + // The halves are already in order relative to each other + if (comparer.Compare(array[mid - 1], array[mid]) <= 0) + { + return; + } + + // Merge, taking from the left half on ties so equal elements keep their order + int leftLength = mid - low; + Array.Copy(array, low, buffer, 0, leftLength); + + int i = 0; + int j = mid; + int k = low; + while (i < leftLength && j < high) + { + array[k++] = comparer.Compare(array[j], buffer[i]) < 0 ? array[j++] : buffer[i++]; + } + + if (i < leftLength) + { + Array.Copy(buffer, i, array, k, leftLength - i); + } + } + + /// + /// Stable insertion sort of the elements of in [, ). + /// + private static void InsertionSort(T[] array, int low, int high, IComparer comparer) + { + for (int i = low + 1; i < high; i++) + { + T item = array[i]; + int j = i - 1; + while (j >= low && comparer.Compare(array[j], item) > 0) + { + array[j + 1] = array[j]; + j--; + } + + array[j + 1] = item; + } + } +}