Re: [PATCH v9 3/4] qcow2: add zstd cluster compression

2020-03-27 Thread Vladimir Sementsov-Ogievskiy

27.03.2020 12:40, Denis Plotnikov wrote:



On 27.03.2020 11:43, Vladimir Sementsov-Ogievskiy wrote:

Should we note somehow in qcow2 spec that we use streamed version of zstd with 
specific end byte?

We didn't do it for zlib. zstd does it the same way as zlib, saves the 
compression output to some buffer.


Looked through zstd format spec, I don't thing that there are may be any 
principal difference between data
compressed by simple api or by streamed, so now I don't think that we need any 
additional comment.





23.03.2020 17:25, Denis Plotnikov wrote:

zstd significantly reduces cluster compression time.
It provides better compression performance maintaining
the same level of the compression ratio in comparison with
zlib, which, at the moment, is the only compression
method available.

The performance test results:
Test compresses and decompresses qemu qcow2 image with just
installed rhel-7.6 guest.
Image cluster size: 64K. Image on disk size: 2.2G

The test was conducted with brd disk to reduce the influence
of disk subsystem to the test results.
The results is given in seconds.

compress cmd:
   time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
   src.img [zlib|zstd]_compressed.img
decompress cmd
   time ./qemu-img convert -O qcow2
   [zlib|zstd]_compressed.img uncompressed.img

    compression   decompression
  zlib   zstd   zlib zstd

real 65.5   16.3 (-75 %)    1.9  1.6 (-16 %)
user 65.0   15.8    5.3  2.5
sys   3.3    0.2    2.0  2.0

Both ZLIB and ZSTD gave the same compression ratio: 1.57
compressed image size in both cases: 1.4G

Signed-off-by: Denis Plotnikov 
QAPI part:
Acked-by: Markus Armbruster 
---


[..]


+
+/*
+ * qcow2_zstd_compress()
+ *
+ * Compress @src_size bytes of data using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: compressed size on success
+ *  -ENOMEM destination buffer is not enough to store compressed data
+ *  -EIO    on any other error
+ */
+static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
+   const void *src, size_t src_size)
+{
+    size_t ret;
+    ZSTD_outBuffer output = { dest, dest_size, 0 };
+    ZSTD_inBuffer input = { src, src_size, 0 };
+    ZSTD_CCtx *cctx = ZSTD_createCCtx();
+
+    if (!cctx) {
+    return -EIO;
+    }
+    /*
+ * ZSTD spec: "You must continue calling ZSTD_compressStream2()
+ * with ZSTD_e_end until it returns 0, at which point you are
+ * free to start a new frame"
+ */
+    {
+    /*
+ * zstd simple interface requires the exact compressed size.
+ * zstd stream interface reads the comressed size from
+ * the compressed stream frame.
+ * Instruct zstd to compress the whole buffer and write
+ * the frame which includes the compressed size.
+ * This allows as to use zstd streaming semantics and
+ * don't store the compressed size for the zstd decompression.
+ */
+    ret = ZSTD_compressStream2(cctx, , , ZSTD_e_end);
+    if (ZSTD_isError(ret)) {
+    ret = -EIO;
+    goto out;
+    }
+    /* Dest buffer isn't big enough to store compressed content */
+    if (output.pos + ret > output.size) {
+    ret = -ENOMEM;
+    goto out;
+    }
+    } while (ret);
+
+    /* if no error, the input data must be fully consumed */
+    assert(input.pos == input.size);
+    /* make sure we can safely return compressed buffer size with ssize_t *//z
+    assert(output.pos <= SSIZE_MAX);
+    ret = output.pos;
+
+out:
+    ZSTD_freeCCtx(cctx);
+    return ret;
+}
+
+/*
+ * qcow2_zstd_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce exactly
+ * @dest_size bytes using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *  -EIO on any error
+ */
+static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size,
+ const void *src, size_t src_size)
+{
+    size_t ret = 0;
+    ZSTD_outBuffer output = { dest, dest_size, 0 };
+    ZSTD_inBuffer input = { src, src_size, 0 };
+    ZSTD_DCtx *dctx = ZSTD_createDCtx();
+
+    if (!dctx) {
+    return -EIO;
+    }
+
+    {
+    ret = ZSTD_decompressStream(dctx, , );
+    if (ZSTD_isError(ret)) {
+    ret = -EIO;
+    goto out;
+    }
+    /*
+ * Dest buffer size is the image cluster size.
+ * It should be big enough to store uncompressed content.
+ * There shouldn't be any cases when the decompressed content
+ * size is greater then the cluster size.
+ 

Re: [PATCH v9 3/4] qcow2: add zstd cluster compression

2020-03-27 Thread Denis Plotnikov




On 27.03.2020 11:43, Vladimir Sementsov-Ogievskiy wrote:
Should we note somehow in qcow2 spec that we use streamed version of 
zstd with specific end byte?
We didn't do it for zlib. zstd does it the same way as zlib, saves the 
compression output to some buffer.




23.03.2020 17:25, Denis Plotnikov wrote:

zstd significantly reduces cluster compression time.
It provides better compression performance maintaining
the same level of the compression ratio in comparison with
zlib, which, at the moment, is the only compression
method available.

The performance test results:
Test compresses and decompresses qemu qcow2 image with just
installed rhel-7.6 guest.
Image cluster size: 64K. Image on disk size: 2.2G

The test was conducted with brd disk to reduce the influence
of disk subsystem to the test results.
The results is given in seconds.

compress cmd:
   time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
   src.img [zlib|zstd]_compressed.img
decompress cmd
   time ./qemu-img convert -O qcow2
   [zlib|zstd]_compressed.img uncompressed.img

    compression   decompression
  zlib   zstd   zlib zstd

real 65.5   16.3 (-75 %)    1.9  1.6 (-16 %)
user 65.0   15.8    5.3  2.5
sys   3.3    0.2    2.0  2.0

Both ZLIB and ZSTD gave the same compression ratio: 1.57
compressed image size in both cases: 1.4G

Signed-off-by: Denis Plotnikov 
QAPI part:
Acked-by: Markus Armbruster 
---


[..]


+
+/*
+ * qcow2_zstd_compress()
+ *
+ * Compress @src_size bytes of data using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: compressed size on success
+ *  -ENOMEM destination buffer is not enough to store 
compressed data

+ *  -EIO    on any other error
+ */
+static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
+   const void *src, size_t src_size)
+{
+    size_t ret;
+    ZSTD_outBuffer output = { dest, dest_size, 0 };
+    ZSTD_inBuffer input = { src, src_size, 0 };
+    ZSTD_CCtx *cctx = ZSTD_createCCtx();
+
+    if (!cctx) {
+    return -EIO;
+    }
+    /*
+ * ZSTD spec: "You must continue calling ZSTD_compressStream2()
+ * with ZSTD_e_end until it returns 0, at which point you are
+ * free to start a new frame"
+ */
+    {
+    /*
+ * zstd simple interface requires the exact compressed size.
+ * zstd stream interface reads the comressed size from
+ * the compressed stream frame.
+ * Instruct zstd to compress the whole buffer and write
+ * the frame which includes the compressed size.
+ * This allows as to use zstd streaming semantics and
+ * don't store the compressed size for the zstd decompression.
+ */
+    ret = ZSTD_compressStream2(cctx, , , ZSTD_e_end);
+    if (ZSTD_isError(ret)) {
+    ret = -EIO;
+    goto out;
+    }
+    /* Dest buffer isn't big enough to store compressed content */
+    if (output.pos + ret > output.size) {
+    ret = -ENOMEM;
+    goto out;
+    }
+    } while (ret);
+
+    /* if no error, the input data must be fully consumed */
+    assert(input.pos == input.size);
+    /* make sure we can safely return compressed buffer size with 
ssize_t *//z

+    assert(output.pos <= SSIZE_MAX);
+    ret = output.pos;
+
+out:
+    ZSTD_freeCCtx(cctx);
+    return ret;
+}
+
+/*
+ * qcow2_zstd_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce 
exactly

+ * @dest_size bytes using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *  -EIO on any error
+ */
+static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size,
+ const void *src, size_t src_size)
+{
+    size_t ret = 0;
+    ZSTD_outBuffer output = { dest, dest_size, 0 };
+    ZSTD_inBuffer input = { src, src_size, 0 };
+    ZSTD_DCtx *dctx = ZSTD_createDCtx();
+
+    if (!dctx) {
+    return -EIO;
+    }
+
+    {
+    ret = ZSTD_decompressStream(dctx, , );
+    if (ZSTD_isError(ret)) {
+    ret = -EIO;
+    goto out;
+    }
+    /*
+ * Dest buffer size is the image cluster size.
+ * It should be big enough to store uncompressed content.
+ * There shouldn't be any cases when the decompressed content
+ * size is greater then the cluster size.
+ */
+    if (output.pos + ret > output.size) {
+    ret = -EIO;
+    goto out;
+    }
+    } while (ret);



Hmm. Unfortunately, zstd spec is not enough verbose to understand how 
to use

these functions :).

But I found this 

Re: [PATCH v9 3/4] qcow2: add zstd cluster compression

2020-03-27 Thread Vladimir Sementsov-Ogievskiy

Should we note somehow in qcow2 spec that we use streamed version of zstd with 
specific end byte?

23.03.2020 17:25, Denis Plotnikov wrote:

zstd significantly reduces cluster compression time.
It provides better compression performance maintaining
the same level of the compression ratio in comparison with
zlib, which, at the moment, is the only compression
method available.

The performance test results:
Test compresses and decompresses qemu qcow2 image with just
installed rhel-7.6 guest.
Image cluster size: 64K. Image on disk size: 2.2G

The test was conducted with brd disk to reduce the influence
of disk subsystem to the test results.
The results is given in seconds.

compress cmd:
   time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
   src.img [zlib|zstd]_compressed.img
decompress cmd
   time ./qemu-img convert -O qcow2
   [zlib|zstd]_compressed.img uncompressed.img

compression   decompression
  zlib   zstd   zlib zstd

real 65.5   16.3 (-75 %)1.9  1.6 (-16 %)
user 65.0   15.85.3  2.5
sys   3.30.22.0  2.0

Both ZLIB and ZSTD gave the same compression ratio: 1.57
compressed image size in both cases: 1.4G

Signed-off-by: Denis Plotnikov 
QAPI part:
Acked-by: Markus Armbruster 
---


[..]


+
+/*
+ * qcow2_zstd_compress()
+ *
+ * Compress @src_size bytes of data using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: compressed size on success
+ *  -ENOMEM destination buffer is not enough to store compressed data
+ *  -EIOon any other error
+ */
+static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
+   const void *src, size_t src_size)
+{
+size_t ret;
+ZSTD_outBuffer output = { dest, dest_size, 0 };
+ZSTD_inBuffer input = { src, src_size, 0 };
+ZSTD_CCtx *cctx = ZSTD_createCCtx();
+
+if (!cctx) {
+return -EIO;
+}
+/*
+ * ZSTD spec: "You must continue calling ZSTD_compressStream2()
+ * with ZSTD_e_end until it returns 0, at which point you are
+ * free to start a new frame"
+ */
+{
+/*
+ * zstd simple interface requires the exact compressed size.
+ * zstd stream interface reads the comressed size from
+ * the compressed stream frame.
+ * Instruct zstd to compress the whole buffer and write
+ * the frame which includes the compressed size.
+ * This allows as to use zstd streaming semantics and
+ * don't store the compressed size for the zstd decompression.
+ */
+ret = ZSTD_compressStream2(cctx, , , ZSTD_e_end);
+if (ZSTD_isError(ret)) {
+ret = -EIO;
+goto out;
+}
+/* Dest buffer isn't big enough to store compressed content */
+if (output.pos + ret > output.size) {
+ret = -ENOMEM;
+goto out;
+}
+} while (ret);
+
+/* if no error, the input data must be fully consumed */
+assert(input.pos == input.size);
+/* make sure we can safely return compressed buffer size with ssize_t *//z
+assert(output.pos <= SSIZE_MAX);
+ret = output.pos;
+
+out:
+ZSTD_freeCCtx(cctx);
+return ret;
+}
+
+/*
+ * qcow2_zstd_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce exactly
+ * @dest_size bytes using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *  -EIO on any error
+ */
+static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size,
+ const void *src, size_t src_size)
+{
+size_t ret = 0;
+ZSTD_outBuffer output = { dest, dest_size, 0 };
+ZSTD_inBuffer input = { src, src_size, 0 };
+ZSTD_DCtx *dctx = ZSTD_createDCtx();
+
+if (!dctx) {
+return -EIO;
+}
+
+{
+ret = ZSTD_decompressStream(dctx, , );
+if (ZSTD_isError(ret)) {
+ret = -EIO;
+goto out;
+}
+/*
+ * Dest buffer size is the image cluster size.
+ * It should be big enough to store uncompressed content.
+ * There shouldn't be any cases when the decompressed content
+ * size is greater then the cluster size.
+ */
+if (output.pos + ret > output.size) {
+ret = -EIO;
+goto out;
+}
+} while (ret);



Hmm. Unfortunately, zstd spec is not enough verbose to understand how to use
these functions :).

But I found this in comment in 
https://github.com/facebook/zstd/blob/dev/examples/streaming_decompression.c :

/* The return code is zero if the frame is complete, but there may
 *