-
-
Notifications
You must be signed in to change notification settings - Fork 673
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
General Unsafe Formatters #1053
General Unsafe Formatters #1053
Conversation
This (de)serializes struct T where T : unmanaged directly and unsafely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing, again. :)
I like the idea, and Experimental seems like reasonable place to start with this. My main concern is the new attribute, as I explain below.
src/MessagePack.Experimental/UnsafeBlittableFormatter/DynamicUnsafeUnmanagedStructResolver.cs
Outdated
Show resolved
Hide resolved
src/MessagePack.Experimental/UnsafeBlittableFormatter/UnsafeUnmanagedStructArrayFormatter.cs
Outdated
Show resolved
Hide resolved
|
||
namespace MessagePack.Formatters | ||
{ | ||
public sealed unsafe class UnsafeUnmanagedStructArrayFormatter<T> : IMessagePackFormatter<T[]?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any value to defining this as Memory<T>
or Span<T>
or their readonly counterparts instead of requiring an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Span<T>
can not be the generic type argument.
I made Memory<T>
, ReadOnlyMemory<T>
and ReadOnlySequence<T>
formatters.
src/MessagePack.Experimental/UnsafeBlittableFormatter/UnsafeUnmanagedStructFormatter.cs
Outdated
Show resolved
Hide resolved
Thank you for your review. Ok, I will remove the resolver, the attribute and ExtensionTypeCode(50/51). Done. |
Make constructor public and extension type parameter. Rename folder UnsafeBlittableFormatter to UnsafeUnmanagedStructFormatter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. @neuecc do you want to take a look? All the changes are in Experimental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this same(similar) as https://github.com/Cysharp/MagicOnion/blob/master/src/MagicOnion/UnsafeDirectBlitResolver.cs that before I wrote?
Injecting ExtCode from the outside is a good idea.
OK.
Wow, very similar! |
Abstract
There are some unsafe resolvers such as
UnsafeBlitResolver
.Such unsafe formatters are affected by the endian and struct layout.
This Pull Request offers the general way to serialize/deserialize the unmanaged struct by copying the binary presentation.
Usage
Add
UnsafeUnmanagedStructAttribute
to the module to specify which type the programmer wants to serialize/deserialize unsafely.DynamicUnsafeUnmanagedStructResolver
generates unsafe formatter dynamically.Performance