Skip to content

Conversation

elachlan
Copy link
Contributor

Relates to #7174

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jan 11, 2022
@Forgind Forgind merged commit 8132cae into dotnet:main Jan 11, 2022
// So keeping that map here
var originalLocations = new Dictionary<EvaluationLocation, EvaluationLocation>(EvaluationLocationIdAgnosticComparer.Singleton);

while (!_profiledResults.IsEmpty)
Copy link
Member

Choose a reason for hiding this comment

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

@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();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@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?

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...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants