Closed testfirstcoder closed 5 months ago
Hi @testfirstcoder ,
You raised a similar PR last week https://github.com/Particular/NServiceBus/pull/6969https://github.com/Particular/NServiceBus/pull/6969
That one was closed as not deduplicating, which this one also does not do.
What issue are you trying to fix with these changes?
Hi @mikeminutillo,
it's not a particular issue fixing but rather a performance related one.
[SimpleJob(RuntimeMoniker.Net80)]
[MemoryDiagnoser]
public class Benchmark
{
private int[] visitedNodes;
private const int NewNode = -1;
[Params(0, 1, 10)]
public int N;
public Benchmark() => visitedNodes = Enumerable.Range(start: 1, count: N).ToArray();
[Benchmark]
public void Union_ToArray()
{
int[] newVisitedNodes = visitedNodes.Union(new[] { NewNode }).ToArray();
}
[Benchmark]
public void Append_ToArray()
{
int[] newVisitedNodes = visitedNodes.Append(NewNode).ToArray();
}
[Benchmark]
public void Collection_Expression_Spread_Operator()
{
int[] newVisitedNodes = [.. visitedNodes, NewNode];
}
}
void Main() => BenchmarkRunner.Run<Benchmark>();
// Summary
BenchmarkDotNet v0.13.12, Windows 10 (10.0.19045.4170/22H2/2022Update) Intel Core i7-10700 CPU 2.90GHz, 1 CPU, 16 logical and 8 physical cores .NET SDK 8.0.202 [Host] : .NET 8.0.3 (8.0.324.11423), X64 RyuJIT AVX2
Job=.NET 8.0 Runtime=.NET 8.0
Method | N | Mean | Error | StdDev | Median | Gen0 | Allocated |
---|---|---|---|---|---|---|---|
Union_ToArray | 0 | 83.037 ns | 1.6542 ns | 1.6988 ns | 83.384 ns | 0.0401 | 336 B |
Append_ToArray | 0 | 47.313 ns | 0.8261 ns | 0.7727 ns | 47.171 ns | 0.0105 | 88 B |
Collection_Expression_Spread_Operator | 0 | 4.394 ns | 0.1457 ns | 0.4296 ns | 4.241 ns | 0.0038 | 32 B |
Union_ToArray | 1 | 79.278 ns | 1.4234 ns | 2.9077 ns | 79.304 ns | 0.0401 | 336 B |
Append_ToArray | 1 | 49.852 ns | 0.8911 ns | 1.1587 ns | 50.158 ns | 0.0105 | 88 B |
Collection_Expression_Spread_Operator | 1 | 4.527 ns | 0.1248 ns | 0.3622 ns | 4.508 ns | 0.0038 | 32 B |
Union_ToArray | 10 | 78.397 ns | 1.5664 ns | 2.3446 ns | 78.125 ns | 0.0401 | 336 B |
Append_ToArray | 10 | 46.757 ns | 0.9523 ns | 2.0087 ns | 46.258 ns | 0.0105 | 88 B |
Collection_Expression_Spread_Operator | 10 | 4.166 ns | 0.1129 ns | 0.3203 ns | 4.089 ns | 0.0038 | 32 B |
Is an implicit deduplication using Union
necessary? Could visitedNodes
have some duplicates?
There is an explicit duplication check here, isn't?
if (visitedNodes.Any(n => n == node))
{
return true;
}
Union
method