[Spice-devel,spice-server] Use LZ4_compress_fast_continue if available

Submitted by Frediano Ziglio on Nov. 28, 2016, 5:50 p.m.

Details

Message ID 20161128175016.20243-2-fziglio@redhat.com
State Accepted
Headers show
Series "Detect LZ4_compress_fast_continue function" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Nov. 28, 2016, 5:50 p.m.
This make compression faster and avoids a warning on newer
lz4 versions.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/lz4-encoder.c | 9 ++++++++-
 spice-common         | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
index f193fd8..9fabc35 100644
--- a/server/lz4-encoder.c
+++ b/server/lz4-encoder.c
@@ -75,9 +75,16 @@  int lz4_encode(Lz4EncoderContext *lz4, int height, int stride, uint8_t *io_ptr,
         in_buf = lines;
         in_size = stride * num_lines;
         lines += in_size;
-        compressed_lines = (uint8_t *) malloc(LZ4_compressBound(in_size) + 4);
+        int bound_size = LZ4_compressBound(in_size);
+        compressed_lines = (uint8_t *) malloc(bound_size + 4);
+#ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
+        enc_size = LZ4_compress_fast_continue(stream, (const char *) in_buf,
+                                              (char *) compressed_lines + 4, in_size,
+                                              bound_size, 1);
+#else
         enc_size = LZ4_compress_continue(stream, (const char *) in_buf,
                                          (char *) compressed_lines + 4, in_size);
+#endif
         if (enc_size <= 0) {
             spice_error("compress failed!");
             free(compressed_lines);
diff --git a/spice-common b/spice-common
index 31819a2..9218da2 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@ 
-Subproject commit 31819a24248ba5311e7293ddac519b8134e67fa0
+Subproject commit 9218da25ebd8737aea88662c6d537b72cd380cd9

Comments

On Mon, 2016-11-28 at 17:50 +0000, Frediano Ziglio wrote:
> This make compression faster and avoids a warning on newer
> lz4 versions.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
> ---
>  server/lz4-encoder.c | 9 ++++++++-
>  spice-common         | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
> index f193fd8..9fabc35 100644
> --- a/server/lz4-encoder.c
> +++ b/server/lz4-encoder.c
> @@ -75,9 +75,16 @@ int lz4_encode(Lz4EncoderContext *lz4, int
> height, int stride, uint8_t *io_ptr,
>          in_buf = lines;
>          in_size = stride * num_lines;
>          lines += in_size;
> -        compressed_lines = (uint8_t *)
> malloc(LZ4_compressBound(in_size) + 4);
> +        int bound_size = LZ4_compressBound(in_size);
> +        compressed_lines = (uint8_t *) malloc(bound_size + 4);
> +#ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
> +        enc_size = LZ4_compress_fast_continue(stream, (const char
> *) in_buf,
> +                                              (char *)
> compressed_lines + 4, in_size,
> +                                              bound_size, 1);
> +#else
>          enc_size = LZ4_compress_continue(stream, (const char *)
> in_buf,
>                                           (char *) compressed_lines
> + 4, in_size);
> +#endif
>          if (enc_size <= 0) {
>              spice_error("compress failed!");
>              free(compressed_lines);
> diff --git a/spice-common b/spice-common
> index 31819a2..9218da2 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 31819a24248ba5311e7293ddac519b8134e67fa0
> +Subproject commit 9218da25ebd8737aea88662c6d537b72cd380cd9
On 11/28/2016 07:50 PM, Frediano Ziglio wrote:
> This make compression faster and avoids a warning on newer
> lz4 versions.
>
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/lz4-encoder.c | 9 ++++++++-
>  spice-common         | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
> index f193fd8..9fabc35 100644
> --- a/server/lz4-encoder.c
> +++ b/server/lz4-encoder.c

Hi Frediano,

Nitpick: Since the functions signatures are the same, I think
a bit nicer is to "chose" the right function somewhere at the
top and later use it in the code.

For example:

#ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
#define spice_lz4_compress_continue LZ4_compress_fast_continue
#else
#define spice_lz4_compress_continue LZ4_compress_continue
#endif

> @@ -75,9 +75,16 @@ int lz4_encode(Lz4EncoderContext *lz4, int height, int stride, uint8_t *io_ptr,
>          in_buf = lines;
>          in_size = stride * num_lines;
>          lines += in_size;
> -        compressed_lines = (uint8_t *) malloc(LZ4_compressBound(in_size) + 4);
> +        int bound_size = LZ4_compressBound(in_size);
> +        compressed_lines = (uint8_t *) malloc(bound_size + 4);
> +#ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
> +        enc_size = LZ4_compress_fast_continue(stream, (const char *) in_buf,
> +                                              (char *) compressed_lines + 4, in_size,
> +                                              bound_size, 1);
> +#else
>          enc_size = LZ4_compress_continue(stream, (const char *) in_buf,
>                                           (char *) compressed_lines + 4, in_size);
> +#endif


and here simply
            enc_size = spice_lz4_compress_continue(stream, ...

Regards,
     Uri.

>          if (enc_size <= 0) {
>              spice_error("compress failed!");
>              free(compressed_lines);
> diff --git a/spice-common b/spice-common
> index 31819a2..9218da2 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 31819a24248ba5311e7293ddac519b8134e67fa0
> +Subproject commit 9218da25ebd8737aea88662c6d537b72cd380cd9
>
> 
> On 11/28/2016 07:50 PM, Frediano Ziglio wrote:
> > This make compression faster and avoids a warning on newer
> > lz4 versions.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  server/lz4-encoder.c | 9 ++++++++-
> >  spice-common         | 2 +-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
> > index f193fd8..9fabc35 100644
> > --- a/server/lz4-encoder.c
> > +++ b/server/lz4-encoder.c
> 
> Hi Frediano,
> 
> Nitpick: Since the functions signatures are the same, I think
> a bit nicer is to "chose" the right function somewhere at the
> top and later use it in the code.
> 
> For example:
> 
> #ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
> #define spice_lz4_compress_continue LZ4_compress_fast_continue
> #else
> #define spice_lz4_compress_continue LZ4_compress_continue
> #endif
> 

LZ4_compress_fast_continue have 2 additional parameters.
Perhaps however LZ4_compress_fast_continue can be implemented using
LZ4_compress_continue but looks confusing stripping a boundary
size (looks like looking for overflows).

> > @@ -75,9 +75,16 @@ int lz4_encode(Lz4EncoderContext *lz4, int height, int
> > stride, uint8_t *io_ptr,
> >          in_buf = lines;
> >          in_size = stride * num_lines;
> >          lines += in_size;
> > -        compressed_lines = (uint8_t *) malloc(LZ4_compressBound(in_size) +
> > 4);
> > +        int bound_size = LZ4_compressBound(in_size);
> > +        compressed_lines = (uint8_t *) malloc(bound_size + 4);
> > +#ifdef HAVE_LZ4_COMPRESS_FAST_CONTINUE
> > +        enc_size = LZ4_compress_fast_continue(stream, (const char *)
> > in_buf,
> > +                                              (char *) compressed_lines +
> > 4, in_size,
> > +                                              bound_size, 1);
> > +#else
> >          enc_size = LZ4_compress_continue(stream, (const char *) in_buf,
> >                                           (char *) compressed_lines + 4,
> >                                           in_size);
> > +#endif
> 
> 
> and here simply
>             enc_size = spice_lz4_compress_continue(stream, ...
> 
> Regards,
>      Uri.
> 

Frediano
On 11/29/2016 01:51 PM, Frediano Ziglio wrote:
>>
>> On 11/28/2016 07:50 PM, Frediano Ziglio wrote:
>>> This make compression faster and avoids a warning on newer
>>> lz4 versions.
>>>
>>> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
>> Hi Frediano,
>>
>> Nitpick: Since the functions signatures are the same,

[snipped]
>>
>
> LZ4_compress_fast_continue have 2 additional parameters.

Indeed, I should have looked closer

Uri.