Skip to content
This repository has been archived by the owner on Jan 11, 2021. It is now read-only.

Add ZSTD compression #107

Merged
merged 2 commits into from
May 2, 2018
Merged

Add ZSTD compression #107

merged 2 commits into from
May 2, 2018

Conversation

sunchao
Copy link
Owner

@sunchao sunchao commented Apr 30, 2018

This adds ZSTD compression along with benchmark for it. The
implementation is backed by the zstd-rs crate.

Fixes #52.

@coveralls
Copy link

coveralls commented Apr 30, 2018

Coverage Status

Coverage decreased (-0.02%) to 94.963% when pulling 26136e9 on zstd into 132e2e2 on master.

Copy link
Collaborator

@sadikovi sadikovi left a comment

Choose a reason for hiding this comment

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

Looks good. I left one comment.


fn compress(&mut self, input_buf: &[u8]) -> Result<Vec<u8>> {
let output = Vec::new();
let mut encoder = zstd::Encoder::new(output, 1)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you consider making compression level 1 to be 0 or a constant for ZSTDCodec?
Could you also add a small comment describing what it means? Thanks.

Compression level (1-21).A level of 0 uses zstd's default (currently 3).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good point. Will change. The default level 0 has poor compression speed comparing to 1, so I'm thinking to use 1 instead (the same as parquet-cpp).

Level 0:
test compress_zstd_binary ... bench: 3,789,414 ns/iter (+/- 240,197) = 96 MB/s
test compress_zstd_boolean ... bench: 271,415 ns/iter (+/- 42,601) = 4 MB/s
test compress_zstd_double ... bench: 453,922 ns/iter (+/- 105,050) = 176 MB/s
test compress_zstd_fixed ... bench: 268,887 ns/iter (+/- 46,441) = 49 MB/s
test compress_zstd_float ... bench: 355,983 ns/iter (+/- 34,437) = 112 MB/s
test compress_zstd_int32 ... bench: 565,103 ns/iter (+/- 44,072) = 72 MB/s
test compress_zstd_int64 ... bench: 806,926 ns/iter (+/- 95,566) = 100 MB/s
test compress_zstd_int96 ... bench: 265,995 ns/iter (+/- 36,753)

Level 1:
test compress_zstd_binary ... bench: 593,655 ns/iter (+/- 195,929) = 615 MB/s
test compress_zstd_boolean ... bench: 14,001 ns/iter (+/- 2,614) = 92 MB/s
test compress_zstd_double ... bench: 107,195 ns/iter (+/- 16,586) = 746 MB/s
test compress_zstd_fixed ... bench: 3,906 ns/iter (+/- 785) = 3425 MB/s
test compress_zstd_float ... bench: 59,267 ns/iter (+/- 10,907) = 674 MB/s
test compress_zstd_int32 ... bench: 68,394 ns/iter (+/- 17,695) = 596 MB/s
test compress_zstd_int64 ... bench: 143,389 ns/iter (+/- 35,973) = 566 MB/s
test compress_zstd_int96 ... bench: 2,698 ns/iter (+/- 557) = 5 MB/s

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, one would expect it, since 0 falls back to level 3.

Copy link
Collaborator

@sadikovi sadikovi left a comment

Choose a reason for hiding this comment

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

Looks good. Could you rebase - it looks like there are some conflicts with master branch? Thanks!

This adds ZSTD compression along with benchmark for it. The
implementation is backed by the `zstd-rs` crate.

Fixes #52.
@sunchao
Copy link
Owner Author

sunchao commented May 1, 2018

Oops. Forgot about that. Will update.

@sunchao
Copy link
Owner Author

sunchao commented May 2, 2018

Merged. Thanks @sadikovi for the review.

@sunchao sunchao deleted the zstd branch May 2, 2018 03:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants