Reduce ConcurrentDictionary.TryGetValue overhead

[stephentoub]

The first commit is purely style; I initially found myself fixing up things as I was making changes for perf, and wanted those cleanly separated.

The rest of the commits are primarily trying to make ConcurrentDictionary<>.TryGetValue faster, in particular by porting the kinds of changes that have been made in the past to Dictionary<>. The biggest impact comes when no comparer is specified, especially when the key is a value type.

Method	Toolchain	Size	Mean	Error	StdDev	Ratio
GetInts	master	1000	8.055 us	0.0475 us	0.0444 us	1.00
GetInts	pr	1000	4.345 us	0.0171 us	0.0152 us	0.54
GetTypes	master	1000	16.616 us	0.0647 us	0.0606 us	1.00
GetTypes	pr	1000	13.887 us	0.0476 us	0.0445 us	0.84
GetStrings	master	1000	37.802 us	0.0953 us	0.0892 us	1.00
GetStrings	pr	1000	35.077 us	0.1693 us	0.1501 us	0.93
GetStringsCustom	master	1000	43.375 us	0.1143 us	0.1069 us	1.00
GetStringsCustom	pr	1000	40.630 us	0.1518 us	0.1420 us	0.94

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Collections.Concurrent;
using System.Linq;

[MemoryDiagnoser]
public class Program
{
    static void Main(string[] args) => BenchmarkSwitcher.FromAssemblies(new[] { typeof(Program).Assembly }).Run(args);

    private ConcurrentDictionary<int, int> _int32Dictionary = new ConcurrentDictionary<int, int>();
    private ConcurrentDictionary<string, string> _stringDictionary = new ConcurrentDictionary<string, string>();
    private ConcurrentDictionary<Type, string> _typeDictionary = new ConcurrentDictionary<Type, string>();
    private ConcurrentDictionary<string, string> _stringCustomDictionary = new ConcurrentDictionary<string, string>(StringComparer.OrdinalIgnoreCase);
    private string[] _stringKeys;
    private Type[] _typeKeys;

    [GlobalSetup]
    public void Setup()
    {
        Type[] types = typeof(object).Assembly.GetTypes();

        for (int i = 0; i < Size; i++)
        {
            _int32Dictionary.TryAdd(i, i);

            string str = Guid.NewGuid().ToString("N");
            _stringDictionary.TryAdd(str, str);
            _stringCustomDictionary.TryAdd(str, str);
            _typeDictionary.TryAdd(types[i], str);
        }

        _stringKeys = _stringDictionary.Keys.ToArray();
        _typeKeys = _typeDictionary.Keys.ToArray();
    }

    [Params(1_000)]
    public int Size { get; set; }

    [Benchmark]
    public int GetInts()
    {
        ConcurrentDictionary<int, int> d = _int32Dictionary;
        int count = 0;
        for (int i = 0; i < Size; i++)
        {
            if (d.TryGetValue(i, out _))
                count++;
        }
        return count;
    }

    [Benchmark]
    public int GetTypes()
    {
        ConcurrentDictionary<Type, string> d = _typeDictionary;
        int count = 0;
        foreach (Type key in _typeKeys)
        {
            if (d.TryGetValue(key, out _))
                count++;
        }
        return count;
    }

    [Benchmark]
    public int GetStrings()
    {
        ConcurrentDictionary<string, string> d = _stringDictionary;
        int count = 0;
        foreach (string key in _stringKeys)
        {
            if (d.TryGetValue(key, out _))
                count++;
        }
        return count;
    }

    [Benchmark]
    public int GetStringsCustom()
    {
        ConcurrentDictionary<string, string> d = _stringCustomDictionary;
        int count = 0;
        foreach (string key in _stringKeys)
        {
            if (d.TryGetValue(key, out _))
                count++;
        }
        return count;
    }
}

cc: @eiriktsarpalis, @benaadams, @AntonLapounov, @GrabYourPitchforks

[ghost]

Tagging subscribers to this area: @eiriktsarpalis
Notify danmosemsft if you want to be subscribed.

[benaadams]

That's a really nice clean up :)

[danmoseley]

#1989 proposed to use FastMod in HashSet, but we bailed on the PR because we could not use #if BIT64 since it's not in corelib. But I see you bypassed that problem by doing a runtime check against IntPtr length. Coudl we do that in HashSet too?

[edit] Does the JIT treat IntPtr.Size as a constant and elide dead codepaths? If so, then it has no real runtime penalty.

[saucecontrol]

Does the JIT treat IntPtr.Size as a constant and elide dead codepaths?

It sure does :)

[danmoseley]

@saucecontrol I guess someone could revive #2143 then. That would be a perf win in HashSet.

[stephentoub]

I guess someone could revive #2143 then

I can take care of it.

[drieseng]

@stephentoub Why did you choose to duplicate the logic that is in TryGetValueInternal(...)? Is this worth a comment?

[stephentoub]

Do you mean the duplication between TryGetValue and TryGetValueInternal? The difference is that TryGetValueInternal is used when we already have a hashcode, whereas TryGetValue is used when we don't and need to compute one. In order to compute one, I'd need to go through the same decision tree that TryGetValueInternal does, which means we'd be doing it twice. Rather than paying to do it twice, I duplicated the code, as TryGetValue is in most cases the hot path / most important to be fast member of ConcurrentDictionary.