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 pure Go zstd implementation #1162
Comments
We're working on this @narqo. I'll try to make this change as soon as possible. |
Hi @narqo , we've decided to not use the pure go based ZSTD because of performance issues. Please see #1176 (comment) and the benchmarks in #1176 (comment) . You can build badger with |
The discussion in #1176 implies to me that this decision should be reconsidered. |
I'm also interested in seeing this reopened. I have a package that imports badger/v2, and the indirect dependency on DataDog/zstd is really unfortunate. It takes a long time to build all that C code, and because it's a single package, it means a huge bottleneck in my build time. |
I did some benchmarks against both the libraries using the badger benchmark write tool https://github.com/dgraph-io/badger/blob/master/badger/cmd/write_bench.go (compresion is disabled by default, you'll have to enable it) and here's what I found
Datadog/zstd logs(run master with compression enabled)
Klauspost/compress(run code in https://github.com/dgraph-io/badger/tree/ibrahim/klauspost-compress with compression enabled)
@johanbrandhorst @mvdan I understand that CGO is an evil and should not be used but do you have any strong reasons for switching to pure go based library? I'd like to move away from CGO but the performance difference is what makes me reluctant for this change. I'd love to know your thoughts |
For those wondering what the code looks like, it's https://github.com/dgraph-io/badger/compare/ibrahim/klauspost-compress. I'll leave it to @klauspost to comment if the use of his pure Go version could be improved or if it's equivalent to the cgo version. It might be impossible for compression to beat well optimized C code, even with the cgo cost of DataDog/zstd. But still, we could use this issue to track progress or reevaluate the situation every now and then. To me, this is a tradeoff - I would gladly have 5% slower code that doesn't require cgo with all of its drawbacks, for example. I get that you're seeing a difference larger than a few percent, but I imagine that gap can be made smaller over time. |
Great with a real world test. However it doesn't seem too real world. You seem to be writing random (incompressible) data. That is a very, very limited test of a compressor and I assume the data isn't really compressed at all? For this test you can use the You should disable CRC. It is disabled by default in datadog IIRC. This is about 5% improvement for me. Honestly, overall, if it is disabled by default I don't really see a problem. Also it could at least be used as a fallback. You can just use the Edit: 2x faster random data in fastest mode: klauspost/compress#270 |
I made the changes @klauspost suggested and I see very different results now. I made the following two changes
Here's what I see now
I'm assuming the performance improvement is mostly because the data is no longer random. In real use cases, the data won't be random. On master
On ibrahim/klauspost-compress branch
I think we should switch over to the pure go based implementation. Badger compactions (which will compress data) happen in the background and they can get affected by multiple factors. The compression algorithm wouldn't be the bottleneck. @mvdan I understand your point but just out of curiosity, why wouldn't you use CGO in your code? I also noticed that the |
Yes they are compatible both ways. The only exception is 0 bytes of input which will give 0 bytes output with the Go zstd. But you already have the I am fuzz testing the change above. It will have much less compression impact than completely disabling entropy coding, but will handle the random input blocks better. I would probably leave out the
The dd library allocates a lot. That could be why. Go zstd does not allocate for 4K blocks after a few runs.
Compiling is much slower. Cross compilation is a pain to set up/impossible for some. Deployment requires dependencies whereas plain Go is just the executable. cgo is inherently less secure since none of the c code has the security features the Go runtime provides. |
FYI, I just released |
Github issues have been deprecated. |
What version of Go are you using (
go version
)?What version of Badger are you using?
latest master
Does this issue reproduce with the latest master?
Yes
As it was mentioned in #1081 #1104 BadgerDB consider switching to a pure Go implementation of zstd (github.com/klauspost/compress) after it went out of Beta.
Version 1.9.4 is no longer marked as beta, refer to https://github.com/klauspost/compress/tree/v1.9.4/zstd#status
The text was updated successfully, but these errors were encountered: