Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API] [Wire Format] KnownTypeProvider need to be improved #287

Open
Arkatufus opened this issue Jan 18, 2022 · 1 comment
Open

[API] [Wire Format] KnownTypeProvider need to be improved #287

Arkatufus opened this issue Jan 18, 2022 · 1 comment
Labels

Comments

@Arkatufus
Copy link
Contributor

Arkatufus commented Jan 18, 2022

Moved from #275

As of now, KnownTypeProvider is just an array passed into Hyperion. Hyperion will then use the index of each type in the array as the type manifest in the serialized message. This would not be a problem in transient messages such as remote message with the caveat that both nodes uses the same known types array. If Hyperion is ever used as a persistence mechanism, this will be a forward compatibility problem as it will break if the known type array were to change anytime in the future.

Copied over from #275:

[ NOTE ] This part is a wire format breaking change, we might not implement this in a single PR, this will be implemented in a different PR in draft until we can nail a good wire format compatibility for it.

In SerializerOptions class:

  • IEnumerable<Type> knownType .ctor parameter should be changed to IDictionary<ushort, Type> knownTypes
  • Type[] KnownTypes should be changed to Dictionary<ushort, Type> KnownTypes
  • Changes would be made so that it is backward compatible with the old .ctor

In Serializer class:

  • ValueSerializer[] _knownValueSerializers should be changed to Dictionary<ushort, ValueSerializer> _knownValueSerializerMap

In SerializerSession:

  • _nextTypeId should be initialized with serializer.Options.KnownTypes.Max(kvp => kvp.Key) instead of array length
  • _trackedTypes should be changed to Dictionary<ushort, Type>

In DeserializerSession:

  • _offset should be initialized with serializer.Options.KnownTypes.Max(kvp => kvp.Key) instead of array length
  • _identifierToType should also be changed to `Dictionary<ushort, Type>

User known types ID will be limited from 0 to 32768 (half a ushort) because we need to make sure that there are enough leeway for the dynamic IDs, this will be checked during serializer initialization

Reasoning:
As of now value serializer manifest index is based on an array index of the knownTypes type array passed in the option, this is bad because manifest can only be appended, not added in the middle nor removed.

@Arkatufus
Copy link
Contributor Author

Will need to discuss the wire format and persistence repercussion of these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant