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

pkg/chunkenc: change default LZ4 buffer size to 64k. #1421

Merged
merged 5 commits into from
Dec 19, 2019
Merged

pkg/chunkenc: change default LZ4 buffer size to 64k. #1421

merged 5 commits into from
Dec 19, 2019

Conversation

pstibrany
Copy link
Member

Also added more LZ4 buffer size options for testing.

What this PR does / why we need it:
LZ4 uses 4M buffer size by default when compressing data. It actually uses twice as much memory for decompression. Even though we pool lz4 readers, it can be a lot of allocated and pooled memory, if there are many concurrent decompressions.

In addition to that, we avoid pooling readers that were used to read chunks compressed with higher block sizes. Reason is that such readers must have used twice as much memory, and we don't want to keep these big-allocations in the pool.

Using smaller buffer doesn't affect (de-)compression time, although it may slightly affect ratio.

Time:

Read/none-4      51.1ms ± 6%
Read/gzip-4       211ms ± 6%
Read/lz4-64k-4   81.7ms ± 4%
Read/lz4-256k-4  81.3ms ± 7%
Read/lz4-1M-4    83.8ms ± 6%
Read/lz4-4M-4    81.6ms ± 4%
Read/snappy-4    80.6ms ± 3%

Memory:

name             alloc/op
Read/none-4       176kB ± 0%
Read/gzip-4       189kB ± 0%
Read/lz4-64k-4    185kB ± 1%
Read/lz4-256k-4   213kB ± 2%
Read/lz4-1M-4     334kB ± 5%
Read/lz4-4M-4     800kB ± 9%
Read/snappy-4     187kB ± 0%

Special notes for your reviewer:
"lz4" now defaults to LZ4_64k. Other lz4 buffer sizes are not available for user, but used for testing only.

Checklist

  • Documentation added
  • Tests updated

Added more LZ4 buffer size options for testing.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Such readers must have allocated twice as many bytes as the block size,
and we don't want to pool them and keep that memory allocated.

This can happen when reading chunks that were compressed with high buffer
size.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit a2c2a90 into grafana:master Dec 19, 2019
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

Successfully merging this pull request may close these issues.

None yet

2 participants