Skip to content

Conversation

@dennisdoomen
Copy link
Member

This was caused by the increased dependency on the CallerIdentifier, and the additional work caller identification takes in v6.

#1657

@dennisdoomen dennisdoomen force-pushed the Fix/CallerIdentifierPerformance branch from dc2fb5b to e008609 Compare August 22, 2021 12:27
@dennisdoomen dennisdoomen requested a review from jnyrup August 22, 2021 12:35
Copy link
Member

@jnyrup jnyrup left a 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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed in pipeline

Copy link
Member Author

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.
@dennisdoomen dennisdoomen force-pushed the Fix/CallerIdentifierPerformance branch from e008609 to c3d260a Compare August 22, 2021 13:49
@dennisdoomen
Copy link
Member Author

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.

@dennisdoomen dennisdoomen merged commit 577d555 into fluentassertions:master Aug 22, 2021
@dennisdoomen dennisdoomen deleted the Fix/CallerIdentifierPerformance branch August 22, 2021 15:36
@jnyrup jnyrup mentioned this pull request Aug 22, 2021
@lg2de
Copy link
Contributor

lg2de commented Sep 1, 2021

@jnyrup, you created a benchmark but did not added it to the solution?
Or was THIS benchmark already existing?

@jnyrup
Copy link
Member

jnyrup commented Sep 1, 2021

@lg2de I used the example in #1657 for my benchmark, but didn't add it.
Good point about adding it - I'll do that.

@jnyrup
Copy link
Member

jnyrup commented Sep 5, 2021

I was just about to add the benchmarks, but seems my former self already did that in #1661.

Copy link

@mdentremont mdentremont left a 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!

Elanis pushed a commit to Elanis/portfolio that referenced this pull request Dec 13, 2022
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]>
Elanis added a commit to Dysnomia-Studio/dehash-me that referenced this pull request Mar 12, 2023
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]>
Elanis added a commit to Dysnomia-Studio/dysnomia-website that referenced this pull request Jul 14, 2023
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants