IdentityServer icon indicating copy to clipboard operation
IdentityServer copied to clipboard

remove unnecessary distinct method

Open testfirstcoder opened this issue 1 year ago • 3 comments

Enumerable.Union method already excludes duplicates.

Returns An IEnumerable<T> that contains the elements from both input sequences, excluding duplicates.


IEnumerable<string> first = ["a", "b"];
IEnumerable<string> second = ["a", "c"];

var union = first.Union(second); // => [a, b, c]

var unionDistinct = first.Union(second).Distinct(); // => [a, b, c]

Debug.Assert(union.SequenceEqual(unionDistinct));

Some performance improvement:

[SimpleJob(RuntimeMoniker.Net80)]
[MemoryDiagnoser]
public class Benchmark
{
    private IEnumerable<string> list1 = ["a", "b"];
    private IEnumerable<string> list2 = ["a", "c"];

    [Benchmark]
    public void Union()
    {
        var union = list1.Union(list2);
    }

    [Benchmark]
    public void Union_Distinct()
    {
        var unionDistinct = list1.Union(list2).Distinct();
    }
}

void Main() => BenchmarkRunner.Run<Benchmark>();
// * Summary *

BenchmarkDotNet v0.13.12, Windows 10 (10.0.19045.4651/22H2/2022Update)
Intel Core i7-10700 CPU 2.90GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-preview.2.24157.14
  [Host] : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2

Job=.NET 8.0  Runtime=.NET 8.0  

| Method         | Mean     | Error    | StdDev   | Gen0   | Allocated |
|--------------- |---------:|---------:|---------:|-------:|----------:|
| Union          | 13.21 ns | 0.291 ns | 0.674 ns | 0.0086 |      72 B |
| Union_Distinct | 24.71 ns | 0.778 ns | 2.293 ns | 0.0162 |     136 B |
// * Hints *
Outliers
  Benchmark.Union: .NET 8.0 -> 5 outliers were removed (16.60 ns..17.69 ns)

// * Legends *
  Mean      : Arithmetic mean of all measurements
  Error     : Half of 99.9% confidence interval
  StdDev    : Standard deviation of all measurements
  Gen0      : GC Generation 0 collects per 1000 operations
  Allocated : Allocated memory per single operation (managed only, inclusive, 1KB = 1024B)
  1 ns      : 1 Nanosecond (0.000000001 sec)

testfirstcoder avatar Aug 12 '24 11:08 testfirstcoder

Thanks for the PR! Are you hoping to get this in fast to address a performance problem you're seeing in practice? Or would it be okay to wait for 7.1 (probably Dec/Jan)?

Also, I'd be curious to see if we can find more places like this. E.g., usages like Where(x=>SomeLogic).SingleOrDefault that could just use SingleOrDefault directly.

josephdecock avatar Aug 13 '24 15:08 josephdecock

It doesn't address any specific performance case now and can certainly wait.

testfirstcoder avatar Aug 16 '24 12:08 testfirstcoder

Ok, thanks! I'll keep this pr open for the time being as a note to self to dig around for similar usages.

josephdecock avatar Aug 17 '24 00:08 josephdecock

Looks like these are the only two Union's followed by a Distinct, and there is only one other call to Union anywhere in the repo - so nothing else to do here. Thanks very much!

josephdecock avatar Dec 05 '24 02:12 josephdecock