bazel-remote icon indicating copy to clipboard operation
bazel-remote copied to clipboard

Consider adding option to use cgo zstd implementation

Open glukasiknuro opened this issue 3 years ago • 4 comments

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 https://github.com/buchgr/bazel-remote/commit/591894a55266d3552cbd983c65115e24e252df8e, 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 https://github.com/klauspost/compress/issues/444

glukasiknuro avatar Oct 08 '21 22:10 glukasiknuro

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

mostynb avatar Oct 08 '21 23:10 mostynb

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.

mostynb avatar Oct 14 '21 22:10 mostynb

Added WIP implementation: https://github.com/buchgr/bazel-remote/pull/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 avatar Oct 15 '21 17:10 glukasiknuro

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

glukasiknuro avatar Oct 15 '21 20:10 glukasiknuro