-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-4807: Support serialization of Memory<T> and ReadOnlyMemory<T> #1330
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
Conversation
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.
LGTM. Just a question about supported value types.
}; | ||
break; | ||
default: | ||
throw new NotSupportedException($"Not supported memory type {typeof(TItem)}. Only primitive numeric types are supported."); |
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.
Do we want to support decimal
, bool
, and char
as well? nuint
and nint
seem reasonable to omit.
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.
Yes we do, ideally we'd support as much primitive types as possible.
It's quick to add, thanks.
!(__isByte && representation == BsonType.Binary)) | ||
{ | ||
throw new ArgumentOutOfRangeException(nameof(representation)); | ||
} |
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.
We will want to eventually support serialization to BSON vector (BSON Binary subtype 9) for all value types. But that can be done in the context of a new ticket.
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.
Yup, it will be easy to add additional representation here.
} | ||
else | ||
{ | ||
var inner = e.InnerException.Should().BeOfType<TypeInitializationException>().Subject.InnerException; |
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.
I would prefer to avoid such exceptions. Should we move the failing code from static ctor to some helper method?
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.
LGTM
This PR introduces custom fast serialization path for
Memory<T>
andReadOnlyMemory<T>
.Deserializing
Memory
/ReadOnlyMemory
is x4.5 times faster than regular arrays on .net8, and x7 faster on .net6.The main performance gain is in the deserialization path, while the main serialization improvements are left out from this PR.
Overflow and truncations checks are not implement yet, as the use-cases this scenario is optimized for exclude such need.
Deserialization performance results:
.net8
.net6