Re: [PATCH v3 3/8] lib: add zstd support to decompress

[Nick Terrell]

> On Mar 26, 2020, at 9:47 AM, Petr Malat <oss@xxxxxxxxx> wrote:
> 
> Hi!
> On Wed, Mar 25, 2020 at 12:58:44PM -0700, Nick Terrell wrote:
>> From: Nick Terrell <terrelln@xxxxxx>
>> * Add unzstd() and the zstd decompress interface.
> Here I do not understand why you limit the window size to 8MB even when
> you read a larger value from the header. I do not see a reason why there
> should be such a limitation at the first place and if there should be,
> why it differs from ZSTD_WINDOWLOG_MAX.

When we are doing streaming decompression (either flush or fill is provided)
we have to allocate memory proportional to the window size. We want to
bound that memory so we donât accidentally allocate too much memory.
When we are doing a single-pass decompression (neither flush nor fill
are provided) the window size doesnât matter, and we only have to allocate
a fixed amount of memory ~192 KB.

The zstd spec [0] specifies that all decoders should allow window sizes
up to 8 MB. Additionally, the zstd CLI wonât produce window sizes greater
than 8 MB by default. The window size is controlled by the compression
level, and can be explicitly set.

I would expect larger window sizes to be beneficial for compression ratio,
though there is demising returns. I would expect that for kernel image
compression larger window sizes are beneficial, since it is decompressed
with a single pass. For initramfs decompression, I would expect that limiting
the window size could help decompression speed, since it uses streaming
compression, so unzstd() has to allocate a buffer of window size bytes.

> I removed that limitation to be able to test it in my environment and I
> found the performance is worst than with my patch by roughly 20% (on
> i7-3520M), which is a major drawback considering the main motivation
> to use zstd is the decompression speed. I will test on arm as well and
> share the result tomorrow.
>  Petr

What do you mean by that? Can you share with me the test you ran?
Is this for kernel decompression or initramfs decompression?

Best,
Nick

[0] https://tools.ietf.org/html/rfc8478#section-3.1.1.1.2