-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Implement new Codec that uses mem.BufferSlice
instead of []byte
#7356
Conversation
This new codec, as outlined in grpc#6619 will allow the reuse of buffers to their fullest extent. Note that this deliberately does not (yet) implement support for stathandlers, however all the relevant APIs have been updated and both old and new Codec implementations are supported.
… the transport package
internal/transport/http2_server.go
Outdated
func (t *http2Server) Write(s *Stream, hdr []byte, data []byte, opts *Options) error { | ||
func (t *http2Server) Write(s *Stream, hdr []byte, data mem.BufferSlice, opts *Options) error { | ||
reader := data.Reader() | ||
data.Free() |
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.
Should Reader
just take ownership instead of acquiring its own ref? Unconditionally free
ing here and in the other usage seems weird.
We may want to call it ToReader
to make it obvious that ownership is transferred in that case.
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.
Talked about this offline. It's less error prone to have .Reader
take a reference and guarantee that it will be there rather than having to make sure Reader
has its own reference. The overall attitude towards buffers is that callers to a function manage the lifecycle of buffers, and if a function wants to guarantee that a buffer lives beyond the function returning, it should always take its own reference. This has been updated in the codec as well
This new codec, as outlined in #6619 will allow the reuse of buffers to their fullest extent. Note that this deliberately does not (yet) implement support for stathandlers, however all the relevant APIs have been updated and both old and new Codec implementations are supported.
RELEASE NOTES: