Improve ImmutableArrayExtensions.SequenceEqual#118932
Merged
jeffhandley merged 16 commits intodotnet:mainfrom Feb 16, 2026
Merged
Improve ImmutableArrayExtensions.SequenceEqual#118932jeffhandley merged 16 commits intodotnet:mainfrom
ImmutableArrayExtensions.SequenceEqual#118932jeffhandley merged 16 commits intodotnet:mainfrom
Conversation
github-actions
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
Contributor
Author
|
@dotnet-policy-service agree |
xtqqczze
reviewed
Aug 20, 2025
src/libraries/System.Collections.Immutable/src/System/Linq/ImmutableArrayExtensions.cs
Outdated
Show resolved
Hide resolved
jkotas
added
area-System.Collections
and removed
needs-area-label
src/libraries/System.Collections.Immutable/src/System/Linq/ImmutableArrayExtensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Collections.Immutable/src/System/Linq/ImmutableArrayExtensions.cs
Outdated
Show resolved
Hide resolved
Contributor
|
Perhaps a simpler (and faster) way to do this is to just call |
…utableArrayExtensions.cs Co-authored-by: Pranav Senthilnathan <pranav.senthilnathan@live.com>
Contributor
Author
|
I'm not sure if it can be applied to |
Contributor
This should work: Requires.NotNull(items, nameof(items));
immutableArray.ThrowNullRefIfNotInitialized();
return immutableArray.array.SequenceEqual((IEnumerable<TBase>)items, comparer);If it is acceptable to delegate the argument validation to return immutableArray.array.SequenceEqual((IEnumerable<TBase>)items, comparer); |
Contributor
Author
|
@xtqqczze Full benchmark results |
Contributor
@prozolic I see a few regressions. Could you collect benchmark data for https://github.com/xtqqczze/dotnet-runtime/tree/ImmutableArrayExtensions.SequenceEqual?. This approach will not be worth the additional complexity, however. |
Contributor
Author
|
Sorry. Indeed, there were memory allocation issues and regressions in some cases. I conducted performance measurements comparing different implementations of SequenceEqual.
Full benchmark results#if NET6_0_OR_GREATER
public static bool SequenceEqual_2<TDerived, TBase>(this ImmutableArray<TBase> immutableArray, IEnumerable<TDerived> items, IEqualityComparer<TBase>? comparer = null) where TDerived : TBase
{
immutableArray.ThrowNullRefIfNotInitialized();
Requires.NotNull(items, nameof(items));
int i = 0;
if (items is ICollection<TBase> itemsCol)
{
if (itemsCol.TryGetSpan(out ReadOnlySpan<TBase> itemsSpan))
{
return immutableArray.array!.SequenceEqual(itemsSpan, comparer);
}
if (immutableArray.array!.Length != itemsCol.Count)
{
return false;
}
if (itemsCol is IList<TDerived> itemsList)
{
comparer ??= EqualityComparer<TBase>.Default;
int count = immutableArray.array!.Length;
for (i = 0; i < count; i++)
{
if (!comparer.Equals(immutableArray.array![i], itemsList[i]))
{
return false;
}
}
return true;
}
}
comparer ??= EqualityComparer<TBase>.Default;
int n = immutableArray.array!.Length;
foreach (TDerived item in items)
{
if (i == n)
{
return false;
}
if (!comparer.Equals(immutableArray.array![i], item))
{
return false;
}
i++;
}
return i == n;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)] // fast type checks that don't add a lot of overhead
private static bool TryGetSpan<TSource>(this IEnumerable<TSource> source, out ReadOnlySpan<TSource> span)
{
// Use `GetType() == typeof(...)` rather than `is` to avoid cast helpers. This is measurably cheaper
// but does mean we could end up missing some rare cases where we could get a span but don't (e.g. a uint[]
// masquerading as an int[]). That's an acceptable tradeoff. The Unsafe usage is only after we've
// validated the exact type; this could be changed to a cast in the future if the JIT starts to recognize it.
// We only pay the comparison/branching costs here for super common types we expect to be used frequently
// with LINQ methods.
bool result = true;
if (source.GetType() == typeof(TSource[]))
{
span = Unsafe.As<TSource[]>(source);
}
else if (source.GetType() == typeof(List<TSource>))
{
span = CollectionsMarshal.AsSpan(Unsafe.As<List<TSource>>(source));
}
else
{
span = default;
result = false;
}
return result;
}
#endif |
Contributor
OK, so using I think the best balance of complexity to performance would be to delegate to |
Contributor
|
Something like: {
if (items is ICollection<TBase> itemsCol)
{
immutableArray.ThrowNullRefIfNotInitialized();
return immutableArray.array!.SequenceEqual(itemsCol, comparer);
}
return Impl(immutableArray, items, comparer);
}and then put argument validation and existing implementation in static local function |
Contributor
Author
|
Thank you for the advice! public static bool SequenceEqual<TDerived, TBase>(this ImmutableArray<TBase> immutableArray, IEnumerable<TDerived> items, IEqualityComparer<TBase>? comparer = null) where TDerived : TBase
{
if (items is ICollection<TBase> itemsCol)
{
immutableArray.ThrowNullRefIfNotInitialized();
return immutableArray.array!.SequenceEqual(itemsCol, comparer);
}
return Impl(immutableArray, items, comparer);
static bool Impl(ImmutableArray<TBase> immutableArray, IEnumerable<TDerived> items, IEqualityComparer<TBase>? comparer)
{
Requires.NotNull(items, nameof(items));
comparer ??= EqualityComparer<TBase>.Default;
int i = 0;
int n = immutableArray.Length;
foreach (TDerived item in items)
{
if (i == n)
{
return false;
}
if (!comparer.Equals(immutableArray[i], item))
{
return false;
}
i++;
}
return i == n;
}
} |
Contributor
|
@prozolic You don't need the Could you also provide benchmark data again? |
Contributor
Author
|
@xtqqczze Here are the benchmark results comparing with the current existing implementation (SequenceEqual_Original): |
Contributor
|
@prozolic The results look excellent, very close to |
Contributor
|
It doesn’t look like we currently have performance coverage for this method in https://github.com/dotnet/performance. |
Contributor
Author
|
You're right, there's no official performance test coverage for this yet. From what I can see, there isn't much performance test coverage for ImmutableArray itself in dotnet/performance either. |
Contributor
Author
|
Regarding the benchmark, I submitted a pull request at dotnet/performance#4915. Original: Modified: |
System.Linq.Tests.Perf_ImmutableArrayExtensions
|
Contributor
|
@EgorBo Blocked on EgorBot/runtime-utils#504 |
Contributor
Author
|
@xtqqczze Thank you for the benchmarks and all the helpful reviews! Benchmark results (MihuBot/runtime-utils#1565):
Benchmark results (MihuBot/runtime-utils#1566):
Is there anything else needed to move this forward? |
Contributor
|
@prozolic Please update description with benchmark results from MihuBot/runtime-utils#1565 as the layout is clearer. I believe the PR is assigned to @PranavSenthilnathan to review. |
Contributor
Author
|
@xtqqczze Thank you for the follow-up! I've updated the PR description with the benchmark results from MihuBot/runtime-utils#1565 (X64). @PranavSenthilnathan The benchmark result shows improvements.
I would appreciate it if you could check the PR. Thank you! |
Member
PranavSenthilnathan
left a comment
PranavSenthilnathan
left a comment
There was a problem hiding this comment.
Perf results look good. Could you add/update tests to exercise the non-collection path? Currently the tests in ImmutableArrayExtensionsTest.cs only test with ICollection<TDerived> typed items objects.
Contributor
Author
|
@PranavSenthilnathan Thank you for confirming. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the performance of ImmutableArrayExtensions.SequenceEqual method by introducing a fast path for collections implementing ICollection<T>. The optimization delegates to the highly optimized Enumerable.SequenceEqual for these common collection types, while preserving the existing implementation logic for pure IEnumerable<T> types.
Key changes:
- Added fast path detection for
ICollection<TBase>types with delegation toEnumerable.SequenceEqual - Refactored the original implementation into a local
Enumeratefunction that serves as the fallback path - Added comprehensive test coverage using a new
ToEnumerablehelper method to test the fallback path
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Collections.Immutable/src/System/Linq/ImmutableArrayExtensions.cs | Implements the performance optimization with fast path for ICollection and fallback to original logic for IEnumerable |
| src/libraries/System.Collections.Immutable/tests/ImmutableArrayExtensionsTest.cs | Adds ToEnumerable helper and comprehensive tests for both fast path and fallback scenarios including edge cases with default arrays |
3 tasks
Member
|
@EgorBot -amd --filter "System.Linq.Tests.Perf_ImmutableArrayExtensions*" |
1 similar comment
Member
|
@EgorBot -amd --filter "System.Linq.Tests.Perf_ImmutableArrayExtensions*" |
jeffhandley
approved these changes
Feb 16, 2026
Member
jeffhandley
left a comment
jeffhandley
left a comment
There was a problem hiding this comment.
The latest numbers are great. Thanks for exploring all the options suggested along the way, @prozolic.
jeffhandley
added
tenet-performance
Contributor
Author
|
Thank you for the merge! I learned a lot from this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves the performance of ImmutableArrayExtensions.SequenceEqual method through optimized implementation strategies:
Key Performance Improvements:
Implementation Approach
The implementation was refined based on review feedback from @neon-sunset and @xtqqczze to achieve optimal performance while balancing complexity:
ICollection<T>and delegates to the highly optimizedEnumerable.SequenceEqualusing the underlying arrayIEnumerable<T>types to maintain behavioral consistencyBenchmark (dotnet/performance#4915)
Full benchmark results: MihuBot/runtime-utils#1565 (X64)