-
Notifications
You must be signed in to change notification settings - Fork 733
Resolved a significant performance degradation in BeEquivalentTo
#1660
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
Resolved a significant performance degradation in BeEquivalentTo
#1660
Conversation
dc2fb5b to
e008609
Compare
jnyrup
left a comment
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.
Benchmark results:
master:
| Runtime | N | Mean | Error | StdDev | Gen 0 | Gen 1 | Allocated |
|--------------------- |--- |----------------:|--------------:|--------------:|------------:|----------:|-------------:|
| .NET 5.0 | 1 | 57.09 us | 0.299 us | 0.279 us | 11.1694 | 0.3052 | 69 KB |
| .NET Framework 4.7.2 | 1 | 2,025.39 us | 10.364 us | 9.695 us | 136.7188 | 7.8125 | 855 KB |
| .NET 5.0 | 10 | 165,775.29 us | 1,407.268 us | 1,316.360 us | 11000.0000 | 1000.0000 | 68,264 KB |
| .NET Framework 4.7.2 | 10 | 216,483.12 us | 687.123 us | 573.779 us | 14333.3333 | 1000.0000 | 88,899 KB |
| .NET 5.0 | 50 | 4,604,075.87 us | 28,517.080 us | 26,674.895 us | 308000.0000 | 1000.0000 | 1,892,259 KB |
| .NET Framework 4.7.2 | 50 | 5,552,894.02 us | 47,078.673 us | 41,734.018 us | 365000.0000 | 1000.0000 | 2,243,784 KB |
PR 1660
| Runtime | N | Mean | Error | StdDev | Gen 0 | Gen 1 | Allocated |
|--------------------- |--- |--------------:|-------------:|-------------:|-----------:|----------:|-----------:|
| .NET 5.0 | 1 | 56.26 us | 0.351 us | 0.328 us | 11.1694 | 0.3052 | 69 KB |
| .NET Framework 4.7.2 | 1 | 277.59 us | 3.679 us | 3.441 us | 28.8086 | 0.9766 | 178 KB |
| .NET 5.0 | 10 | 11,297.99 us | 25.180 us | 23.553 us | 1468.7500 | 140.6250 | 9,038 KB |
| .NET Framework 4.7.2 | 10 | 16,182.11 us | 320.831 us | 315.099 us | 1843.7500 | 156.2500 | 11,492 KB |
| .NET 5.0 | 50 | 329,054.39 us | 1,672.983 us | 1,483.056 us | 40000.0000 | 7000.0000 | 249,728 KB |
| .NET Framework 4.7.2 | 50 | 462,394.07 us | 1,273.735 us | 1,129.133 us | 50000.0000 | 2000.0000 | 308,181 KB |
| } | ||
|
|
||
| [Fact] | ||
| public void Comparing_lots_of_complex_objects_should_still_be_fast() |
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.
Failed in pipeline
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.
Yeah, AppVeyor is a lot slower. Had to increase the threshold to 20 seconds.
This was caused by the increased dependency on the CallerIdentifier, and the additional work caller identification takes in v6.
e008609 to
c3d260a
Compare
|
All in all, I think it would be nice if we can come up with some approach to fail the build if those benchmarks exceed a certain value. |
|
@jnyrup, you created a benchmark but did not added it to the solution? |
|
I was just about to add the benchmarks, but seems my former self already did that in #1661. |
mdentremont
left a comment
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.
Good catch and fix!
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 5.10.3 to 6.1.0. <details> <summary>Release notes</summary> *Sourced from [FluentAssertions's releases](https://github.com/fluentassertions/fluentassertions/releases).* > ## 6.1.0 > [Release Notes](https://fluentassertions.com/releases/[#610](https://github.com/fluentassertions/fluentassertions/issues/610)) > > ## 6.0.0 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/master/docs/_pages/releases.md#600) > > ## 6.0.0-beta.3 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/6.0.0-beta.3/docs/_pages/releases.md#600-beta-3) > > ## 6.0.0-beta.2 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-2) > > ## 6.0.0-beta.1 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-1) > > ## 6.0.0-alpha.2 > See [release notes](https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600-alpha-2). > > ## 6.0.0-alpha.1 > See https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600 </details> <details> <summary>Commits</summary> - [`ce07e5c`](fluentassertions/fluentassertions@ce07e5c) Bump the release number for 6.1 - [`577d555`](fluentassertions/fluentassertions@577d555) Resolved a significant performance degradation in `BeEquivalentTo` ([#1660](fluentassertions/fluentassertions#1660)) - [`a38a54a`](fluentassertions/fluentassertions@a38a54a) Fix small grammatical error ([#1659](fluentassertions/fluentassertions#1659)) - [`8ad5e52`](fluentassertions/fluentassertions@8ad5e52) Merge pull request [#1658](fluentassertions/fluentassertions#1658) from LennartKoot/async-ex-asserts - [`559e10b`](fluentassertions/fluentassertions@559e10b) Update release page - [`f69c87c`](fluentassertions/fluentassertions@f69c87c) Update approved api - [`1d6b2a3`](fluentassertions/fluentassertions@1d6b2a3) Add extensions method 'WithInnerExceptionExactly()' for async ExceptionAssert... - [`a977427`](fluentassertions/fluentassertions@a977427) Merge pull request [#1656](fluentassertions/fluentassertions#1656) from eNeRGy164/markdown-linting - [`d6ea6c1`](fluentassertions/fluentassertions@d6ea6c1) Update header style and add additional installation path - [`75bfd96`](fluentassertions/fluentassertions@75bfd96) Correct white-space in documentation - Additional commits viewable in [compare view](fluentassertions/fluentassertions@5.10.3...6.1.0) </details> <br /> Reviewed-on: https://gitea.dysnomia.studio/elanis/portfolio/pulls/8 Co-authored-by: elanis <[email protected]> Co-committed-by: elanis <[email protected]>
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 5.10.3 to 6.1.0. <details> <summary>Release notes</summary> *Sourced from [FluentAssertions's releases](https://github.com/fluentassertions/fluentassertions/releases).* > ## 6.1.0 > [Release Notes](https://fluentassertions.com/releases/[#610](https://github.com/fluentassertions/fluentassertions/issues/610)) > > ## 6.0.0 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/master/docs/_pages/releases.md#600) > > ## 6.0.0-beta.3 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/6.0.0-beta.3/docs/_pages/releases.md#600-beta-3) > > ## 6.0.0-beta.2 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-2) > > ## 6.0.0-beta.1 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-1) > > ## 6.0.0-alpha.2 > See [release notes](https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600-alpha-2). > > ## 6.0.0-alpha.1 > See https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600 </details> <details> <summary>Commits</summary> - [`ce07e5c`](fluentassertions/fluentassertions@ce07e5c) Bump the release number for 6.1 - [`577d555`](fluentassertions/fluentassertions@577d555) Resolved a significant performance degradation in `BeEquivalentTo` ([#1660](fluentassertions/fluentassertions#1660)) - [`a38a54a`](fluentassertions/fluentassertions@a38a54a) Fix small grammatical error ([#1659](fluentassertions/fluentassertions#1659)) - [`8ad5e52`](fluentassertions/fluentassertions@8ad5e52) Merge pull request [#1658](fluentassertions/fluentassertions#1658) from LennartKoot/async-ex-asserts - [`559e10b`](fluentassertions/fluentassertions@559e10b) Update release page - [`f69c87c`](fluentassertions/fluentassertions@f69c87c) Update approved api - [`1d6b2a3`](fluentassertions/fluentassertions@1d6b2a3) Add extensions method 'WithInnerExceptionExactly()' for async ExceptionAssert... - [`a977427`](fluentassertions/fluentassertions@a977427) Merge pull request [#1656](fluentassertions/fluentassertions#1656) from eNeRGy164/markdown-linting - [`d6ea6c1`](fluentassertions/fluentassertions@d6ea6c1) Update header style and add additional installation path - [`75bfd96`](fluentassertions/fluentassertions@75bfd96) Correct white-space in documentation - Additional commits viewable in [compare view](fluentassertions/fluentassertions@5.10.3...6.1.0) </details> <br /> Co-authored-by: Elanis <[email protected]> Reviewed-on: https://gitea.dysnomia.studio/elanis/dehash-me/pulls/14 Co-authored-by: elanis <[email protected]> Co-committed-by: elanis <[email protected]>
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 5.10.3 to 6.1.0. <details> <summary>Release notes</summary> *Sourced from [FluentAssertions's releases](https://github.com/fluentassertions/fluentassertions/releases).* > ## 6.1.0 > [Release Notes](https://fluentassertions.com/releases/[#610](https://github.com/fluentassertions/fluentassertions/issues/610)) > > ## 6.0.0 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/master/docs/_pages/releases.md#600) > > ## 6.0.0-beta.3 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/6.0.0-beta.3/docs/_pages/releases.md#600-beta-3) > > ## 6.0.0-beta.2 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-2) > > ## 6.0.0-beta.1 > [Release Notes](https://github.com/fluentassertions/fluentassertions/blob/release-6.0/docs/_pages/releases.md#600-beta-1) > > ## 6.0.0-alpha.2 > See [release notes](https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600-alpha-2). > > ## 6.0.0-alpha.1 > See https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md#600 </details> <details> <summary>Commits</summary> - [`ce07e5c`](fluentassertions/fluentassertions@ce07e5c) Bump the release number for 6.1 - [`577d555`](fluentassertions/fluentassertions@577d555) Resolved a significant performance degradation in `BeEquivalentTo` ([#1660](fluentassertions/fluentassertions#1660)) - [`a38a54a`](fluentassertions/fluentassertions@a38a54a) Fix small grammatical error ([#1659](fluentassertions/fluentassertions#1659)) - [`8ad5e52`](fluentassertions/fluentassertions@8ad5e52) Merge pull request [#1658](fluentassertions/fluentassertions#1658) from LennartKoot/async-ex-asserts - [`559e10b`](fluentassertions/fluentassertions@559e10b) Update release page - [`f69c87c`](fluentassertions/fluentassertions@f69c87c) Update approved api - [`1d6b2a3`](fluentassertions/fluentassertions@1d6b2a3) Add extensions method 'WithInnerExceptionExactly()' for async ExceptionAssert... - [`a977427`](fluentassertions/fluentassertions@a977427) Merge pull request [#1656](fluentassertions/fluentassertions#1656) from eNeRGy164/markdown-linting - [`d6ea6c1`](fluentassertions/fluentassertions@d6ea6c1) Update header style and add additional installation path - [`75bfd96`](fluentassertions/fluentassertions@75bfd96) Correct white-space in documentation - Additional commits viewable in [compare view](fluentassertions/fluentassertions@5.10.3...6.1.0) </details> <br /> Co-authored-by: Elanis <[email protected]> Reviewed-on: https://gitea.dysnomia.studio/elanis/dysnomia-website/pulls/11 Co-authored-by: elanis <[email protected]> Co-committed-by: elanis <[email protected]>
This was caused by the increased dependency on the CallerIdentifier, and the additional work caller identification takes in v6.
#1657