Use Int Array lookup for PolymorphicFormatterResolver #42
Conversation
| @@ -24,11 +24,12 @@ public sealed class MsgPackSerializer : Serializer | |||
| { | |||
| private readonly MsgPackSerializerSettings _settings; | |||
| private readonly IFormatterResolver _resolver; | |||
| public readonly MessagePackSerializerOptions _serializerOptions; | |||
| public readonly MessagePackSerializerOptions SerializerOptions; | |||
There was a problem hiding this comment.
It's the polite thing to do for consumers that need to do some ugly things. :)
|
|
||
| namespace Akka.Serialization.MessagePack.Resolvers | ||
| { | ||
| public class BackwardsCompatibleSurrogatedFormatterResolver : IFormatterResolver, IDoNotUsePolymorphicFormatter |
There was a problem hiding this comment.
No changes here, just cleaned up and moved to it's own file.
| if (value == null) | ||
| { | ||
| writer.WriteNil(); | ||
| } | ||
| else if (value.GetType() == typeof(T)) |
There was a problem hiding this comment.
This helps some optimizations and probably adds some needed safety (I do think Messagepack has it's own null protection but this helped perf since JITter knows .GetType() won't throw, and at best fixes a hazard I haven't seen previously.)
Semi related is that valuetype of default should be OK here, since null is not the same as default in generic context.
There may still be a better way to handle this (opt and more safety), open to suggestions!
| private readonly object _lockObj = new(); | ||
| public IntIndexedMessagePackFormatterDict(int initialSize) | ||
| { | ||
| _formatters = new IMessagePackFormatter[256]; |
There was a problem hiding this comment.
Not sure if a smaller size would be better here, I wanted to minimize re-allocs of the array but keep size 'fair'.
256 means it's gonna be 1Kib or 2Kib for the array to start.
| //Should it happen, it just means we trust `TryAdd` | ||
| //which does a full fence and then checks state. | ||
| //Shouldn't happen much and only on startup... | ||
| var f = _formatters; |
There was a problem hiding this comment.
Meant to leave a comment about 'capturing' the array here, OTOH I left enough exposition where hopefully context is provided?
| @@ -121,7 +122,9 @@ public static IFormatterResolver LoadFormatterResolverByType(Type type, Extended | |||
| LoadFormatterResolverByType(t, system)) | |||
| .Concat(new[] | |||
| { | |||
| #if SERIALIZATION | |||
There was a problem hiding this comment.
IDK how these crept back in, I can remove?
Changes
Use optimistic lookup Int Array instead of Concurrentdictionary for
PolymorphicFormatterResolver.No wire or format changes, internal only.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
Benchmarks
I don't have numbers handy but this results in a >5% improvement in serialization benchmarks (due to lack of volatile reads or barriers on happy path and unsafe magic). Can produce receipts if needed. <3