-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Optimize ImmutableSortedSet<T>.SetEquals to avoid unnecessary allocations #126549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -375,24 +375,55 @@ public bool SetEquals(IEnumerable<T> other) | |
| return true; | ||
| } | ||
|
|
||
| var otherSet = new SortedSet<T>(other, this.KeyComparer); | ||
| if (this.Count != otherSet.Count) | ||
| switch (other) | ||
| { | ||
| return false; | ||
| case ImmutableSortedSet<T> otherAsImmutableSortedSet: | ||
| if (otherAsImmutableSortedSet.Count != this.Count) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (EqualityComparer<IComparer<T>>.Default.Equals(this.KeyComparer, otherAsImmutableSortedSet.KeyComparer)) | ||
| { | ||
| return SetEqualsWithImmutableSortedSet(otherAsImmutableSortedSet, this); | ||
| } | ||
| break; | ||
|
|
||
| case SortedSet<T> otherAsSortedSet: | ||
| if (otherAsSortedSet.Count != this.Count) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (EqualityComparer<IComparer<T>>.Default.Equals(this.KeyComparer, otherAsSortedSet.Comparer)) | ||
| { | ||
| return SetEqualsWithSortedSet(otherAsSortedSet, this); | ||
| } | ||
| break; | ||
|
|
||
| case ICollection<T> otherAsICollectionGeneric: | ||
| // We check for < instead of != because other is not guaranteed to be a set; it could be a collection with duplicates. | ||
| if (otherAsICollectionGeneric.Count < this.Count) | ||
| { | ||
| return false; | ||
| } | ||
| break; | ||
|
|
||
| case ICollection otherAsICollection: | ||
| if (otherAsICollection.Count < this.Count) | ||
| { | ||
| return false; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| int matches = 0; | ||
| foreach (T item in otherSet) | ||
| var otherSet = new SortedSet<T>(other, this.KeyComparer); | ||
| if (otherSet.Count != this.Count) | ||
| { | ||
| if (!this.Contains(item)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| matches++; | ||
| return false; | ||
| } | ||
|
|
||
| return matches == this.Count; | ||
| return SetEqualsWithSortedSet(otherSet, this); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -1016,6 +1047,62 @@ private static bool TryCastToImmutableSortedSet(IEnumerable<T> sequence, [NotNul | |
| return false; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks whether a given sequence of items entirely describe the contents of this set. | ||
| /// Note: | ||
| /// 1. Count equality must be verified by the caller before calling this method, | ||
| /// as it is invoked by multiple callers with different validation logic. | ||
| /// </summary> | ||
| /// <param name="other">The sequence of items to check against this set.</param> | ||
| /// <param name="source">The sequence of items in this.</param> | ||
| /// <returns>A value indicating whether the sets are equal.</returns> | ||
| private static bool SetEqualsWithImmutableSortedSet(ImmutableSortedSet<T> other, ImmutableSortedSet<T> source) | ||
| { | ||
| using var e1 = other.GetEnumerator(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case it's not evident to future readers, consider adding a comment explaining that the current algorithm works because both sets are sorted. |
||
| using var e2 = source.GetEnumerator(); | ||
|
|
||
| while (e1.MoveNext()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remind me, what prevents us from using a foreach to iterate |
||
| { | ||
| bool e2HasMore = e2.MoveNext(); | ||
| Debug.Assert(e2HasMore); | ||
|
|
||
| if (source.KeyComparer.Compare(e1.Current, e2.Current) != 0) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks whether a given sequence of items entirely describe the contents of this set. | ||
| /// Note: | ||
| /// 1. Count equality must be verified by the caller before calling this method, | ||
| /// as it is invoked by multiple callers with different validation logic. | ||
| /// </summary> | ||
| /// <param name="other">The sequence of items to check against this set.</param> | ||
| /// <param name="source">The sequence of items in this.</param> | ||
| /// <returns>A value indicating whether the sets are equal.</returns> | ||
| private static bool SetEqualsWithSortedSet(SortedSet<T> other, ImmutableSortedSet<T> source) | ||
| { | ||
| using var e1 = other.GetEnumerator(); | ||
| using var e2 = source.GetEnumerator(); | ||
|
|
||
| while (e1.MoveNext()) | ||
| { | ||
| bool e2HasMore = e2.MoveNext(); | ||
| Debug.Assert(e2HasMore); | ||
|
|
||
| if (source.KeyComparer.Compare(e1.Current, e2.Current) != 0) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates a new sorted set wrapper for a node tree. | ||
| /// </summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove XML comments from private methods.