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

New Span-based serialization logic (followup for #7351) #7576

Merged

Conversation

jtattermusch
Copy link
Contributor

@jtattermusch jtattermusch commented May 29, 2020

Span-based parsing has been added by #7351 , this PR adds the serialization counterpart, which allows serialization to IBufferWriter (which is much more GC friendly).

Based on #7490

Update 2020-06-29:
The new benchmarks (as of commit 4f0afc7) are here: https://gist.github.com/jtattermusch/c9e5389a398f7c79710a9bf12aee215f
For both writing primitives and writing messages the performance is comparable (there are some significant improvement and some slight regressions) to the original state, which seems good enough (as this PR offers an API to serialize directly into a Span and/or to IBufferWriter, which are modern APIs allowing for much better GC efficiency).

@JamesNK
Copy link
Contributor

JamesNK commented Jun 2, 2020

Is IBufferWriter<byte> supported yet? It is referenced but at the moment all tests write to byte[] or Stream.

@jtattermusch
Copy link
Contributor Author

Is IBufferWriter<byte> supported yet? It is referenced but at the moment all tests write to byte[] or Stream.

I still need to expose the API and add tests, but vast majority of the internal implementation is already there.

@jtattermusch
Copy link
Contributor Author

With my few last commits (thanks @JamesNK for the ideas), the writing performance for messages now seems ~at par with the previous state (= no regressions) so I think this PR is ready for final review.
@haberman can you please review?

@haberman
Copy link
Member

@gerben-s Can you review please?

@jtattermusch
Copy link
Contributor Author

The new benchmarks (as of commit 4f0afc7) are here: https://gist.github.com/jtattermusch/c9e5389a398f7c79710a9bf12aee215f
For both writing primitives and writing messages the performance is comparable (there are some significant improvement and some slight regressions) to the original state, which seems good enough (as this PR offers an API to serialize directly into a Span and/or to IBufferWriter, which are modern APIs allowing for much better GC efficiency).

Copy link
Contributor

@gerben-s gerben-s left a comment

Choose a reason for hiding this comment

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

Yes this looks good to me

@MaximGurschi
Copy link

Is there a timeline, for when the nuget package for C# will be released? Eagerly waiting to be able to use Span with the serialization/deserialization methods you have developed :)

@jtattermusch
Copy link
Contributor Author

Is there a timeline, for when the nuget package for C# will be released? Eagerly waiting to be able to use Span with the serialization/deserialization methods you have developed :)

If everything goes as planned, it should be available over the course of the next few weeks.

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

Successfully merging this pull request may close these issues.

None yet

7 participants