-
Notifications
You must be signed in to change notification settings - Fork 733
IgnoreCyclicReferences in BeEquivalentTo now works with ComparingByMembers #1708
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
Conversation
7a3fd27 to
aaba81c
Compare
aaba81c to
81ca9a3
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.
While correctness is more important that speed, I'm curious what effect deletion of the cache has.
I thought about that, but the |
|
I added the caching of using System.Collections.Generic;
using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Jobs;
using FluentAssertions;
namespace Benchmarks
{
[MemoryDiagnoser]
[SimpleJob(RuntimeMoniker.Net472)]
[SimpleJob(RuntimeMoniker.Net50)]
public class PR817
{
public class Complex
{
public int A { get; set; }
public Complex B { get; set; }
}
private static Complex CreateComplex(int i)
{
if (i == 0)
{
return new Complex();
}
return new Complex
{
A = i,
B = CreateComplex(i - 1)
};
}
[Params(1, 10, 100, 500)]
public int N { get; set; }
[Params(1, 2, 6)]
public int Depth { get; set; }
[GlobalSetup]
public void GlobalSetup()
{
list = Enumerable.Range(0, N).Select(_ => CreateComplex(Depth)).ToList();
list2 = Enumerable.Range(0, N).Select(_ => CreateComplex(Depth)).ToList();
}
private List<Complex> list;
private List<Complex> list2;
[Benchmark]
public void BeEquivalentTo()
{
list.Should().BeEquivalentTo(list2);
}
}
}Before - 9190b26
After - b6bab8f
Relative + absolute diffs
ConclusionSo across time and GC for this benchmark, there's a 4-10% regression. |
|
While profiling the 400 equivalency specs using dotTrace (and Rider's integrated profiler), I get this: So it seems we have several opportunities:
|
|
I'll create a follow-up PR |


When an object graph implementing value semantics (by overriding
Equals) contains cyclic references, theIgnoringCyclicReferencesoption did not work.Fixes #1370