Serialize Type instances without requiring registration #2952
Conversation
ReubenBond
changed the title
| /// <summary> | ||
| /// Cached version of <see cref="dictionary"/>. | ||
| /// </summary> | ||
| private volatile Dictionary<TKey, TValue> readCache; |
There was a problem hiding this comment.
Is there a real need for volatile here? Just want to reduce the complexity if it's not.
Because it looks like if you remove volatile, you still can read your writes, and if another thread wants to access it, it probably doesn't matter that the cache was stale (otherwise more attention needs to be paid to when you invalidate the cache, which in this case it looks like is after you modify it, so there are still chances that the cache is stale when read from another thread, even with the volatile).
Or am I missing some edge case?
There was a problem hiding this comment.
I'll remove it, it's not needed.
There was a problem hiding this comment.
I fixed it, squashed, and force-pushed so that this can be merged as 3 commits
| } | ||
|
|
||
| internal static object DeserializeType(Type expected, IDeserializationContext context) | ||
| { | ||
| return context.SerializationManager.ResolveTypeName(context.StreamReader.ReadString()); | ||
| return Type.GetType(context.StreamReader.ReadString()); |
There was a problem hiding this comment.
I assume you chose to use Type.GetType as opposed to TypeUtils.ResolveType since the type is always qualified with the assembly name, hence assembly loading is not an issue?
ReubenBond
force-pushed
the
fix-type-name-formatting
branch
from
4ac050a to
c931e4a
Compare
|
LGTM! |
Currently, Types need to be registered by
SerializationManager.FindSerializationInfo(..)before they can be serialized over the wire.This PR removes that requirement.
Includes a utility class,
RuntimeTypeNameFormatterwhich takes aTypeand returns a string suitable for use withType.GetTypewhich does not include assembly metadata other than assembly name.If it's preferred, I can remove the
CachedReadConcurrentDictionary. In my benchmarks I find that it's significantly better than eitherDictionary+lockorConcurrentDictionaryfor our use-cases.This can be merged without squashing.