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

Consider adding option to use cgo zstd implementation #482

Open
glukasiknuro opened this issue Oct 8, 2021 · 4 comments
Open

Consider adding option to use cgo zstd implementation #482

glukasiknuro opened this issue Oct 8, 2021 · 4 comments

Comments

@glukasiknuro
Copy link
Contributor

When checking performance of bazel-remote noticed that most of the time is spent in zstd compression.

Created a benchmark that shows that cgo implementation of zstd is more performant (2x-3x compression, 4x-6x decompression) : https://github.com/glukasiknuro/go-zstd-benchmarks

I have created a proof of concept implementation using go zstd implementation of bazel-remote in 591894a, and created a load test that confirms the benefits of using cgo implementation are significant:
https://github.com/glukasiknuro/go-zstd-benchmarks/tree/master/bazel-remote-load-test

I will try to suggest a patch that will allow to select cgo implementation of zstd to bazel-remote if compiled in CGO mode.

Also, some more details about performance of cgo implementation in klauspost/compress#444

@mostynb
Copy link
Collaborator

mostynb commented Oct 8, 2021

Thanks for the excellent investigation work- I will try to take a look early next week.

@mostynb
Copy link
Collaborator

mostynb commented Oct 14, 2021

I think this is something worth trying to land, and it would be good to keep both implementations around at least to begin with. I hope it doesn't make cross-compilation too difficult.

glukasiknuro added a commit to glukasiknuro/bazel-remote that referenced this issue Oct 14, 2021
Allows to select cgo implementation of zstd if compiled in.

The cgo implementation is more performant than pure go implementation,
see buchgr#482 for actual
benchmarks.
glukasiknuro added a commit to glukasiknuro/bazel-remote that referenced this issue Oct 15, 2021
Allows to select cgo implementation of zstd if compiled in.

The cgo implementation is more performant than pure go implementation,
see buchgr#482 for actual
benchmarks.
glukasiknuro added a commit to glukasiknuro/bazel-remote that referenced this issue Oct 15, 2021
Allows to select cgo implementation of zstd if compiled in.

The cgo implementation is more performant than pure go implementation,
see buchgr#482 for actual
benchmarks.
@glukasiknuro
Copy link
Contributor Author

Added WIP implementation: #485

For now results for downloading files are promising (40% overhead of enabling zstd instead of 160%), but looks like overhead of uploading files is too high in the bazel itself - the PR reduced it to 310% from 490%, but those numbers are far from expected based on raw zstd benchmarks. Investigating.

@glukasiknuro
Copy link
Contributor Author

Marked #485 as ready for review, the overhead of compression is actually expected.

glukasiknuro added a commit to glukasiknuro/bazel-remote that referenced this issue Oct 15, 2021
Allows to select cgo implementation of zstd if compiled in.

The cgo implementation is more performant than pure go implementation,
see buchgr#482 for actual
benchmarks.
mostynb pushed a commit to mostynb/bazel-remote that referenced this issue Jun 28, 2022
The cgo implementation may be more performant than the pure go
implementation, see buchgr#482 for benchmarks.
mostynb pushed a commit to mostynb/bazel-remote that referenced this issue Jun 28, 2022
The cgo implementation may be more performant than the pure go
implementation, see buchgr#482 for benchmarks.
mostynb pushed a commit to mostynb/bazel-remote that referenced this issue Jul 9, 2022
The cgo implementation may be more performant than the pure go
implementation, see buchgr#482 for benchmarks.
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

No branches or pull requests

2 participants