jodydonetti
commented
3 months ago Hi @MarkCiliaVincenti , good catch!
I was not sure about the perf though, meaning how much it would be faster on get VS how slower the initial setup would be, so I did some benchmarks which I'd like to share.
Get
For Get/TryGet operations (which are more frequent than the initial setup) we can see the same allocations (ofcourse), but around -20% cpu time: nothing monumental, sure, but still quite nice.
| Method | CachesCount | Rounds | Mean | Error | StdDev | P95 | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
|---|---|---|---|---|---|---|---|---|---|---|---|
| Dict | 1 | 1 | 25.53 ns | 0.193 ns | 0.171 ns | 25.74 ns | 1.00 | 0.00 | 0.0032 | 40 B | 1.00 |
| FrozenDict | 1 | 1 | 21.61 ns | 0.438 ns | 0.431 ns | 22.36 ns | 0.85 | 0.02 | 0.0032 | 40 B | 1.00 |
| Dict | 1 | 50 | 1,187.55 ns | 16.320 ns | 15.266 ns | 1,207.00 ns | 1.00 | 0.00 | 0.1583 | 2000 B | 1.00 |
| FrozenDict | 1 | 50 | 1,025.46 ns | 18.075 ns | 16.023 ns | 1,053.19 ns | 0.86 | 0.02 | 0.1583 | 2000 B | 1.00 |
| Dict | 10 | 1 | 265.67 ns | 4.642 ns | 4.115 ns | 272.17 ns | 1.00 | 0.00 | 0.0315 | 400 B | 1.00 |
| FrozenDict | 10 | 1 | 237.32 ns | 3.382 ns | 3.163 ns | 241.99 ns | 0.89 | 0.02 | 0.0317 | 400 B | 1.00 |
| Dict | 10 | 50 | 13,233.85 ns | 153.521 ns | 136.092 ns | 13,426.19 ns | 1.00 | 0.00 | 1.5869 | 20000 B | 1.00 |
| FrozenDict | 10 | 50 | 10,680.41 ns | 150.454 ns | 140.734 ns | 10,842.57 ns | 0.81 | 0.02 | 1.5869 | 20000 B | 1.00 |
| Dict | 50 | 1 | 1,345.35 ns | 10.495 ns | 9.304 ns | 1,361.36 ns | 1.00 | 0.00 | 0.1583 | 2000 B | 1.00 |
| FrozenDict | 50 | 1 | 1,477.01 ns | 28.189 ns | 30.162 ns | 1,535.61 ns | 1.10 | 0.02 | 0.1583 | 2000 B | 1.00 |
| Dict | 50 | 50 | 69,932.06 ns | 1,326.372 ns | 1,527.452 ns | 72,373.69 ns | 1.00 | 0.00 | 7.9346 | 100000 B | 1.00 |
| FrozenDict | 50 | 50 | 57,328.07 ns | 592.618 ns | 554.335 ns | 58,219.32 ns | 0.82 | 0.02 | 7.9346 | 100000 B | 1.00 |
Create (initial setup)
The setup is slower for the frozen one, basically 2 times slower:
| Method | CachesCount | Mean | Error | StdDev | P95 | Ratio | RatioSD | Gen0 | Gen1 | Allocated | Alloc Ratio |
|---|---|---|---|---|---|---|---|---|---|---|---|
| Dict | 1 | 65.46 ns | 1.323 ns | 1.358 ns | 67.23 ns | 1.00 | 0.00 | 0.0293 | - | 368 B | 1.00 |
| FrozenDict | 1 | 197.94 ns | 2.880 ns | 2.553 ns | 201.63 ns | 3.02 | 0.07 | 0.0598 | - | 752 B | 2.04 |
| Dict | 10 | 613.80 ns | 8.810 ns | 8.241 ns | 624.51 ns | 1.00 | 0.00 | 0.1993 | 0.0010 | 2512 B | 1.00 |
| FrozenDict | 10 | 1,437.42 ns | 13.200 ns | 12.347 ns | 1,453.61 ns | 2.34 | 0.04 | 0.3643 | 0.0019 | 4592 B | 1.83 |
| Dict | 50 | 3,098.58 ns | 31.471 ns | 29.438 ns | 3,145.35 ns | 1.00 | 0.00 | 0.9727 | 0.0381 | 12224 B | 1.00 |
| FrozenDict | 50 | 7,009.71 ns | 133.205 ns | 158.571 ns | 7,277.33 ns | 2.26 | 0.05 | 1.6251 | 0.0534 | 20448 B | 1.67 |
But we are talking about 4μs (note: micro, non milli) for 50 caches, so I think it doesn't really matter that much. Allocations are higher, but we are talking about very small numbers, and since get operations (way more frequent) would become faster, it seems like a nice win.
Additional dependency
To get this perf boost a new dependency on System.Collections.Immutable is needed, and this is the part I don't particularly like: an extra dependency for such a small improvement. On one hand we are talking about a core one, not a random 3rd party one, but on the other hand if we consider scenarios like AOT + tree shaking, I'm not that sure it is worth the added dependency.
Conclusions
As of now I'm for merging the PR and just take the extra dependency, it doesn't seem like a very big deal.
Just give me some time to better research the extra dependency and make up my mind about it.
Meanwhile, thanks for the PR, it is really appreciated!
MarkCiliaVincenti
This improves performance because FrozenDictionary is optimized for reading and is more performant than a normal Dictionary.