[Spice-devel] [RFC PATH 1/2] stat: use a better design for statistic functions

Frediano Ziglio fziglio at redhat.com
Tue Nov 17 05:58:28 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/display-channel.c |  2 --
 server/display-channel.h |  4 +---
 server/red_worker.c      | 62 ++++++++++++++++++++----------------------------
 server/stat.h            | 57 ++++++++++++++++++++++++++++----------------
 4 files changed, 64 insertions(+), 61 deletions(-)

diff --git a/server/display-channel.c b/server/display-channel.c
index 63d56b4..e26fec4 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -24,7 +24,6 @@ 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);
@@ -32,7 +31,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)
diff --git a/server/display-channel.h b/server/display-channel.h
index 40a3dc6..198b679 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -325,10 +325,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
@@ -337,7 +337,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;
@@ -345,7 +344,6 @@ struct DisplayChannel {
     stat_info_t zlib_glz_stat;
     stat_info_t jpeg_alpha_stat;
     stat_info_t lz4_stat;
-#endif
 };
 typedef struct SurfaceDestroyItem {
     SpiceMsgSurfaceDestroy surface_destroy;
diff --git a/server/red_worker.c b/server/red_worker.c
index 3a718fd..66f8ce2 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -66,7 +66,6 @@
 #include "cursor-channel.h"
 #include "tree.h"
 
-//#define COMPRESS_STAT
 //#define DUMP_BITMAP
 //#define COMPRESS_DEBUG
 
@@ -1192,9 +1191,8 @@ static inline void __exclude_region(RedWorker *worker, Ring *ring, TreeItem *ite
                                     Ring **top_ring, Drawable *frame_candidate)
 {
     QRegion and_rgn;
-#ifdef RED_WORKER_STAT
-    stat_time_t start_time = stat_now(worker->clockid);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &worker->display_channel->__exclude_stat);
 
     region_clone(&and_rgn, rgn);
     region_and(&and_rgn, &item->rgn);
@@ -1262,9 +1260,8 @@ static inline void __exclude_region(RedWorker *worker, Ring *ring, TreeItem *ite
 static void exclude_region(RedWorker *worker, Ring *ring, RingItem *ring_item, QRegion *rgn,
                            TreeItem **last, Drawable *frame_candidate)
 {
-#ifdef RED_WORKER_STAT
-    stat_time_t start_time = stat_now(worker->clockid);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &worker->display_channel->exclude_stat);
     Ring *top_ring;
 
     if (!ring_item) {
@@ -2219,9 +2216,8 @@ static void red_use_stream_trace(DisplayChannel *display, Drawable *drawable)
 static inline int current_add(RedWorker *worker, Ring *ring, Drawable *drawable)
 {
     DrawItem *item = &drawable->tree_item;
-#ifdef RED_WORKER_STAT
-    stat_time_t start_time = stat_now(worker->clockid);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &worker->display_channel->add_stat);
     RingItem *now;
     QRegion exclude_rgn;
     RingItem *exclude_base = NULL;
@@ -2338,8 +2334,9 @@ static inline int current_add(RedWorker *worker, Ring *ring, Drawable *drawable)
 static inline int current_add_with_shadow(RedWorker *worker, Ring *ring, Drawable *item)
 {
     DisplayChannel *display = worker->display_channel;
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &display->add_stat);
 #ifdef RED_WORKER_STAT
-    stat_time_t start_time = stat_now(worker->clockid);
     ++display->add_with_shadow_count;
 #endif
 
@@ -4140,9 +4137,8 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
 {
     DisplayChannel *display_channel = DCC_TO_DC(dcc);
     RedWorker *worker = display_channel->common.worker;
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(display_channel->zlib_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;
@@ -4183,9 +4179,7 @@ static inline int red_glz_compress_image(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 = &worker->zlib_data;
 
     zlib_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
@@ -4246,9 +4240,8 @@ static inline int red_lz_compress_image(DisplayChannelClient *dcc,
     LzImageType type = MAP_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 = red_display_alloc_compress_buf(dcc);
     lz_data->data.bufs_head = lz_data->data.bufs_tail;
@@ -4331,10 +4324,9 @@ static int red_jpeg_compress_image(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_alpha_stat.clock);
-#endif
     switch (src->format) {
     case SPICE_BITMAP_FMT_16BIT:
         jpeg_in_type = JPEG_IMAGE_TYPE_RGB16;
@@ -4466,9 +4458,8 @@ static int red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
     Lz4EncoderContext *lz4 = worker->lz4;
     int lz4_size = 0;
 
-#ifdef COMPRESS_STAT
-    stat_time_t start_time = stat_now(worker->clockid);
-#endif
+    stat_start_time_t start_time;
+    stat_start_time_init(&start_time, &display_channel->lz4_stat);
 
     lz4_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
     lz4_data->data.bufs_head = lz4_data->data.bufs_tail;
@@ -4532,9 +4523,8 @@ static inline int red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
     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:
@@ -8534,13 +8524,13 @@ static void display_channel_create(RedWorker *worker, int migrate, int stream_vi
     display_channel->non_cache_counter = stat_add_counter(channel->stat,
                                                           "non_cache", TRUE);
 #endif
-    stat_compress_init(&display_channel->lz_stat, "lz");
-    stat_compress_init(&display_channel->glz_stat, "glz");
-    stat_compress_init(&display_channel->quic_stat, "quic");
-    stat_compress_init(&display_channel->jpeg_stat, "jpeg");
-    stat_compress_init(&display_channel->zlib_glz_stat, "zlib");
-    stat_compress_init(&display_channel->jpeg_alpha_stat, "jpeg_alpha");
-    stat_compress_init(&display_channel->lz4_stat, "lz4");
+    stat_compress_init(&display_channel->lz_stat, "lz", worker->clockid);
+    stat_compress_init(&display_channel->glz_stat, "glz", worker->clockid);
+    stat_compress_init(&display_channel->quic_stat, "quic", worker->clockid);
+    stat_compress_init(&display_channel->jpeg_stat, "jpeg", worker->clockid);
+    stat_compress_init(&display_channel->zlib_glz_stat, "zlib", worker->clockid);
+    stat_compress_init(&display_channel->jpeg_alpha_stat, "jpeg_alpha", worker->clockid);
+    stat_compress_init(&display_channel->lz4_stat, "lz4", worker->clockid);
 
     display_channel->num_renderers = num_renderers;
     memcpy(display_channel->renderers, renderers, sizeof(display_channel->renderers));
diff --git a/server/stat.h b/server/stat.h
index 3c980d0..4f961ad 100644
--- a/server/stat.h
+++ b/server/stat.h
@@ -43,22 +43,31 @@ void stat_remove_counter(uint64_t *counter);
 #define stat_inc_counter(c, v)
 #endif /* RED_STATISTICS */
 
-typedef unsigned long stat_time_t;
+typedef uint64_t stat_time_t;
 
-static inline stat_time_t stat_now(clockid_t clock_id)
+static inline uint64_t stat_now(clockid_t clock_id)
 {
     struct timespec ts;
 
     clock_gettime(clock_id, &ts);
-    return ts.tv_nsec + ts.tv_sec * 1000 * 1000 * 1000;
+    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,36 +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)
+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)
@@ -106,32 +129,26 @@ static inline double stat_byte_to_mega(uint64_t size)
     return (double)size / (1000 * 1000);
 }
 
-#else
-#define stat_compress_init(a, b)
-#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