[Spice-devel] [PATCH 3/4] stat: use a better design for statistic functions

Frediano Ziglio fziglio at redhat.com
Wed Dec 2 03:43:09 PST 2015


make sure code compile with statistics enabled or disabled.
Dummy (empty) structures and functions are used instead of preprocessor.
Also fix a problem as stat_compress_init did not initialize clock
field.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/dcc.c             | 31 +++++++++++-------------------
 server/display-channel.c | 47 +++++++++++++++++-----------------------------
 server/display-channel.h |  4 +---
 server/stat.h            | 49 +++++++++++++++++++++++++++++++-----------------
 4 files changed, 61 insertions(+), 70 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index 78452f4..5d55e5e 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -651,9 +651,8 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
                            compress_send_data_t* o_comp_data)
 {
     DisplayChannel *display_channel = DCC_TO_DC(dcc);
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(display_channel->glz_stat.clock);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &display_channel->zlib_glz_stat);
     spice_assert(bitmap_fmt_is_rgb(src->format));
     GlzData *glz_data = &dcc->glz_data;
     ZlibData *zlib_data;
@@ -687,9 +686,7 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
     if (!display_channel->enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
         goto glz;
     }
-#ifdef COMPRESS_STAT
-    start_time = stat_now(display_channel->zlib_glz_stat.clock);
-#endif
+    stat_start_time_init(&start_time, &display_channel->zlib_glz_stat);
     zlib_data = &dcc->zlib_data;
 
     zlib_data->data.bufs_tail = compress_buf_new();
@@ -741,9 +738,8 @@ int dcc_compress_image_lz(DisplayChannelClient *dcc,
     LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
     int size;            // size of the compressed data
 
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz_stat.clock);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->lz_stat);
 
     lz_data->data.bufs_tail = compress_buf_new();
     lz_data->data.bufs_head = lz_data->data.bufs_tail;
@@ -817,10 +813,9 @@ int dcc_compress_image_jpeg(DisplayChannelClient *dcc, SpiceImage *dest,
     int comp_head_left;
     int stride;
     uint8_t *lz_out_start_byte;
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->jpeg_alpha_stat);
 
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->jpeg_stat.clock);
-#endif
     switch (src->format) {
     case SPICE_BITMAP_FMT_16BIT:
         jpeg_in_type = JPEG_IMAGE_TYPE_RGB16;
@@ -940,10 +935,8 @@ int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage *dest,
     Lz4Data *lz4_data = &dcc->lz4_data;
     Lz4EncoderContext *lz4 = dcc->lz4;
     int lz4_size = 0;
-
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz4_stat.clock);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->lz4_stat);
 
     lz4_data->data.bufs_tail = compress_buf_new();
     lz4_data->data.bufs_head = lz4_data->data.bufs_tail;
@@ -1003,10 +996,8 @@ int dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage *dest,
     QuicContext *quic = dcc->quic;
     volatile QuicImageType type;
     int size, stride;
-
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->quic_stat.clock);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->quic_stat);
 
     switch (src->format) {
     case SPICE_BITMAP_FMT_32BIT:
diff --git a/server/display-channel.c b/server/display-channel.c
index 4ef5524..f37cb2b 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -29,26 +29,13 @@ uint32_t display_channel_generate_uid(DisplayChannel *display)
     return ++display->bits_unique;
 }
 
-static stat_time_t display_channel_stat_now(DisplayChannel *display)
-{
-#ifdef RED_WORKER_STAT
-    RedWorker *worker = COMMON_CHANNEL(display)->worker;
-
-    return stat_now(red_worker_get_clockid(worker));
-
-#else
-    return 0;
-#endif
-}
-
-#define stat_start(display, var)                                        \
-    G_GNUC_UNUSED stat_time_t var = display_channel_stat_now((display));
+#define stat_start(stat, var)                                        \
+    stat_start_time_t var; stat_start_time_init(&var, stat);
 
 void display_channel_compress_stats_reset(DisplayChannel *display)
 {
     spice_return_if_fail(display);
 
-#ifdef COMPRESS_STAT
     stat_reset(&display->quic_stat);
     stat_reset(&display->lz_stat);
     stat_reset(&display->glz_stat);
@@ -56,7 +43,6 @@ void display_channel_compress_stats_reset(DisplayChannel *display)
     stat_reset(&display->zlib_glz_stat);
     stat_reset(&display->jpeg_alpha_stat);
     stat_reset(&display->lz4_stat);
-#endif
 }
 
 void display_channel_compress_stats_print(const DisplayChannel *display_channel)
@@ -568,7 +554,7 @@ static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem *item
                              Ring **top_ring, Drawable *frame_candidate)
 {
     QRegion and_rgn;
-    stat_start(display, start_time);
+    stat_start(&display->__exclude_stat, start_time);
 
     region_clone(&and_rgn, rgn);
     region_and(&and_rgn, &item->rgn);
@@ -637,7 +623,7 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
                            QRegion *rgn, TreeItem **last, Drawable *frame_candidate)
 {
     Ring *top_ring;
-    stat_start(display, start_time);
+    stat_start(&display->exclude_stat, start_time);
 
     if (!ring_item) {
         return;
@@ -692,7 +678,7 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
 
 static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable *item)
 {
-    stat_start(display, start_time);
+    stat_start(&display->add_stat, start_time);
 #ifdef RED_WORKER_STAT
     ++display->add_with_shadow_count;
 #endif
@@ -739,7 +725,7 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
     RingItem *now;
     QRegion exclude_rgn;
     RingItem *exclude_base = NULL;
-    stat_start(display, start_time);
+    stat_start(&display->add_stat, start_time);
 
     spice_assert(!region_is_empty(&item->base.rgn));
     region_init(&exclude_rgn);
@@ -1782,9 +1768,10 @@ DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_v
         &cbs, dcc_handle_message);
     spice_return_val_if_fail(display, NULL);
 
-    stat_init(&display->add_stat, "add", red_worker_get_clockid(worker));
-    stat_init(&display->exclude_stat, "exclude", red_worker_get_clockid(worker));
-    stat_init(&display->__exclude_stat, "__exclude", red_worker_get_clockid(worker));
+    clockid_t stat_clock = red_worker_get_clockid(worker);
+    stat_init(&display->add_stat, "add", stat_clock);
+    stat_init(&display->exclude_stat, "exclude", stat_clock);
+    stat_init(&display->__exclude_stat, "__exclude", stat_clock);
 #ifdef RED_STATISTICS
     RedChannel *channel = RED_CHANNEL(display);
     display->cache_hits_counter = stat_add_counter(channel->stat,
@@ -1794,13 +1781,13 @@ DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_v
     display->non_cache_counter = stat_add_counter(channel->stat,
                                                           "non_cache", TRUE);
 #endif
-    stat_compress_init(&display->lz_stat, "lz", red_worker_get_clockid(worker));
-    stat_compress_init(&display->glz_stat, "glz", red_worker_get_clockid(worker));
-    stat_compress_init(&display->quic_stat, "quic", red_worker_get_clockid(worker));
-    stat_compress_init(&display->jpeg_stat, "jpeg", red_worker_get_clockid(worker));
-    stat_compress_init(&display->zlib_glz_stat, "zlib", red_worker_get_clockid(worker));
-    stat_compress_init(&display->jpeg_alpha_stat, "jpeg_alpha", red_worker_get_clockid(worker));
-    stat_compress_init(&display->lz4_stat, "lz4", red_worker_get_clockid(worker));
+    stat_compress_init(&display->lz_stat, "lz", stat_clock);
+    stat_compress_init(&display->glz_stat, "glz", stat_clock);
+    stat_compress_init(&display->quic_stat, "quic", stat_clock);
+    stat_compress_init(&display->jpeg_stat, "jpeg", stat_clock);
+    stat_compress_init(&display->zlib_glz_stat, "zlib", stat_clock);
+    stat_compress_init(&display->jpeg_alpha_stat, "jpeg_alpha", stat_clock);
+    stat_compress_init(&display->lz4_stat, "lz4", stat_clock);
 
     display->n_surfaces = n_surfaces;
     display->num_renderers = num_renderers;
diff --git a/server/display-channel.h b/server/display-channel.h
index 69d287a..733b4b7 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -201,10 +201,10 @@ struct DisplayChannel {
     RedCompressBuf *free_compress_bufs;
 
 /* TODO: some day unify this, make it more runtime.. */
-#ifdef RED_WORKER_STAT
     stat_info_t add_stat;
     stat_info_t exclude_stat;
     stat_info_t __exclude_stat;
+#ifdef RED_WORKER_STAT
     uint32_t add_count;
     uint32_t add_with_shadow_count;
 #endif
@@ -213,7 +213,6 @@ struct DisplayChannel {
     uint64_t *add_to_cache_counter;
     uint64_t *non_cache_counter;
 #endif
-#ifdef COMPRESS_STAT
     stat_info_t lz_stat;
     stat_info_t glz_stat;
     stat_info_t quic_stat;
@@ -221,7 +220,6 @@ struct DisplayChannel {
     stat_info_t zlib_glz_stat;
     stat_info_t jpeg_alpha_stat;
     stat_info_t lz4_stat;
-#endif
 };
 
 #define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient,   \
diff --git a/server/stat.h b/server/stat.h
index 5e8fc32..aa21f2b 100644
--- a/server/stat.h
+++ b/server/stat.h
@@ -53,12 +53,21 @@ static inline stat_time_t stat_now(clockid_t clock_id)
     return ts.tv_nsec + (uint64_t) ts.tv_sec * (1000 * 1000 * 1000);
 }
 
+typedef struct {
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
+    stat_time_t time;
+#endif
+} stat_start_time_t;
+
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
 static inline double stat_cpu_time_to_sec(stat_time_t time)
 {
     return (double)time / (1000 * 1000 * 1000);
 }
+#endif
 
-typedef struct stat_info_s {
+typedef struct {
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
     const char *name;
     clockid_t clock;
     uint32_t count;
@@ -69,37 +78,50 @@ typedef struct stat_info_s {
     uint64_t orig_size;
     uint64_t comp_size;
 #endif
+#endif
 } stat_info_t;
 
+static inline void stat_start_time_init(stat_start_time_t *tm, const stat_info_t *info)
+{
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
+    tm->time = stat_now(info->clock);
+#endif
+}
+
 static inline void stat_reset(stat_info_t *info)
 {
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
     info->count = info->max = info->total = 0;
     info->min = ~(stat_time_t)0;
 #ifdef COMPRESS_STAT
     info->orig_size = info->comp_size = 0;
 #endif
+#endif
 }
 
-#ifdef COMPRESS_STAT
 static inline void stat_compress_init(stat_info_t *info, const char *name, clockid_t clock)
 {
+#ifdef COMPRESS_STAT
     info->name = name;
     info->clock = clock;
     stat_reset(info);
+#endif
 }
 
 static inline void stat_compress_add(stat_info_t *info,
-                                     stat_time_t start, int orig_size,
+                                     stat_start_time_t start, int orig_size,
                                      int comp_size)
 {
+#ifdef COMPRESS_STAT
     stat_time_t time;
     ++info->count;
-    time = stat_now(info->clock) - start;
+    time = stat_now(info->clock) - start.time;
     info->total += time;
     info->max = MAX(info->max, time);
     info->min = MIN(info->min, time);
     info->orig_size += orig_size;
     info->comp_size += comp_size;
+#endif
 }
 
 static inline double stat_byte_to_mega(uint64_t size)
@@ -107,32 +129,25 @@ static inline double stat_byte_to_mega(uint64_t size)
     return (double)size / (1000 * 1000);
 }
 
-#else
-#define stat_compress_init(a, b, c)
-#define stat_compress_add(a, b, c, d)
-#endif
-
-#ifdef RED_WORKER_STAT
 static inline void stat_init(stat_info_t *info, const char *name, clockid_t clock)
 {
+#ifdef RED_WORKER_STAT
     info->name = name;
     info->clock = clock;
     stat_reset(info);
+#endif
 }
 
-static inline void stat_add(stat_info_t *info, stat_time_t start)
+static inline void stat_add(stat_info_t *info, stat_start_time_t start)
 {
+#ifdef RED_WORKER_STAT
     stat_time_t time;
     ++info->count;
-    time = stat_now(info->clock) - start;
+    time = stat_now(info->clock) - start.time;
     info->total += time;
     info->max = MAX(info->max, time);
     info->min = MIN(info->min, time);
+#endif
 }
 
-#else
-#define stat_add(a, b)
-#define stat_init(a, b, c)
-#endif /* RED_WORKER_STAT */
-
 #endif /* _H_STAT */
-- 
2.4.3



More information about the Spice-devel mailing list