From 37e905df2c7e2457692477d42fd9063de615f109 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 10:30:10 +0000 Subject: [PATCH] fix: make ContiguousSet and InsertionOrderSet IntersectWith/SymmetricExceptWith linear [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both operations dropped elements one at a time. ContiguousSet called Remove, which rescans the array from index 0 and shifts the tail per element, and InsertionOrderSet called List.RemoveAt per element. Dropping most of a set was O(n²): about 2.3 s (Release) or 23 s (Debug) to intersect a 100k-element ContiguousSet with an empty sequence. ContiguousSet now drops elements in a single compaction pass that clears the vacated slots and bumps the version once. InsertionOrderSet uses List.RemoveAll. Both are O(n + m) and keep the surviving elements in order. Fixes ktsu-dev/Containers#83 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J6q8NVYVFUeBVAH6tJCHnU --- Containers.Test/ContiguousSetTests.cs | 70 +++++++++++++++++++++++ Containers.Test/InsertionOrderSetTests.cs | 46 +++++++++++++++ Containers/ContiguousSet.cs | 55 ++++++++++++------ Containers/InsertionOrderSet.cs | 25 ++------ 4 files changed, 159 insertions(+), 37 deletions(-) diff --git a/Containers.Test/ContiguousSetTests.cs b/Containers.Test/ContiguousSetTests.cs index fd1d91a..b11d513 100644 --- a/Containers.Test/ContiguousSetTests.cs +++ b/Containers.Test/ContiguousSetTests.cs @@ -2,6 +2,7 @@ namespace ktsu.Containers.Tests; +using System.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -558,4 +559,73 @@ public void UnionWith_Itself_DoesNotThrow() Assert.AreSequenceEqual([1, 2, 3], set); } + + // Removing k of n elements one Remove at a time rescans and shifts the array per element: about + // 23 s for n = 100k in a Debug build. A single compaction pass takes milliseconds, so the bound + // below is far from both and does not depend on the machine. + private const int LargeSetSize = 100_000; + private const int LargeSetBudgetMilliseconds = 1_500; + + [TestMethod] + public void IntersectWith_LargeSetWithEmpty_RunsInLinearTime() + { + ContiguousSet set = [.. Enumerable.Range(0, LargeSetSize)]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.IntersectWith([]); + stopwatch.Stop(); + + Assert.IsEmpty(set); + Assert.IsLessThan(LargeSetBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void SymmetricExceptWith_LargeSetWithSameElements_RunsInLinearTime() + { + ContiguousSet set = [.. Enumerable.Range(0, LargeSetSize)]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.SymmetricExceptWith(Enumerable.Range(0, LargeSetSize)); + stopwatch.Stop(); + + Assert.IsEmpty(set); + Assert.IsLessThan(LargeSetBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void IntersectWith_KeepsSurvivorsInInsertionOrderAndUpdatesLookups() + { + ContiguousSet set = [5, 1, 4, 2, 3]; + + set.IntersectWith([3, 4, 5, 9]); + + Assert.AreSequenceEqual([5, 4, 3], set); + Assert.IsFalse(set.Contains(1)); + Assert.IsTrue(set.Add(1)); + Assert.AreSequenceEqual([5, 4, 3, 1], set); + } + + [TestMethod] + public void SymmetricExceptWith_KeepsSurvivorsInOrderThenAppendsOtherOnlyItems() + { + ContiguousSet set = [5, 1, 4, 2, 3]; + + set.SymmetricExceptWith([4, 9, 1]); + + Assert.AreSequenceEqual([5, 2, 3, 9], set); + Assert.IsFalse(set.Contains(4)); + Assert.IsTrue(set.Contains(9)); + } + + [TestMethod] + public void IntersectWith_RemovingElements_InvalidatesEnumerators() + { + ContiguousSet set = [1, 2, 3]; + using IEnumerator enumerator = set.GetEnumerator(); + Assert.IsTrue(enumerator.MoveNext()); + + set.IntersectWith([2]); + + Assert.ThrowsExactly(() => enumerator.MoveNext()); + } } diff --git a/Containers.Test/InsertionOrderSetTests.cs b/Containers.Test/InsertionOrderSetTests.cs index 79e7ace..a9aa634 100644 --- a/Containers.Test/InsertionOrderSetTests.cs +++ b/Containers.Test/InsertionOrderSetTests.cs @@ -2,6 +2,7 @@ namespace ktsu.Containers.Tests; +using System.Diagnostics; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -365,4 +366,49 @@ public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() string[] expectedItems = ["apple"]; Assert.AreSequenceEqual(expectedItems, set); } + + // One RemoveAt per dropped element shifts the list tail each time, so dropping the lower half of + // 200k elements moves about 1e10 elements (seconds); a single RemoveAll pass takes milliseconds. + private const int LargeSetSize = 200_000; + private const int LargeSetBudgetMilliseconds = 1_500; + + [TestMethod] + public void IntersectWith_LargeSetKeepingUpperHalf_RunsInLinearTime() + { + InsertionOrderSet set = [.. Enumerable.Range(0, LargeSetSize)]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.IntersectWith(Enumerable.Range(LargeSetSize / 2, LargeSetSize / 2)); + stopwatch.Stop(); + + Assert.HasCount(LargeSetSize / 2, set); + Assert.AreEqual(LargeSetSize / 2, set.First()); + Assert.IsLessThan(LargeSetBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void SymmetricExceptWith_LargeSetRemovingLowerHalf_RunsInLinearTime() + { + InsertionOrderSet set = [.. Enumerable.Range(0, LargeSetSize)]; + + Stopwatch stopwatch = Stopwatch.StartNew(); + set.SymmetricExceptWith(Enumerable.Range(0, LargeSetSize / 2)); + stopwatch.Stop(); + + Assert.HasCount(LargeSetSize / 2, set); + Assert.AreEqual(LargeSetSize / 2, set.First()); + Assert.IsLessThan(LargeSetBudgetMilliseconds, stopwatch.ElapsedMilliseconds); + } + + [TestMethod] + public void SymmetricExceptWith_KeepsSurvivorsInOrderThenAppendsOtherOnlyItems() + { + InsertionOrderSet set = [5, 1, 4, 2, 3]; + + set.SymmetricExceptWith([4, 9, 1]); + + Assert.AreSequenceEqual([5, 2, 3, 9], set); + Assert.IsFalse(set.Contains(4)); + Assert.IsTrue(set.Add(4)); + } } diff --git a/Containers/ContiguousSet.cs b/Containers/ContiguousSet.cs index cffdd28..f23c499 100644 --- a/Containers/ContiguousSet.cs +++ b/Containers/ContiguousSet.cs @@ -365,14 +365,7 @@ public void IntersectWith(IEnumerable other) HashSet otherSet = new(other, uniquenessSet.Comparer); // Remove items that are not in the other collection - for (int i = Count - 1; i >= 0; i--) - { - T item = items[i]; - if (!otherSet.Contains(item)) - { - Remove(item); - } - } + RemoveWhere(item => !otherSet.Contains(item)); } /// @@ -409,21 +402,51 @@ public void SymmetricExceptWith(IEnumerable other) HashSet otherSet = new(other, uniquenessSet.Comparer); // Remove items that are in both sets - for (int i = Count - 1; i >= 0; i--) + RemoveWhere(otherSet.Remove); + + // Add remaining items from other set (items that were only in other) + foreach (T item in otherSet) { - T item = items[i]; - if (otherSet.Remove(item)) + Add(item); + } + } + + /// + /// Removes every element that matches in a single compaction pass. + /// + /// Called once per element, in order; returns true to drop the element. + /// + /// O(n), and preserves the order of the elements that remain. Calling once per + /// dropped element would rescan and shift the array each time, which is O(n²) when most elements go. + /// + private void RemoveWhere(Func shouldRemove) + { + int write = 0; + for (int read = 0; read < Count; read++) + { + T item = items[read]; + if (shouldRemove(item)) + { + uniquenessSet.Remove(item); + } + else { - // Item exists in both - remove from current set - Remove(item); + items[write++] = item; } } - // Add remaining items from other set (items that were only in other) - foreach (T item in otherSet) + if (write == Count) { - Add(item); + return; } + + if (SlotClearing.IsNeeded()) + { + Array.Clear(items, write, Count - write); + } + + Count = write; + version++; } /// diff --git a/Containers/InsertionOrderSet.cs b/Containers/InsertionOrderSet.cs index 54df8b0..76da6ad 100644 --- a/Containers/InsertionOrderSet.cs +++ b/Containers/InsertionOrderSet.cs @@ -287,16 +287,8 @@ public void IntersectWith(IEnumerable other) HashSet otherSet = new(other, uniquenessSet.Comparer); - // Remove items that are not in the other collection - for (int i = items.Count - 1; i >= 0; i--) - { - T item = items[i]; - if (!otherSet.Contains(item)) - { - items.RemoveAt(i); - uniquenessSet.Remove(item); - } - } + // Remove items that are not in the other collection, in one pass rather than one RemoveAt each + items.RemoveAll(item => !otherSet.Contains(item) && uniquenessSet.Remove(item)); } /// @@ -333,17 +325,8 @@ public void SymmetricExceptWith(IEnumerable other) // Use a temporary set to avoid modifying the collection while enumerating HashSet otherSet = new(other, uniquenessSet.Comparer); - // Remove items that are in both sets - for (int i = items.Count - 1; i >= 0; i--) - { - T item = items[i]; - if (otherSet.Remove(item)) - { - // Item exists in both - remove from current set - items.RemoveAt(i); - uniquenessSet.Remove(item); - } - } + // Remove items that are in both sets, in one pass rather than one RemoveAt each + items.RemoveAll(item => otherSet.Remove(item) && uniquenessSet.Remove(item)); // Add remaining items from other set (items that were only in other) foreach (T item in otherSet)