What if we did not use dictionaries?

[mattleibow]

Description of Change

What if we did not implement core handler mappers using a big dictionary? We know exactly what there will be at build time.

This has overhead:

• dictionary creation at runtime
• dictionary lookups
• delegate overheads

This PR attempts to investigate a possiblity using a giant switch statement.

Benchmarks on:

BenchmarkDotNet v0.13.10, macOS Sonoma 14.7.3 (23H417) [Darwin 23.6.0]
Apple M1 Pro, 1 CPU, 8 logical and 8 physical cores
.NET SDK 9.0.100
  [Host]     : .NET 9.0.0 (9.0.24.52809), Arm64 RyuJIT AdvSIMD
  DefaultJob : .NET 9.0.0 (9.0.24.52809), Arm64 RyuJIT AdvSIMD

BEFORE

With #28077, @albyrock87 made things are really fast:

| Method                    | Mean     | Error    | StdDev   | Gen0      | Allocated  |
|-------------------------- |---------:|---------:|---------:|----------:|-----------:|
| BenchmarkUpdateProperties | 61.61 ms | 0.224 ms | 0.187 ms | 4000.0000 | 25604194 B |
| BenchmarkUpdateProperty   | 18.14 ms | 0.044 ms | 0.037 ms |         - |       23 B |

// * Hints *
Outliers
  PropertyMapperBenchmarker.BenchmarkUpdateProperties: Default -> 2 outliers were removed (62.71 ms, 63.42 ms)
  PropertyMapperBenchmarker.BenchmarkUpdateProperty: Default   -> 2 outliers were removed (18.31 ms, 18.37 ms)

AFTER

However, that PR was too good and cached all the things. However, it cached the delegates which we don't even have. So I had to revert that and add a TryUpdateProperty instead. This meant that after the revert with the base code:

| Method                    | Mean      | Error    | StdDev   | Gen0      | Allocated  |
|-------------------------- |----------:|---------:|---------:|----------:|-----------:|
| BenchmarkUpdateProperties | 224.61 ms | 0.652 ms | 0.578 ms | 4666.6667 | 29604549 B |
| BenchmarkUpdateProperty   |  45.93 ms | 0.144 ms | 0.135 ms |         - |       67 B |

// * Hints *
Outliers
  PropertyMapperBenchmarker.BenchmarkUpdateProperties: Default -> 1 outlier  was  removed, 2 outliers were detected (223.49 ms, 226.00 ms)

Terrible, but if I then switch to the new mapper (assuming I did it right):

| Method                    | Mean      | Error    | StdDev   | Gen0      | Allocated  |
|-------------------------- |----------:|---------:|---------:|----------:|-----------:|
| BenchmarkUpdateProperties | 156.64 ms | 0.778 ms | 0.728 ms | 4500.0000 | 29604434 B |
| BenchmarkUpdateProperty   |  28.28 ms | 0.102 ms | 0.090 ms |         - |       23 B |

// * Hints *
Outliers
  PropertyMapperBenchmarker.BenchmarkUpdateProperty: Default -> 1 outlier  was  removed (28.80 ms)

This means that there is a large improvement just by reducing the lookups. If we were to replace all our implementations with a giant switch statement, we could see a large improvement in performance overall.

Issues Fixed

Fixes #

[mattleibow]

@albyrock87 you did some good work there, but do you think that this idea could make it better? Did you try an approach like this at all?

[albyrock87]

@mattleibow may you expand on this?

However, it cached the delegates which we don't even have.

I don't understand what use case was not covered.

[mattleibow]

I don't understand what use case was not covered.

Nothing actually. But theoretically not using Action<T, T> is faster. @jonathanpeppers what part is faster? AOT or just the runtime in general?

I think it is also the dictionary. So we construct a dictionary, create delegates/actions, insert into the dic, then have to lookup and execute - all of shich is slower than exeuting simple IL.

This PR is an idea to potentially remove dictionaries and delegates from the equation.

However, it cached the delegates which we don't even have.

You are caching the Action<IElementHandler, IElement> whic a pure method and big switch wont have. I forgot to push the last stuff.

On a separate note, I did see that you run SnapshotMappers() 3 times, 1 per field - could that be cached to get even faster results?

[albyrock87]

I don't understand what use case was not covered.

Nothing actually.

@mattleibow So why are you saying these two sentences?

So I had to revert that and add a TryUpdateProperty instead.
However, it cached the delegates which we don't even have.

What have I broken?

Regarding performance

I think it is also the dictionary. So we construct a dictionary, create delegates/actions, insert into the dic, then have to lookup and execute - all of which is slower than exeuting simple IL.

Looking at the numbers you reported it seems to me the switch is slower than my approach.

• On my machine the original test takes 167.67 ms
• On your it takes 224.61 ms
• Which makes my machine faster 0.7464939228

Now taking your final timings: 156.64 ms and divide by the multiplier we get 116.93 ms which is way slower than 61.61 ms.

On a separate note, I did see that you run SnapshotMappers() 3 times, 1 per field - could that be cached to get even faster results?

That method is invoked just once because it sets all the 3 fields.

Properties:

=> _updatePropertiesKeys ?? SnapshotMappers().UpdatePropertiesKeys;
=> _updatePropertiesMappers ?? SnapshotMappers().UpdatePropertiesMappers;
=> _cachedMappers ?? SnapshotMappers().CachedMappers;

SnapshotMappers

_updatePropertiesKeys = updatePropertiesKeys;
_updatePropertiesMappers = updatePropertiesMappers;
_cachedMappers = cachedMappers;

[mattleibow]

What have I broken?

Nothing. I am trying something totally different in where we don't use dictionaries with actions at all. Your optimizations were about optimizating the lists of actions and caching keys. My idea is to not do anything and just use a giant switch statement.

I had to revert becasue you made the snapshot code cache the actions, and this PR hopes to remove the need for the the list of actions and keys entirely and just be a direct call to the static method.

And about the perf yeah, I know it is slower than your work, mostly because I had to revert and it is all hacks right now. Just an idea. Once I have fixed my code a bit more, I will have to see if it is faster. But, the comparison after reverting shows that it is a fair bit faster than the original implemention, which confirms the hypothesis that a switch is faster than the dictionary scanning we did before.

So yeah, your code is way better than what we had before, but I am basically wanting to convert the updates to just be a call to Update, then switch and then execute the MapPropertyName directly.

[albyrock87]

I see. I was concerned I broke something:)

That said, I think your approach may give good results.

I've seen nested switch patterns done like this:

• switch(key[0])
  • if only one key starts with that char, we're good
  • else switch or convenient char on given index or use key.length

I think it should bring better performance.

[Dreamescaper]

I've seen nested switch patterns done like this:

• switch(key[0])

  • if only one key starts with that char, we're good
  • else switch or convenient char on given index or use key.length

That probably doesn't make much sense. C# compiler uses hash function for string switch-case. You can take a look what switch-case is translated into:
https://sharplab.io/#v2:CYLg1APgAgTADAWAFCwIzOQNwIYCcAEAdtgLYCm+AvPqjAMwB0AKgPYDKALrgJaEDmACgCUAbgxIAzgHduHAMYALAcXJCA3snxb8c7BIoAiWnQMhN2i1FQBOAQZbGDo8xa0AjXGWwBrMUgu6+vhGMAAspi6uVrb2tOHO/q7unj5+AXqGtACsEYlJ0XYOMDkJSclevpGBmTAAbLll+AWxdU5pZR4V7VrVwah0JmZ5UTaFtADsbZEWnalVGX2h4UONzUUAHFPDMymVw71GWTkrZWu01luNs3vpQUa19Sf5oy1wlx273ToLBvSD09ozgN3klrl8DmFlgCtGdQiZSh8uvM7sVjtCmi8HFl4V8dkj9j86o90Wdajj0WDkABfIA===

[albyrock87]

@Dreamescaper that's amazing! Thanks for sharing.

So at this point the only big improvement could be something like a FrozenDictionary, possibly generated at compile time.

For reference: dotnet/runtime#77891 @ "Source Generators"

[mattleibow]

These should be TryUpdateProperty.