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

add sync pool for gzip 1-9 #1560

Merged
merged 1 commit into from
Feb 23, 2020
Merged

add sync pool for gzip 1-9 #1560

merged 1 commit into from
Feb 23, 2020

Conversation

iyacontrol
Copy link
Contributor

@iyacontrol iyacontrol commented Dec 16, 2019

when large number of messages, for gzip(1-9) cost high cpu, because of gc.

New: func() interface{} {
gz, err := gzip.NewWriterLevel(nil, 1)
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big fan of panic, what happens if we reach here and panic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NewWriterLevel is like NewWriter but specifies the compression level instead of assuming DefaultCompression.

The compression level can be DefaultCompression, NoCompression, HuffmanOnly or any integer value between BestSpeed and BestCompression inclusive. The error returned will be nil if the level is valid.

1-9 level is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am really not a big fan of using panic. but 👍

Copy link
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

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

The changes look reasonable to me and had previously been mentioned under the original PR #1185 which added the re-usable sync.Pools for default lz4 and gzip. I think it would be fine to merge these in

/cc @varun06 @bai

@iyacontrol iyacontrol requested a review from varun06 February 23, 2020 12:31
@varun06 varun06 merged commit 356f1ed into IBM:master Feb 23, 2020
@iyacontrol iyacontrol deleted the pool branch February 24, 2020 01:37
sthagen added a commit to sthagen/Shopify-sarama that referenced this pull request Feb 24, 2020
add sync pool for gzip 1-9 (IBM#1560)
ronanh pushed a commit to ronanh/sarama that referenced this pull request Mar 4, 2020
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.

4 participants