Fastpaths for equivalency + super/sub set tests for certain types

[stevenaw]

Fixes #4254

This PR adds some fastpaths for certain commonly used types of collections. It:

• Switch to using a generic CollectionTally<T> class
• Opt-in to using specifically typed instantiations of CollectionTally<T> for a predefined list of common types.
  • This was done to balance optimizing for the likely common cases while avoiding a reflection-based lookup of generic type params
• Opts in to using a built-in EqualityComparer<T>.Default instead of the NUnitEqualityComparer when there would be no behavioural difference
• Opts in to using a O(n+m) algorithms instead of a O(n*m) where possible

I am submitting this as a draft for now as I'm still completing some cleanup of different paths, as well as some final investigation and experimentation for which types to "light up" for these fast paths. My performance testing has focused on updating the following equivalency tests to apply the constraint 500 times. I then run the test 5 times, discard the first timing, and average the next 4. All tests were run against .NET 10.

All times are for the full 500 iterations to run. "Before" is on main branch and "After" is using this branch.

TestName	Underlying data type	Before (ms)	After (ms)
LargeStringCollection	StringCollection	3,713	644
LargeStringCollectionsInSameOrder	IEnumerable<string>	3,636	548
LargeStringCollectionsInReversedOrder	IEnumerable<string>	3,636	514
LargeIntCollectionsInSameOrder	IEnumerable<int>	2,112	147
LargeByteCollectionsNotEquivalentAtEnd	byte[]	3,823	237

There is a general speedup of 6-12x for most data types based on:

• If it is a primitive type
• If the type is sortable
• If no "modifiers" are applied to the nunit comparer and default comparer rules can be used

[stevenaw]

I held off marking the non-generic CollectionTally obsolete for now as it becomes a bit tricky to do gracefully with the nested class that still gets referenced from a non-generic context

[stevenaw]

I've put this here rather than on the generic version with the long-term thought that the non-generic class will become obsolete and removed over time, and the need to convert to it will go away with it

[stevenaw]

I suspect there to be lots of buffer baked into even these numbers, but I don't think we should reduce them further. The affected tests each pass in single-digit ms times on my own machine (albeit with nothing running in the background).

In practice, these numbers on my own machine also correctly trigger warnings or failures if the "wrong" tallying strategy gets chosen, causing the algorithmic complexity and time to increase.

[stevenaw]

I'm marking this as Ready for Review. I think the changes are largely stable now, aside from perhaps if I notice any missing test coverage

[Copilot]

Pull request overview

This PR improves performance of collection equivalency, subset, and superset constraints by introducing typed fast paths (avoiding O(n·m) removal where possible) and by bypassing NUnitEqualityComparer in cases where EqualityComparer<T>.Default is behaviorally identical.

Changes:

• Added CollectionTally<T> and updated collection constraints to use a new TallyResult(...) helper with typed fast paths for common element types.
• Introduced generic IsSortable<T> / ImplementsIComparable<T> helpers and expanded tests to cover generic paths.
• Updated/expanded tests for null handling and adjusted “large collection” timing thresholds.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/NUnitFramework/framework/Constraints/CollectionItemsEqualConstraint.cs	Adds TallyResult/TallyResultCore helper selecting typed fast paths.
src/NUnitFramework/framework/Constraints/CollectionEquivalentConstraint.cs	Uses TallyResult instead of CollectionTally for equivalency checks.
src/NUnitFramework/framework/Constraints/CollectionSubsetConstraint.cs	Uses TallyResult for subset checks.
src/NUnitFramework/framework/Constraints/CollectionSupersetConstraint.cs	Uses TallyResult for superset checks.
src/NUnitFramework/framework/Constraints/CollectionTally{T}.cs	New generic tally implementation with multiple removal strategies.
src/NUnitFramework/framework/Constraints/CollectionTally.cs	Adds conversion helper from generic tally result to non-generic result type.
src/NUnitFramework/framework/Constraints/NUnitEqualityComparer.cs	Adds IsModified flag for deciding when default equality can be used safely.
src/NUnitFramework/framework/Constraints/NUnitEqualityComparerAdapter.cs	New adapter to use NUnitEqualityComparer as an IEqualityComparer<T>.
src/NUnitFramework/framework/Internal/Extensions/TypeExtensions.cs	Adds generic ImplementsIComparable<T> and IsSortable<T>.
src/NUnitFramework/framework/Internal/Extensions/IEnumerableExtensions.cs	Adds generic IsSortable<T>(IEnumerable<T>?) overload.
src/NUnitFramework/tests/Internal/CollectionTallyTests.cs	Updates tests to exercise CollectionTally<T> under multiple comparer configurations.
src/NUnitFramework/tests/Internal/Extensions/TypeExtensionTests.cs	Updates tests to validate both generic and reflection-based type checks.
src/NUnitFramework/tests/Internal/Extensions/IEnumerableExtensionTests.cs	Adds generic sortability tests and reworks test case sources.
src/NUnitFramework/tests/Constraints/CollectionEquivalentConstraintTests.cs	Expands null/modifier coverage and updates large-collection timing constants.
.github/copilot-instructions.md	Adds repository guidance about preserving existing performance optimizations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[manfred-brands]

I added 2 commits to address 2 issues found.
Others are questions

[manfred-brands]

We should not get here if c is IEquatable<T>.

[manfred-brands]

As we call this several times for the same type T, I wonder if we can cache the EqualMethod

It is either:

• The user specified external comparer of type EqualityAdapter
• The EqualMethod found in the list of NUnits Comparers
• PropertiesComparer.Equal

[stevenaw]

Great minds think alike.
I had actually initially worked on #3985 instead of this equivalency issue and had started going down a related but slightly different route which didn't quite feel right at the time.

I think your idea is a great way we could take some of the performance in both these areas further, however I'd prefer we not add it here in order to keep the PR focused more on the collectiontally and related algorithms there.

[stevenaw]

@manfred-brands One of your commits has broken a test. WorksWithCollectionsOfArrays was a particularly tricky one for myself not to break as well as I was adjusting the primitive/sortable/hashable logic and strategy choice.
I'd like to revert that commit temporarily and then make a follow-up commit to address your feedback about tracking "missing items" within the strategies without adjusting other logic

[manfred-brands]

@manfred-brands One of your commits has broken a test. WorksWithCollectionsOfArrays was a particularly tricky one for myself not to break as well as I was adjusting the primitive/sortable/hashable logic and strategy choice. I'd like to revert that commit temporarily and then make a follow-up commit to address your feedback about tracking "missing items" within the strategies without adjusting other logic

That is weird, I have a green build.
Ok it is because I call the non-generic CollectionTally which I hadn't pushed up. I'll see what goes wrong here.

[stevenaw]

@manfred-brands Please hold off on pushing further commits for now. There were a few conscious design choices of mine that I haven't had the opportunity to respond to your questions about yet

[manfred-brands]

@manfred-brands Please hold off on pushing further commits for now. There were a few conscious design choices of mine that I haven't had the opportunity to respond to your questions about yet

I won't, but the error is caused by the code picking the HashableItemStrategy and the HashCode of identical arrays is not the same:

array2
{byte[0x00000004]}
    [0]: 0x42
    [1]: 0x52
    [2]: 0x72
    [3]: 0xef
array2.GetHashCode()
0x00ad8b02
array4
{byte[0x00000004]}
    [0]: 0x42
    [1]: 0x52
    [2]: 0x72
    [3]: 0xef
array4.GetHashCode()
0x01d830be

So for the object case, you only can pick the QuadraticItemsStrategy

[stevenaw]

@manfred-brands Thanks for your detailed suggestions. I think I've been able to chip away at most of the suggested changes in bits of free time I had today. Comments which have been addressed by code changes have been marked resolved, but those which I have just responded to with comments haven't. I've also responded with some of my thinking for the design decisions around generic and non-generic collectiontally

There's still a handful of nullability-related items I'd like to investigate as well as changing ownership of _missingItems to the strategies for me to look at potential code changes for. That may not happen until Monday, however.

[stevenaw]

I think I've addressed all the suggestions. The three remaining unaddressed improvements which I would prefer be tackled separately are:

• IEnumerable support within NUnitEqualityComparerAdapter.GetHashCode()
• Expanding the number of types supported by CanUseDefaultEquality to include struct, record and possibly IEquatable<T>
• Caching or storing the resolved comparer used by NUnitEqualityComparer for a given or last-compared type

The last of those already has #3985. I can create tracking issues for the others as part of merging

[OsirisTerje]

@stevenaw There are several comments above that are not marked as Resolved. Your comment seems to indicate that they are resolved - do I read you correctly?

[stevenaw]

@OsirisTerje I consider a few to be "by design", however I left the actual comments unresolved in order to invite future discussion instead of hiding the ideas.

There are also separately two future improvements which can be made to take this further which I would feel more comfortable doing later. I think there's a lot of value merging this as it is already as it sets up several patterns for us to build on and/or use to modernise the codebase going forward