Make FindSerializerForType faster #2576
Conversation
|
Looks like DData types serializers have been resolved to |
| if (serializer == null) | ||
| throw new SerializationException($"Serializer not found for type {objectType.Name}"); | ||
|
|
||
| _serializerMap.Add(type, serializer); |
There was a problem hiding this comment.
Shouldn't that become threadsafe collection in presence of updates? In Scala ConcurrentHashMap is used.
There was a problem hiding this comment.
Also there will be duplicate exception if we first found object on line 251 and then trying to add it back. What the point of this btw?
There was a problem hiding this comment.
We will get an exception only if we remove all serializers from all configs
There was a problem hiding this comment.
I've changed it to ConcurrentDictionary
There was a problem hiding this comment.
Why not just change it to _serializerMap.Add[type] = serializer; ? Becomes an upsert that way.
| if (_serializerMap.ContainsKey(_objectType) && _objectType.IsAssignableFrom(type)) | ||
| return _serializerMap[_objectType]; | ||
| //do a final check for the "object" serializer | ||
| if (serializer == null && _serializerMap.ContainsKey(_objectType) && _objectType.IsAssignableFrom(type)) |
There was a problem hiding this comment.
Why we need to check that type is subclass of objec?
There was a problem hiding this comment.
lol.... it's literally checking to see if we're assignable from object ?
| /// </summary> | ||
| public class Information | ||
| internal class Information |
There was a problem hiding this comment.
Made it internal
| @@ -60,7 +56,7 @@ public static T SerializeWithTransport<T>(ActorSystem system, Address address, F | |||
|
|
|||
| private readonly Serializer _nullSerializer; | |||
|
|
|||
| private readonly Dictionary<Type, Serializer> _serializerMap = new Dictionary<Type, Serializer>(); | |||
| private readonly ConcurrentDictionary<Type, Serializer> _serializerMap = new ConcurrentDictionary<Type, Serializer>(); | |||
There was a problem hiding this comment.
Changed to ConcurrentDictionary
| /// <param name="serializerId">TBD</param> | ||
| /// <returns>TBD</returns> | ||
| public Serializer GetSerializerById(int serializerId) | ||
| internal Serializer GetSerializerById(int serializerId) |
There was a problem hiding this comment.
This one should be internal
| } | ||
| /// <param name="bytes">The array containing the serialized object</param> | ||
| /// <returns>The object contained in the array</returns> | ||
| public T FromBinary<T>(byte[] bytes) => (T)FromBinary(bytes, typeof(T)); |
There was a problem hiding this comment.
I think we should have this method
| /// </remarks> | ||
| /// <param name="type">TBD</param> | ||
| /// <returns>TBD</returns> | ||
| protected static string TypeQualifiedNameForManifest(Type type) |
There was a problem hiding this comment.
We have an extension method for that. We don't need this method anymore
Related #2566
I've reused some parts of Scala implementation
Before: 1 400 243 ops/s
After: 4 551 167 ops/s