Optimizations #817
Merged
Optimizations #817
Conversation
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
Member
|
Could we perhaps add some benchmark tests to the repo similar to the ones you ran? |
Member
|
What did you use? DotTrace? |
Member
Author
|
I used the builtin profiler in Visual Studio 2017. |
Member
Author
|
@Meir017 Do you have an idea how to structure this? My thoughts:
|
Member
|
yup 👍 I was thinking of maybe using existing tests and executing them multiple times public class CollectionEquivalencySpecsBenchmark
{
public CollectionEquivalencySpecs Subject { get; }
public CollectionEquivalencySpecsBenchmark()
{
Subject = new CollectionEquivalencySpecs();
}
[Benchmark]
public void When_a_collection_does_not_match_it_should_include_items_in_message()
=> Subject.When_a_collection_does_not_match_it_should_include_items_in_message();
}and of course we could generate this code :) |
Member
|
Now let's hope AppVeyor uses the same kind of machine every time. |
Member
Author
|
I wouldn't count on AppVeyor using the same kind of machine every time. The best approach I can currently think of, it setting up a benchmark baseline before doing improvements and running it locally before and after to verify improvements. Ideally it would require a dedicated benchmark machine, but I think that's out of scope. |
dennisdoomen
approved these changes
Mar 30, 2018
95a6c96 to
de9cdbd
Compare
Member
Author
|
I'll be adding more benchmarks before merging this, but here are the results of before and after the Before: |
Member
Author
|
TLDR;
baseline: Cache Deletegate - 4140064 Combine Linq expressions - 6c0adea Cast enumerable to list/collection - e01e0d7 Optimize HasValueSemantics - 9abdff3 Use a list of unmatched indexes - cd719c3 Combine() - 3d3cca7 Cache HasValueSemantics - 95738ed Cache OverridesEquals - 12bd297 |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
I tried to benchmark
BeEquivalentToas that seems to the most heavy assertion due to heavy use of reflection.The most visible bottleneck was recreating the delegate for
TraceBlockinLooselyMatchAgainstin each iteration, instead of caching it.It used 24.15% of the time.
There are also a lot of places, where we convert an
IEnumerableto a new Array/List, but it seems we could just cast the object if it actually is anICollectionorIList,So I created helper functions
ConvertOrCastToCollectionandConvertOrCastToListto help reduce the amount of unnecessary copying.Here's the code I used to test two lists of 20.000 identical 6-nested objects.