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

hash/crc32.update is a major CPU hog #255

Closed
majek opened this issue Feb 6, 2015 · 6 comments
Closed

hash/crc32.update is a major CPU hog #255

majek opened this issue Feb 6, 2015 · 6 comments

Comments

@majek
Copy link

majek commented Feb 6, 2015

When profiling my application, I found that crc32 uses 50% of my CPU resources:

    1165  49.1%  49.1%     1165  49.1% hash/crc32.update

This is being called by real_decoder.go:pop in.check:

While I generally do appreciate sarama verifying checksums by default, I'd like to have a toggle to turn it off. I'd much rather crash once in a while than spend half of my CPU on tedious work.

My app reads about 130k reasonably small requests per second.

@drdee
Copy link

drdee commented Feb 10, 2015

hey @majek
Thanks for filing this issue! I am wondering, instead of adding a config parameter (which I doubt many people will use), maybe we should port this C++ crc32_fast algorithm to Go: http://create.stephan-brumme.com/crc32/. That blog post offers some neat ways to significantly improve performance.

@eapache what do you think?

@majek
Copy link
Author

majek commented Feb 10, 2015

Yes, making crc32 faster could work as well. Relevant: https://issues.apache.org/jira/browse/KAFKA-1449

@joestein
Copy link

Those are some interesting stats. Providing a dynamic option for hashing is an interesting feature too.

For KAFKA-1449 it might be worth building a patch, testing it and then posting results to the https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals process.

btw <- LZ4 compression is available now in 0.8.2.0 https://issues.apache.org/jira/browse/KAFKA-1456 & https://issues.apache.org/jira/browse/KAFKA-1493 if you want to give it a try.

@eapache
Copy link
Contributor

eapache commented Feb 10, 2015

I would be happy to get faster doing CRC32s - when I've benchmarked the producer in the past it showed up as a significant cost there too.

It should be done as a separate package though, that code shouldn't live in Sarama proper.

@drdee
Copy link

drdee commented Feb 10, 2015

@eapache I can take a crack at this later this week.

@wvanbergen
Copy link
Contributor

Instead of disabling CRC checks, we could use this faster implementation: https://github.com/klauspost/crc32

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

No branches or pull requests

5 participants