CA1836 Prefer IsEmpty over Count#7185
Conversation
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
| // So keeping that map here | ||
| var originalLocations = new Dictionary<EvaluationLocation, EvaluationLocation>(EvaluationLocationIdAgnosticComparer.Singleton); | ||
|
|
||
| while (!_profiledResults.IsEmpty) |
There was a problem hiding this comment.
@Forgind when we were talking about this did we talk about .Any() possibly being worse than ! .IsEmpty because the former is an extension method that might have higher overhead?
| Method | ActuallyEmpty | Mean | Error | StdDev | Allocated |
|---|---|---|---|---|---|
| IsEmpty | False | 2.484 ns | 0.0534 ns | 0.0473 ns | - |
| Any | False | 1,361.709 ns | 5.7049 ns | 5.0573 ns | - |
| IsEmpty | True | 178.694 ns | 1.1384 ns | 1.0649 ns | - |
| Any | True | 177.720 ns | 1.7098 ns | 1.5993 ns | - |
using System;
using System.Collections.Concurrent;
using System.Linq;
using BenchmarkDotNet;
using BenchmarkDotNet.Attributes;
namespace any_count;
[MemoryDiagnoser]
public class Benchmarks
{
public static ConcurrentDictionary<string, string> emptyDictionary = new ConcurrentDictionary<string, string>();
public static ConcurrentDictionary<string, string> nonEmptyDictionary = new ConcurrentDictionary<string, string>();
static Benchmarks()
{
for (int i = 0; i < 100; i++)
{
nonEmptyDictionary.TryAdd(i.ToString(), i.ToString());
}
}
[Params(true, false)]
public bool ActuallyEmpty;
[Benchmark]
public bool IsEmpty() => (ActuallyEmpty? emptyDictionary : nonEmptyDictionary).IsEmpty;
[Benchmark]
public bool Any() => (ActuallyEmpty? emptyDictionary : nonEmptyDictionary).Any();
}There was a problem hiding this comment.
@Forgind when we were talking about this did we talk about
.Any()possibly being worse than! .IsEmptybecause the former is an extension method that might have higher overhead?
Oh, I'm sorry; I didn't add a new comment after that part, and it slipped my mind. So do you think it's worth replacing .Any() with !.IsEmpty? (If so, sorry elachlan...)
There was a problem hiding this comment.
@Forgind when we were talking about this did we talk about
.Any()possibly being worse than! .IsEmptybecause the former is an extension method that might have higher overhead?Oh, I'm sorry; I didn't add a new comment after that part, and it slipped my mind. So do you think it's worth replacing .Any() with !.IsEmpty? (If so, sorry elachlan...)
I am happy to. I will make another PR and replace all any calls where I can. That time difference is massive.
Relates to #7174