Implement new SmallCapacityDictionary for dictionaries with a small amount of values.

[jkotalik]

For #31290.

Implements SmallCapacityDictionary, which uses a list for entries up to a threshold of 4, then falls back to an inner dictionary (using the inner dictionary for everything else).

Perf results showing better performance for add/get and allocates much less by default.

|               Method |       Mean |    Error |   StdDev |     Median |        Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------------- |-----------:|---------:|---------:|-----------:|------------:|------:|------:|------:|----------:|
|   OneValue_SmallDict |   923.5 ns | 168.6 ns | 492.0 ns |   700.0 ns | 1,082,872.9 |     - |     - |     - |      48 B |
|        OneValue_Dict | 1,377.6 ns | 195.1 ns | 569.2 ns | 1,100.0 ns |   725,925.9 |     - |     - |     - |      48 B |
| FourValues_SmallDict | 1,804.0 ns | 196.5 ns | 579.4 ns | 1,600.0 ns |   554,323.7 |     - |     - |     - |      48 B |
|      FourValues_Dict | 1,927.6 ns | 227.6 ns | 663.9 ns | 1,700.0 ns |   518,793.0 |     - |     - |     - |      48 B |
|            SmallDict | 2,116.9 ns | 290.8 ns | 805.7 ns | 1,800.0 ns |   472,399.2 |     - |     - |     - |     136 B |
|                 Dict | 2,358.9 ns | 223.3 ns | 622.6 ns | 2,050.0 ns |   423,928.4 |     - |     - |     - |     264 B |

Needs unit tests and to figure out where we want to put this.

[jkotalik]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[jkotalik]

Going to avoid the struct portion of this change in this PR for now (correctness issues in some down stack signalr tests). Will revisit in the future.

[jkotalik]

Hm, I wonder if we could add a mode that make it so setting doesn't check if the item exists before. All sets need to check all values before inserting, may some sort of unsafe thing could be useful.

[gfoidl]

Looked at perf again. On my machine (win-x64) it shows improvements.

Some additional notes:

• Dictionary<TKey, TValue> special cases type string, and uses a NonRandomizedStringEqualityComparer. Should this be done here too?

• if AdaptiveCapacityDictionary should be used for value types too, one could use of JIT's ability to devirtualize EqualityComparer<TValue>.Default and get nice perf-boost. Note: for shared generics (like string keys) this won't happen right now.

[gfoidl]

Suggested change

	if (comparer is not null && comparer != EqualityComparer<TKey>.Default) // first check for null to avoid forcing default comparer instantiation unnecessarily
	if (comparer is not null)

If it's the default comparer it's assigned anyway, so this branch can be simplified?

[gfoidl]

These benchmarks should return a value to prevent dead-code optimization / to be on the safe side.

[gfoidl]

Suggested change

	_dictionaryStorage![key] = value;
	return;
	_dictionaryStorage![key] = value;

[gfoidl]

A matter of taste:

Suggested change

	public int Count
	{
	get
	{
	if (_dictionaryStorage != null)
	{
	return _dictionaryStorage.Count;
	}
	return _count;
	}
	}
	public int Count => _dictionaryStorage != null ? _dictionaryStorage.Count : _count;

[gfoidl]

Suggested change

	public IEqualityComparer<TKey> Comparer
	{
	get
	{
	return _comparer ?? EqualityComparer<TKey>.Default;
	}
	}
	public IEqualityComparer<TKey> Comparer => _comparer;

_comparer won't be null, every ctor-path assigns a value.

[gfoidl]

Suggested change

	private bool ContainsKeyArray(TKey key)
	{
	Debug.Assert(_dictionaryStorage == null);
	Debug.Assert(_arrayStorage != null);
	var array = _arrayStorage;
	var count = _count;
	if ((uint)count <= (uint)array.Length)
	{
	for (var i = 0; i < count; i++)
	{
	if (_comparer.Equals(array[i].Key, key))
	{
	return true;
	}
	}
	}
	return false;
	}
	private bool ContainsKeyArray(TKey key) => TryFindItem(key, out _);

I verifyed the codegen and perf. Looks good that way, as the JIT eliminates the dead unnecessary code due to inlining.

[gfoidl]

Here I'd be more aggressive and prioritize the small sizes.

Suggested change

	if (_dictionaryStorage != null)
	{
	return _dictionaryStorage.ContainsKey(key);
	}
	return ContainsKeyCore(key);
	if (_dictionaryStorage is null)
	{
	return ContainsKeyArray(key);
	}
	return _dictionaryStorage.ContainsKey(key);

For codegen the small sizes are in the hot-path, whilst the fallback is moved down to a colder region.
Additional the cpu's branch predictor prefers the fall-through path when there's not history.

In the micro-benchmarks this effect won't show up, as the branch predictor has history and the I-caches are hot. The benefit should be in real-world usage -- but honestly, it's mini.

The method itself will inline, so aggressive-inline isn't necessary, but you can add if you want. I don't know...

[gfoidl]

Should the logic for ContainsKeyArray go here or remove ContainsKeyCore?

It's just about putting less work to the JIT and it's inliner (budget). Contains will inline in the code-path I checked, but with a deeper call-hierarchy this might hit JIT's inlining limit (I don't know the current limit from top of my head).

[gfoidl]

This comment is outdated, and maybe not true either.
E.g. Remove calls this method, but doesn't call EnsureCapacity.

[gfoidl]

Same ways as TryFindItem:

Suggested change

	var array = _arrayStorage;
	var count = _count;
	for (var i = 0; i < count; i++)
	{
	if (_comparer.Equals(array[i].Key, key))
	{
	return i;
	}
	}
	if (_count > 0)
	{
	for (var i = 0; i < ArrayStorageSpan.Length; ++i)
	{
	if (_comparer.Equals(ArrayStorageSpan[i].Key, key))
	{
	return i;
	}
	}
	}

[Tratcher]

I'd want to find at least one more consumer to verify the functionality before merging it.

[jkotalik]

I'd want to find at least one more consumer to verify the functionality before merging it.

Care to help me find one? What do you think of QueryCollection? https://github.com/dotnet/aspnetcore/blob/main/src/Http/Http/src/QueryCollection.cs

[jkotalik]

|                      Method |       Mean |     Error |    StdDev |     Median |        Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------- |-----------:|----------:|----------:|-----------:|------------:|------:|------:|------:|----------:|
|          OneValue_SmallDict |   553.0 ns |  50.78 ns | 135.55 ns |   500.0 ns | 1,808,278.9 |     - |     - |     - |      48 B |
|               OneValue_Dict | 1,002.4 ns |  68.48 ns | 185.15 ns |   900.0 ns |   997,652.6 |     - |     - |     - |      48 B |
|      OneValue_SmallDict_Set |   542.1 ns |  81.75 ns | 234.56 ns |   400.0 ns | 1,844,660.2 |     - |     - |     - |      48 B |
|           OneValue_Dict_Set |   992.2 ns | 138.49 ns | 386.06 ns |   800.0 ns | 1,007,838.7 |     - |     - |     - |      48 B |
|      OneValue_SmallDict_Get |   120.0 ns |  29.03 ns |  83.28 ns |   100.0 ns | 8,333,333.3 |     - |     - |     - |      48 B |
|           OneValue_Dict_Get |   621.6 ns |  43.85 ns | 120.77 ns |   600.0 ns | 1,608,775.1 |     - |     - |     - |      48 B |
|        FourValues_SmallDict | 1,214.3 ns |  82.56 ns | 221.79 ns | 1,200.0 ns |   823,529.4 |     - |     - |     - |      48 B |
|        FiveValues_SmallDict | 1,547.1 ns | 138.80 ns | 379.98 ns | 1,600.0 ns |   646,359.6 |     - |     - |     - |      48 B |
|         SixValues_SmallDict | 1,654.0 ns |  81.27 ns | 222.46 ns | 1,700.0 ns |   604,586.5 |     - |     - |     - |      48 B |
|       SevenValues_SmallDict | 1,996.4 ns | 134.50 ns | 359.01 ns | 2,000.0 ns |   500,905.3 |     - |     - |     - |      48 B |
|       EightValues_SmallDict | 2,417.0 ns | 140.26 ns | 386.32 ns | 2,300.0 ns |   413,728.3 |     - |     - |     - |      48 B |
|        NineValues_SmallDict | 2,633.3 ns | 188.42 ns | 506.19 ns | 2,700.0 ns |   379,746.8 |     - |     - |     - |      48 B |
|         TenValues_SmallDict | 3,082.4 ns |  66.85 ns | 160.17 ns | 3,050.0 ns |   324,427.5 |     - |     - |     - |      48 B |
|             FourValues_Dict | 1,104.5 ns |  84.27 ns | 233.51 ns | 1,000.0 ns |   905,391.7 |     - |     - |     - |      48 B |
|             FiveValues_Dict | 1,305.6 ns | 134.80 ns | 375.77 ns | 1,200.0 ns |   765,957.4 |     - |     - |     - |      48 B |
|              SixValues_Dict | 1,201.7 ns | 106.35 ns | 294.69 ns | 1,150.0 ns |   832,164.6 |     - |     - |     - |      48 B |
|            SevenValues_Dict | 1,423.3 ns | 164.00 ns | 457.17 ns | 1,250.0 ns |   702,576.1 |     - |     - |     - |      96 B |
|            EightValues_Dict | 1,230.4 ns | 117.62 ns | 331.74 ns | 1,100.0 ns |   812,720.8 |     - |     - |     - |      48 B |
|             NineValues_Dict | 1,326.4 ns | 111.06 ns | 304.03 ns | 1,300.0 ns |   753,899.5 |     - |     - |     - |      48 B |
|              TenValues_Dict | 1,297.6 ns | 108.71 ns | 292.05 ns | 1,300.0 ns |   770,642.2 |     - |     - |     - |      48 B |
|     FourValues_SmallDictGet |   428.2 ns |  28.31 ns |  76.55 ns |   400.0 ns | 2,335,164.8 |     - |     - |     - |      48 B |
|     FiveValues_SmallDictGet |   502.2 ns |  42.86 ns | 120.87 ns |   450.0 ns | 1,991,342.0 |     - |     - |     - |      48 B |
|   SixValues_SmallDictGetGet |   593.1 ns |  59.69 ns | 163.39 ns |   500.0 ns | 1,686,046.5 |     - |     - |     - |      48 B |
| SevenValues_SmallDictGetGet |   487.8 ns |  44.94 ns | 125.27 ns |   500.0 ns | 2,050,113.9 |     - |     - |     - |      48 B |
|    EightValues_SmallDictGet |   549.4 ns |  42.88 ns | 118.82 ns |   500.0 ns | 1,820,040.9 |     - |     - |     - |      48 B |
|     NineValues_SmallDictGet |   548.8 ns |  39.82 ns | 106.98 ns |   500.0 ns | 1,822,125.8 |     - |     - |     - |      48 B |
|      TenValues_SmallDictGet |   560.4 ns |  50.96 ns | 142.89 ns |   500.0 ns | 1,784,313.7 |     - |     - |     - |      48 B |
|           TenValues_DictGet |   748.2 ns |  46.20 ns | 123.33 ns |   700.0 ns | 1,336,553.9 |     - |     - |     - |      48 B |
|                   SmallDict | 1,645.8 ns | 238.26 ns | 635.97 ns | 1,500.0 ns |   607,613.5 |     - |     - |     - |     136 B |
|                        Dict | 2,095.0 ns | 168.05 ns | 439.76 ns | 1,900.0 ns |   477,327.0 |     - |     - |     - |     264 B |
|               SmallDictFour | 1,648.3 ns | 176.94 ns | 484.37 ns | 1,500.0 ns |   606,694.6 |     - |     - |     - |     184 B |
|                    DictFour | 2,020.2 ns |  77.69 ns | 208.71 ns | 2,000.0 ns |   494,991.2 |     - |     - |     - |     376 B |
|                SmallDictTen | 1,924.1 ns | 356.09 ns | 974.79 ns | 1,500.0 ns |   519,713.3 |     - |     - |     - |     280 B |
|                     DictTen | 2,034.5 ns | 102.31 ns | 274.85 ns | 2,000.0 ns |   491,515.5 |     - |     - |     - |     488 B |

[Tratcher]

What do you think of QueryCollection?

Yes, that's an interesting candidate.

[jkotalik]

Yes, that's an interesting candidate.

Let's do a separate PR. Worst case, it fails in the next PR and we revert/fix parts of this.