Fluentassertions: Asserting large array equality is slow

Created on 21 Oct 2016  路  5Comments  路  Source: fluentassertions/fluentassertions

Even with #489 fixed, asserting a large array is way too slow - about 8 seconds for 10M integers. Code to compare these directly is 80 times faster. FluentAssertions seems to be calling ToArray() on the array,s which should not be necessary to compare them.

Repro:

```c#
[TestMethod]
public void Should_be_fast_with_large_collection()
{
var collection1 = new int[10000000];
var collection2 = new int[collection1.Length];

// 8 seconds:
collection1.Should().Equal(collection2);

// ~0.1 seconds:

if (collection1.Length != collection2.Length)
    throw new ApplicationException("Length mistmatch");

for (var i = 0; i < collection1.Length; ++i)
{
    if (collection1[i] != collection2[i])
        throw new ApplicationException("Mistmatch at item " + i);
}

}
```

help wanted performance requires-clarification

Most helpful comment

I would take a look at that, if no objections.

All 5 comments

I would take a look at that, if no objections.

I looked at this for a while, I came to the conclusion that it is really hard to make performance improvements on this due to the way it is written.
To begin with, there are only 3 types of collection assertions this are: NonGenericCollectionAssertions, GenericCollectionAssertions, StringCollectionAssertions. GenericCollectionAssertions will assert any type, to make performance improvements, you would have to distinct between, at least, Primitive(Non-Nullables), Nullables types and other "class" types. While trying to add those types of collection assertion I am breaking existing functionality. If I try to implement sorting by type on a lower level, for instance in "EnumerableExtension" higher level conflict appear. I am really trying to avoid big if statement or switch statements and stick to the generic concept of the way it has been done. The change bellow is the only obvious I can see for now.

```c#
protected void AssertSubjectEquality(IEnumerable expectation,
Func equalityComparison, string because = "", params object[] becauseArgs)
{
...........................

  //1. instead of this use the original "expectation"
TExpected[] expectedItems = expectation.Cast<TExpected>().ToArray();

assertion                                      //2. here use expectation instead of expectedItems
    .WithExpectation("Expected {context:collection} to be equal to {0}{reason}, ", expectedItems)
    .Given(() => Subject.Cast<TActual>().ToList().AsEnumerable())
    .AssertCollectionsHaveSameCount(expectedItems.Length)
    .Then
    .AssertCollectionsHaveSameItems(expectedItems, (a, e) => a.IndexOfFirstDifferenceWith(e, equalityComparison));

}
```

I am still looking at it though. Might be that I don't see something obvious.

As well what is surprising for me that you are allowing to compare collection of different types when comparing their equality, should not there be a check for type equality when checking the collection for equality? Does not seem to be logic to me. You would still go off and loop though both collection despite one is a collection of int the other is of strings. However the, UTs for it says that such behaviour was intended so it is a bit confusing here. But yet again if you think about it, you go back to the problem that every type(except string) is falling under the category of "GenericCollectionAssertation", so you would probably have to have a distinction between types that I mentioned before.

As well what is suprising for me that you are allowing to compare collection of different types when comparing their equality

This should only happen when you use the ShouldBeEquivalentTo API.

@shift-evgeny I've made some optimizations in #840 to reduce both the overhead when comparing generic collections in general, and even more for collections of value types.

In #817 I also made some optimizations and in particular e01e0d7c2a5debc64cc5cb013beaebfba8b55e08 should benefit Equal.
The subject collection is now only copied if the assertions fails.

Even with #840 merged you can still squeeze out a bit more for value types if you provide a custom comparer function to Equal.
E.g. for you example collection1.Should().Equal(collection2, (s, e) => s == e);
In the benchmark result in #840 it isn't much faster, but the amount of memory allocations is a lot lower which should take off some pressure of GC when running a complete test suite.

I re-ran the benchmark code and the overhead stabilized around x12.
By explicitly stating the comparison function, as in collection1.Should().Equal(collection2, (s, e) => s == e);, it went further down to x8.
Also note that the x8 is from 0.017s to 0.14s.

Fluent Assertions will never be as fast as handwritten code and 0.14s doesn't strike me as a very slow unit test.

I'm gonna close this for now.
Feel free to re-open if it is still a problem or if you have any suggestions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felpel picture felpel  路  4Comments

dennisdoomen picture dennisdoomen  路  3Comments

robvanuden picture robvanuden  路  5Comments

matthiaslischka picture matthiaslischka  路  4Comments

carlin-q-scott picture carlin-q-scott  路  4Comments