Faster multi-element serialization for primitive types by pCYSl5EDgo · Pull Request #1872 · MessagePack-CSharp/MessagePack-CSharp · GitHub
Skip to content

Faster multi-element serialization for primitive types#1872

Merged
AArnott merged 7 commits into
MessagePack-CSharp:developfrom
pCYSl5EDgo:faster-multi-element-handling
Jul 18, 2024
Merged

Faster multi-element serialization for primitive types#1872
AArnott merged 7 commits into
MessagePack-CSharp:developfrom
pCYSl5EDgo:faster-multi-element-handling

Conversation

@pCYSl5EDgo

Copy link
Copy Markdown
Contributor

Rewrite of #1744

@pCYSl5EDgo pCYSl5EDgo changed the title Faster multi-element handling Faster multi-element serialization for primitive types Jul 4, 2024

@AArnott AArnott left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the hardware accelerated code is beyond me. But I have a couple questions at least.

Comment thread src/MessagePack/Internal/UnsafeRefSerializeHelper.cs
Comment thread src/MessagePack/Internal/UnsafeRefSerializeHelper.cs
Comment thread src/MessagePack/Internal/UnsafeRefSerializeHelper.cs Outdated
Comment thread src/MessagePack/Internal/UnsafeRefSerializeHelper.cs Outdated
@AArnott AArnott added this to the v3.0 milestone Jul 16, 2024

@AArnott AArnott left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The product changes look good. How are we looking for test coverage? In particular, given the 16-element boundary that is interesting, it would seem prudent to have tests that cover lists of various sizes including at and around that 16-length boundary. Do we already have such tests, and are they running on .NET 8 such that they'll cover your new code?

@pCYSl5EDgo

Copy link
Copy Markdown
Contributor Author

@AArnott

AArnott commented Jul 17, 2024

Copy link
Copy Markdown
Collaborator

There is an awful amount of noise in the failure logs, but there does appear to be a legitimate failure here.

@pCYSl5EDgo

pCYSl5EDgo commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

I've fixed bugs.

I have a question about bool[]? Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options).
Where should I place the simd-accelerated deserialization helper code?
Append it to UnsafeRefSerializeHelper.cs or write another file such as UnsafeRefDeserializeHelper.cs?
The simd-accelerated deserialization helper code does not share any code with the serialization helper methods.

@AArnott

AArnott commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Where should I place the simd-accelerated deserialization helper code?

Given the length of the existing file, my vote is create a second file.

@AArnott

AArnott commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Do you want me to merge this as-is, or do you want to add the deserialization optimization to it first?

@pCYSl5EDgo

pCYSl5EDgo commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

This was referenced Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants