-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Use exact size, if known, to allocate decompression buffer #3048
Conversation
For large messages this generates far less garbage than ioutil.ReadAll(). Implement for gzip - RFC1952 requires it, and the Go implementation checks it already (modulo 2^32). Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
e36a434
to
4d7f286
Compare
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 the new version, this is even better!
Return -1 instead of an error in the case that we can't determine a size. Use a LimitReader set to size+1 so we can detect cases like corrupt messages and overflow of the 32-bit size field. Signed-off-by: Bryan Boreham <bryan@weave.works>
Based on review feedback Signed-off-by: Bryan Boreham <bryan@weave.works>
I pushed a couple more commits which I think cover all the review comments. |
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
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. One last minor change to request for initializing buf
first, though. Thanks!
It's simpler. Signed-off-by: Bryan Boreham <bryan@weave.works>
Thanks for the contribution! |
This brings in some performance improvements, including grpc/grpc-go#3048 which should make a big difference when using gRPC gzip compression. Plus some updated dependencies. Signed-off-by: Bryan Boreham <bryan@weave.works>
Fixes #2635
For large messages this generates far less garbage than
ioutil.ReadAll()
.Implement for gzip - RFC1952 requires it, and the Go implementation checks it already (modulo 2^32).