From f6d12a1dcf162b05ebd61900d9ebf81ebf58a38f Mon Sep 17 00:00:00 2001 From: Linus Hamlin <78953007+lilinus@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:12:29 +0200 Subject: [PATCH] Improve fast-return for HashSet SubSet and SetEquals methods (#102758) Co-authored-by: Stephen Toub --- .../src/System/Collections/Generic/HashSet.cs | 74 ++++++++++--------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs index 658716801fefa..ba6d497a29f89 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs @@ -879,25 +879,26 @@ public bool IsSubsetOf(IEnumerable other) } // The empty set is a subset of any set, and a set is a subset of itself. - // Set is always a subset of itself + // Set is always a subset of itself. if (Count == 0 || other == this) { return true; } - // Faster if other has unique elements according to this equality comparer; so check - // that other is a hashset using the same equality comparer. - if (other is HashSet otherAsSet && EqualityComparersAreEqual(this, otherAsSet)) + if (other is ICollection otherAsCollection) { - // if this has more elements then it can't be a subset - if (Count > otherAsSet.Count) + // If this has more elements then it can't be a subset. + if (Count > otherAsCollection.Count) { return false; } - // already checked that we're using same equality comparer. simply check that - // each element in this is contained in other. - return IsSubsetOfHashSetWithSameComparer(otherAsSet); + // Faster if other has unique elements according to this equality comparer; so check + // that other is a hashset using the same equality comparer. + if (other is HashSet otherAsSet && EqualityComparersAreEqual(this, otherAsSet)) + { + return IsSubsetOfHashSetWithSameComparer(otherAsSet); + } } (int uniqueCount, int unfoundCount) = CheckUniqueAndUnfoundElements(other, returnIfUnfound: false); @@ -922,8 +923,8 @@ public bool IsProperSubsetOf(IEnumerable other) if (other is ICollection otherAsCollection) { - // No set is a proper subset of an empty set. - if (otherAsCollection.Count == 0) + // No set is a proper subset of a set with less or equal number of elements. + if (otherAsCollection.Count <= Count) { return false; } @@ -931,17 +932,13 @@ public bool IsProperSubsetOf(IEnumerable other) // The empty set is a proper subset of anything but the empty set. if (Count == 0) { - return otherAsCollection.Count > 0; + // Based on check above, other is not empty when Count == 0. + return true; } // Faster if other is a hashset (and we're using same equality comparer). if (other is HashSet otherAsSet && EqualityComparersAreEqual(this, otherAsSet)) { - if (Count >= otherAsSet.Count) - { - return false; - } - // This has strictly less than number of items in other, so the following // check suffices for proper subset. return IsSubsetOfHashSetWithSameComparer(otherAsSet); @@ -1088,33 +1085,38 @@ public bool SetEquals(IEnumerable other) return true; } - // Faster if other is a hashset and we're using same equality comparer. - if (other is HashSet otherAsSet && EqualityComparersAreEqual(this, otherAsSet)) + if (other is ICollection otherAsCollection) { - // Attempt to return early: since both contain unique elements, if they have - // different counts, then they can't be equal. - if (Count != otherAsSet.Count) + // If this is empty, they are equal iff other is empty. + if (Count == 0) { - return false; + return otherAsCollection.Count == 0; } - // Already confirmed that the sets have the same number of distinct elements, so if - // one is a subset of the other then they must be equal. - return IsSubsetOfHashSetWithSameComparer(otherAsSet); - } - else - { - // If this count is 0 but other contains at least one element, they can't be equal. - if (Count == 0 && - other is ICollection otherAsCollection && - otherAsCollection.Count > 0) + // Faster if other is a hashset and we're using same equality comparer. + if (other is HashSet otherAsSet && EqualityComparersAreEqual(this, otherAsSet)) { - return false; + // Attempt to return early: since both contain unique elements, if they have + // different counts, then they can't be equal. + if (Count != otherAsSet.Count) + { + return false; + } + + // Already confirmed that the sets have the same number of distinct elements, so if + // one is a subset of the other then they must be equal. + return IsSubsetOfHashSetWithSameComparer(otherAsSet); } - (int uniqueCount, int unfoundCount) = CheckUniqueAndUnfoundElements(other, returnIfUnfound: true); - return uniqueCount == Count && unfoundCount == 0; + // Can't be equal if other set contains fewer elements than this. + if (Count > otherAsCollection.Count) + { + return false; + } } + + (int uniqueCount, int unfoundCount) = CheckUniqueAndUnfoundElements(other, returnIfUnfound: true); + return uniqueCount == Count && unfoundCount == 0; } public void CopyTo(T[] array) => CopyTo(array, 0, Count);