[Spice-devel] [PATCH spice] compress-stat: Add not compressed image to statistics

[Frediano Ziglio]

> 
> To see how many images and data could not be compressed.
> ---
>  server/dcc.c             | 27 +++++++++++++++++++++++++++
>  server/display-channel.c | 11 +++++++++++
>  server/display-channel.h |  1 +
>  3 files changed, 39 insertions(+)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index bf692f8..15cec72 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1029,7 +1029,12 @@ static int
> dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage *dest,
>  
>  #define MIN_SIZE_TO_COMPRESS 54
>  #define MIN_DIMENSION_TO_QUIC 3
> +
> +#ifdef COMPRESS_STAT
> +static int dcc_compress_image_internal(DisplayChannelClient *dcc,
> +#else
>  int dcc_compress_image(DisplayChannelClient *dcc,
> +#endif
>                         SpiceImage *dest, SpiceBitmap *src, Drawable
>                         *drawable,
>                         int can_lossy,
>                         compress_send_data_t* o_comp_data)
> @@ -1154,6 +1159,28 @@ int dcc_compress_image(DisplayChannelClient *dcc,
>      }
>  }
>  
> +#ifdef COMPRESS_STAT
> +int dcc_compress_image(DisplayChannelClient *dcc,
> +                       SpiceImage *dest, SpiceBitmap *src, Drawable
> *drawable,
> +                       int can_lossy,
> +                       compress_send_data_t* o_comp_data)
> +{
> +    int success;
> +    stat_start_time_t start_time;
> +
> +    stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->off_stat);
> +    success = dcc_compress_image_internal(dcc, dest, src, drawable,
> can_lossy, o_comp_data);
> +
> +    if (!success) {
> +        uint64_t image_size = src->stride * src->y;
> +
> +        stat_compress_add(&DCC_TO_DC(dcc)->off_stat, start_time, image_size,
> image_size);
> +    }
> +
> +    return success;
> +}
> +#endif
> +

This change does not look that beautiful. What about your other patches
for image compression? Wouldn't they decrease the number of returns from
dcc_compress_image?
The stat_* interface is designed to produce no code is statistics are
disabled so copying on all exist paths would work. I understand that
now there are too much paths that return FALSE.

>  #define CLIENT_PALETTE_CACHE
>  #include "cache-item.tmpl.c"
>  #undef CLIENT_PALETTE_CACHE
> diff --git a/server/display-channel.c b/server/display-channel.c
> index f0d133a..5effd56 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -36,6 +36,7 @@ void display_channel_compress_stats_reset(DisplayChannel
> *display)
>  {
>      spice_return_if_fail(display);
>  
> +    stat_reset(&display->off_stat);
>      stat_reset(&display->quic_stat);
>      stat_reset(&display->lz_stat);
>      stat_reset(&display->glz_stat);
> @@ -58,6 +59,12 @@ void display_channel_compress_stats_print(const
> DisplayChannel *display_channel)
>  
>      spice_info("==> Compression stats for display %u",
>      display_channel->common.base.id);
>      spice_info("Method   \t  count
>      \torig_size(MB)\tenc_size(MB)\tenc_time(s)");
> +    spice_info("OFF     \t%8d\t%13.2f\t%12.2f\t%12.2f",
> +               display_channel->off_stat.count,
> +               stat_byte_to_mega(display_channel->off_stat.orig_size),
> +               stat_byte_to_mega(display_channel->off_stat.comp_size),
> +               stat_cpu_time_to_sec(display_channel->off_stat.total)
> +               );
>      spice_info("QUIC     \t%8d\t%13.2f\t%12.2f\t%12.2f",
>                 display_channel->quic_stat.count,
>                 stat_byte_to_mega(display_channel->quic_stat.orig_size),
> @@ -103,18 +110,21 @@ void display_channel_compress_stats_print(const
> DisplayChannel *display_channel)
>      spice_info("-------------------------------------------------------------------");
>      spice_info("Total    \t%8d\t%13.2f\t%12.2f\t%12.2f",
>                 display_channel->lz_stat.count +
>                 display_channel->glz_stat.count +
> +
> display_channel->off_stat.count
> +
>                                                  display_channel->quic_stat.count
>                                                  +
>                                                  display_channel->jpeg_stat.count
>                                                  +
>                                                  display_channel->lz4_stat.count
>                                                  +
>                                                  display_channel->jpeg_alpha_stat.count,
>                 stat_byte_to_mega(display_channel->lz_stat.orig_size +
>                                   display_channel->glz_stat.orig_size +
> +                                 display_channel->off_stat.orig_size +
>                                   display_channel->quic_stat.orig_size +
>                                   display_channel->jpeg_stat.orig_size +
>                                   display_channel->lz4_stat.orig_size +
>                                   display_channel->jpeg_alpha_stat.orig_size),
>                 stat_byte_to_mega(display_channel->lz_stat.comp_size +
>                                   glz_enc_size +
> +                                 display_channel->off_stat.comp_size +
>                                   display_channel->quic_stat.comp_size +
>                                   display_channel->jpeg_stat.comp_size +
>                                   display_channel->lz4_stat.comp_size +
> @@ -122,6 +132,7 @@ void display_channel_compress_stats_print(const
> DisplayChannel *display_channel)
>                 stat_cpu_time_to_sec(display_channel->lz_stat.total +
>                                      display_channel->glz_stat.total +
>                                      display_channel->zlib_glz_stat.total +
> +                                    display_channel->off_stat.total +
>                                      display_channel->quic_stat.total +
>                                      display_channel->jpeg_stat.total +
>                                      display_channel->lz4_stat.total +
> diff --git a/server/display-channel.h b/server/display-channel.h
> index bf29cd3..e3527bd 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -213,6 +213,7 @@ struct DisplayChannel {
>      uint64_t *add_to_cache_counter;
>      uint64_t *non_cache_counter;
>  #endif
> +    stat_info_t off_stat;
>      stat_info_t lz_stat;
>      stat_info_t glz_stat;
>      stat_info_t quic_stat;

The rest of the patch looks ok.

Frediano