[Spice-commits] 2 commits - server/dcc-send.c server/display-channel-private.h server/display-channel.c server/red-channel.c server/red-channel.h server/red-worker.c server/reds.c server/spicevmc.c server/stat.h

Frediano Ziglio fziglio at kemper.freedesktop.org
Wed Feb 15 10:17:14 UTC 2017


 server/dcc-send.c                |    6 +--
 server/display-channel-private.h |    8 +---
 server/display-channel.c         |   18 ++++------
 server/red-channel.c             |   29 ++++++-----------
 server/red-channel.h             |    4 +-
 server/red-worker.c              |   24 ++++++--------
 server/reds.c                    |   28 ++++++++++++----
 server/spicevmc.c                |   21 ++++++++++++
 server/stat.h                    |   65 ++++++++++++++++++++++++++++++---------
 9 files changed, 126 insertions(+), 77 deletions(-)

New commits:
commit 5a922af9e6448fa7b3cfe9c23d365ea5c8a7fba5
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Nov 16 23:25:44 2016 +0000

    spicevmc: Add some statistics
    
    Allows to see compressed/uncompressed ratio
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 679ea9d..4c9f442 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -114,6 +114,12 @@ struct RedVmcChannel
     RedVmcPipeItem *pipe_item;
     RedCharDeviceWriteBuffer *recv_from_client_buf;
     uint8_t port_opened;
+    RedStatCounter in_data;
+    RedStatCounter in_compressed;
+    RedStatCounter in_decompressed;
+    RedStatCounter out_data;
+    RedStatCounter out_compressed;
+    RedStatCounter out_uncompressed;
 };
 
 struct RedVmcChannelClass
@@ -194,6 +200,15 @@ red_vmc_channel_constructed(GObject *object)
     client_cbs.connect = spicevmc_connect;
     red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs, NULL);
 
+    red_channel_init_stat_node(RED_CHANNEL(self), NULL, "spicevmc");
+    const RedStatNode *stat = red_channel_get_stat_node(RED_CHANNEL(self));
+    stat_init_counter(&self->in_data, reds, stat, "in_data", TRUE);
+    stat_init_counter(&self->in_compressed, reds, stat, "in_compressed", TRUE);
+    stat_init_counter(&self->in_decompressed, reds, stat, "in_decompressed", TRUE);
+    stat_init_counter(&self->out_data, reds, stat, "out_data", TRUE);
+    stat_init_counter(&self->out_compressed, reds, stat, "out_compressed", TRUE);
+    stat_init_counter(&self->out_uncompressed, reds, stat, "out_uncompressed", TRUE);
+
 #ifdef USE_LZ4
     red_channel_set_cap(RED_CHANNEL(self), SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4);
 #endif
@@ -305,6 +320,8 @@ static RedVmcPipeItem* try_compress_lz4(RedVmcChannel *channel, int n, RedVmcPip
                                                  BUF_SIZE);
 
     if (compressed_data_count > 0 && compressed_data_count < n) {
+        stat_inc_counter(channel->out_uncompressed, n);
+        stat_inc_counter(channel->out_compressed, compressed_data_count);
         msg_item_compressed->type = SPICE_DATA_COMPRESSION_TYPE_LZ4;
         msg_item_compressed->uncompressed_data_size = n;
         msg_item_compressed->buf_used = compressed_data_count;
@@ -355,6 +372,7 @@ static RedPipeItem *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
             return &msg_item_compressed->base;
         }
 #endif
+        stat_inc_counter(channel->out_data, n);
         msg_item->uncompressed_data_size = n;
         msg_item->buf_used = n;
         return &msg_item->base;
@@ -532,6 +550,8 @@ static int handle_compressed_msg(RedVmcChannel *channel, RedChannelClient *rcc,
                                                  (char *)decompressed,
                                                  compressed_data_msg->compressed_size,
                                                  compressed_data_msg->uncompressed_size);
+        stat_inc_counter(channel->in_compressed, compressed_data_msg->compressed_size);
+        stat_inc_counter(channel->in_decompressed, decompressed_size);
         break;
     }
 #endif
@@ -566,6 +586,7 @@ static int spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
     switch (type) {
     case SPICE_MSGC_SPICEVMC_DATA:
         spice_assert(channel->recv_from_client_buf->buf == msg);
+        stat_inc_counter(channel->in_data, size);
         channel->recv_from_client_buf->buf_used = size;
         red_char_device_write_buffer_add(channel->chardev, channel->recv_from_client_buf);
         channel->recv_from_client_buf = NULL;
commit ce06958fb8be9337cbf283371107a7a873dc5cac
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Nov 17 16:36:37 2016 +0000

    Improve statistic code interface
    
    Use new structures and functions to implement the statistics code.
    Use inline functions instead of macros for increased type-safety.
    If statistics are disabled, the structures and functions become
    empty. This confines the configuration-specific #defines to the
    statistics implementation itself and avoids the need for #defines in
    the calling functions. This greatly reduces the chance of accidentally
    breaking the build for one configuration or the other. The reds option
    was removed from stat_inc_counter() as it was not used.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 510dfe0..b9244a6 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -197,13 +197,13 @@ static void red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
                 io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
                 dcc->priv->send_data.pixmap_cache_items[dcc->priv->send_data.num_pixmap_cache_items++] =
                                                                                image->descriptor.id;
-                stat_inc_counter(reds, display_channel->priv->add_to_cache_counter, 1);
+                stat_inc_counter(display_channel->priv->add_to_cache_counter, 1);
             }
         }
     }
 
     if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
-        stat_inc_counter(reds, display_channel->priv->non_cache_counter, 1);
+        stat_inc_counter(display_channel->priv->non_cache_counter, 1);
     }
 }
 
@@ -393,7 +393,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
                                      &bitmap_palette_out, &lzplt_palette_out);
                 spice_assert(bitmap_palette_out == NULL);
                 spice_assert(lzplt_palette_out == NULL);
-                stat_inc_counter(reds, display->priv->cache_hits_counter, 1);
+                stat_inc_counter(display->priv->cache_hits_counter, 1);
                 pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
                 return FILL_BITS_TYPE_CACHE;
             } else {
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index a727ddb..da807d1 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -77,11 +77,9 @@ struct DisplayChannelPrivate
     uint32_t add_count;
     uint32_t add_with_shadow_count;
 #endif
-#ifdef RED_STATISTICS
-    uint64_t *cache_hits_counter;
-    uint64_t *add_to_cache_counter;
-    uint64_t *non_cache_counter;
-#endif
+    RedStatCounter cache_hits_counter;
+    RedStatCounter add_to_cache_counter;
+    RedStatCounter non_cache_counter;
     ImageEncoderSharedData encoder_shared_data;
 };
 
diff --git a/server/display-channel.c b/server/display-channel.c
index 9a9385f..288969c 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2043,18 +2043,14 @@ display_channel_constructed(GObject *object)
     stat_init(&self->priv->add_stat, "add", CLOCK_THREAD_CPUTIME_ID);
     stat_init(&self->priv->exclude_stat, "exclude", CLOCK_THREAD_CPUTIME_ID);
     stat_init(&self->priv->__exclude_stat, "__exclude", CLOCK_THREAD_CPUTIME_ID);
-#ifdef RED_STATISTICS
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
-    self->priv->cache_hits_counter =
-        stat_add_counter(reds, red_channel_get_stat_node(channel),
-                         "cache_hits", TRUE);
-    self->priv->add_to_cache_counter =
-        stat_add_counter(reds, red_channel_get_stat_node(channel),
-                         "add_to_cache", TRUE);
-    self->priv->non_cache_counter =
-        stat_add_counter(reds, red_channel_get_stat_node(channel),
-                         "non_cache", TRUE);
-#endif
+    const RedStatNode *stat = red_channel_get_stat_node(channel);
+    stat_init_counter(&self->priv->cache_hits_counter, reds, stat,
+                      "cache_hits", TRUE);
+    stat_init_counter(&self->priv->add_to_cache_counter, reds, stat,
+                      "add_to_cache", TRUE);
+    stat_init_counter(&self->priv->non_cache_counter, reds, stat,
+                      "non_cache", TRUE);
     image_cache_init(&self->priv->image_cache);
     self->priv->stream_video = SPICE_STREAM_VIDEO_OFF;
     display_channel_init_streams(self);
diff --git a/server/red-channel.c b/server/red-channel.c
index e1ea6d1..2f9173b 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -98,10 +98,8 @@ struct RedChannelPrivate
     // from Channel -> need to protect!
     pthread_t thread_id;
     RedsState *reds;
-#ifdef RED_STATISTICS
-    StatNodeRef stat;
-    uint64_t *out_bytes_counter;
-#endif
+    RedStatNode stat;
+    RedStatCounter out_bytes_counter;
 };
 
 enum {
@@ -198,9 +196,7 @@ red_channel_finalize(GObject *object)
 
 void red_channel_on_output(RedChannel *self, int n)
 {
-#ifdef RED_STATISTICS
-    stat_inc_counter(self->priv->reds, self->priv->out_bytes_counter, n);
-#endif
+    stat_inc_counter(self->priv->out_bytes_counter, n);
 }
 
 static void
@@ -375,24 +371,19 @@ int red_channel_is_waiting_for_migrate_data(RedChannel *channel)
     return red_channel_client_is_waiting_for_migrate_data(rcc);
 }
 
-void red_channel_set_stat_node(RedChannel *channel, StatNodeRef stat)
+void red_channel_init_stat_node(RedChannel *channel, const RedStatNode *parent, const char *name)
 {
     spice_return_if_fail(channel != NULL);
-#ifdef RED_STATISTICS
-    spice_return_if_fail(channel->priv->stat == 0);
 
-    channel->priv->stat = stat;
-    channel->priv->out_bytes_counter =
-        stat_add_counter(channel->priv->reds, stat, "out_bytes", TRUE);
-#endif
+    // TODO check not already initialized
+    stat_init_node(&channel->priv->stat, channel->priv->reds, parent, name, TRUE);
+    stat_init_counter(&channel->priv->out_bytes_counter,
+                      channel->priv->reds, &channel->priv->stat, "out_bytes", TRUE);
 }
 
-StatNodeRef red_channel_get_stat_node(RedChannel *channel)
+const RedStatNode *red_channel_get_stat_node(RedChannel *channel)
 {
-#ifdef RED_STATISTICS
-    return channel->priv->stat;
-#endif
-    return 0;
+    return &channel->priv->stat;
 }
 
 void red_channel_register_client_cbs(RedChannel *channel, const ClientCbs *client_cbs,
diff --git a/server/red-channel.h b/server/red-channel.h
index e22f96e..79aee01 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -147,7 +147,7 @@ GType red_channel_get_type(void) G_GNUC_CONST;
 void red_channel_add_client(RedChannel *channel, RedChannelClient *rcc);
 void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc);
 
-void red_channel_set_stat_node(RedChannel *channel, StatNodeRef stat);
+void red_channel_init_stat_node(RedChannel *channel, const RedStatNode *parent, const char *name);
 
 void red_channel_register_client_cbs(RedChannel *channel, const ClientCbs *client_cbs, gpointer cbs_data);
 // caps are freed when the channel is destroyed
@@ -242,7 +242,7 @@ void red_channel_on_disconnect(RedChannel *self, RedChannelClient *rcc);
 void red_channel_on_output(RedChannel *self, int n);
 void red_channel_send_item(RedChannel *self, RedChannelClient *rcc, RedPipeItem *item);
 void red_channel_reset_thread_id(RedChannel *self);
-StatNodeRef red_channel_get_stat_node(RedChannel *channel);
+const RedStatNode *red_channel_get_stat_node(RedChannel *channel);
 
 const RedChannelCapabilities* red_channel_get_local_capabilities(RedChannel *self);
 
diff --git a/server/red-worker.c b/server/red-worker.c
index 475acc4..e5adbaa 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -76,11 +76,9 @@ struct RedWorker {
     spice_wan_compression_t zlib_glz_state;
 
     uint32_t process_display_generation;
-#ifdef RED_STATISTICS
-    StatNodeRef stat;
-    uint64_t *wakeup_counter;
-    uint64_t *command_counter;
-#endif
+    RedStatNode stat;
+    RedStatCounter wakeup_counter;
+    RedStatCounter command_counter;
 
     int driver_cap_monitors_config;
 
@@ -213,7 +211,7 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
             red_record_qxl_command(worker->record, &worker->mem_slots, ext_cmd);
         }
 
-        stat_inc_counter(reds, worker->command_counter, 1);
+        stat_inc_counter(worker->command_counter, 1);
         worker->display_poll_tries = 0;
         switch (ext_cmd.cmd.type) {
         case QXL_CMD_DRAW: {
@@ -662,7 +660,7 @@ static void handle_dev_wakeup(void *opaque, void *payload)
 {
     RedWorker *worker = opaque;
 
-    stat_inc_counter(reds, worker->wakeup_counter, 1);
+    stat_inc_counter(worker->wakeup_counter, 1);
     red_qxl_clear_pending(worker->qxl->st, RED_DISPATCHER_PENDING_WAKEUP);
 }
 
@@ -1341,13 +1339,11 @@ RedWorker* red_worker_new(QXLInstance *qxl,
     worker->jpeg_state = reds_get_jpeg_state(reds);
     worker->zlib_glz_state = reds_get_zlib_glz_state(reds);
     worker->driver_cap_monitors_config = 0;
-#ifdef RED_STATISTICS
     char worker_str[20];
     sprintf(worker_str, "display[%d]", worker->qxl->id);
-    worker->stat = stat_add_node(reds, INVALID_STAT_REF, worker_str, TRUE);
-    worker->wakeup_counter = stat_add_counter(reds, worker->stat, "wakeups", TRUE);
-    worker->command_counter = stat_add_counter(reds, worker->stat, "commands", TRUE);
-#endif
+    stat_init_node(&worker->stat, reds, NULL, worker_str, TRUE);
+    stat_init_counter(&worker->wakeup_counter, reds, &worker->stat, "wakeups", TRUE);
+    stat_init_counter(&worker->command_counter, reds, &worker->stat, "commands", TRUE);
 
     worker->dispatch_watch =
         worker->core.watch_add(&worker->core, dispatcher_get_recv_fd(dispatcher),
@@ -1371,7 +1367,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
     worker->cursor_channel = cursor_channel_new(reds, qxl,
                                                 &worker->core);
     channel = RED_CHANNEL(worker->cursor_channel);
-    red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat, "cursor_channel", TRUE));
+    red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
     red_channel_register_client_cbs(channel, client_cursor_cbs, dispatcher);
     g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
     reds_register_channel(reds, channel);
@@ -1382,7 +1378,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
                                                   reds_get_video_codecs(reds),
                                                   init_info.n_surfaces);
     channel = RED_CHANNEL(worker->display_channel);
-    red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat, "display_channel", TRUE));
+    red_channel_init_stat_node(channel, &worker->stat, "display_channel");
     red_channel_register_client_cbs(channel, client_display_cbs, dispatcher);
     g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
     reds_register_channel(reds, channel);
diff --git a/server/reds.c b/server/reds.c
index 953a95a..39a7a31 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -352,25 +352,37 @@ static void reds_link_free(RedLinkInfo *link)
 
 #ifdef RED_STATISTICS
 
-StatNodeRef stat_add_node(RedsState *reds, StatNodeRef parent, const char *name, int visible)
+void stat_init_node(RedStatNode *node, SpiceServer *reds, const RedStatNode *parent,
+                    const char *name, int visible)
 {
-    return stat_file_add_node(reds->stat_file, parent, name, visible);
+    StatNodeRef parent_ref = parent ? parent->ref : INVALID_STAT_REF;
+    node->ref = stat_file_add_node(reds->stat_file, parent_ref, name, visible);
 }
 
-void stat_remove_node(RedsState *reds, StatNodeRef ref)
+void stat_remove_node(SpiceServer *reds, RedStatNode *node)
 {
-    stat_file_remove_node(reds->stat_file, ref);
+    if (node->ref != INVALID_STAT_REF) {
+        stat_file_remove_node(reds->stat_file, node->ref);
+        node->ref = INVALID_STAT_REF;
+    }
 }
 
-uint64_t *stat_add_counter(RedsState *reds, StatNodeRef parent, const char *name, int visible)
+void stat_init_counter(RedStatCounter *counter, SpiceServer *reds,
+                       const RedStatNode *parent, const char *name, int visible)
 {
-    return stat_file_add_counter(reds->stat_file, parent, name, visible);
+    StatNodeRef parent_ref = parent ? parent->ref : INVALID_STAT_REF;
+    counter->counter =
+        stat_file_add_counter(reds->stat_file, parent_ref, name, visible);
 }
 
-void stat_remove_counter(RedsState *reds, uint64_t *counter)
+void stat_remove_counter(SpiceServer *reds, RedStatCounter *counter)
 {
-    stat_file_remove_counter(reds->stat_file, counter);
+    if (counter->counter) {
+        stat_file_remove_counter(reds->stat_file, counter->counter);
+        counter->counter = NULL;
+    }
 }
+
 #endif
 
 void reds_register_channel(RedsState *reds, RedChannel *channel)
diff --git a/server/stat.h b/server/stat.h
index a5df7ea..5255efa 100644
--- a/server/stat.h
+++ b/server/stat.h
@@ -24,26 +24,61 @@
 #include "spice.h"
 #include "stat-file.h"
 
+typedef struct {
 #ifdef RED_STATISTICS
-StatNodeRef stat_add_node(SpiceServer *reds, StatNodeRef parent, const char *name, int visible);
-void stat_remove_node(SpiceServer *reds, StatNodeRef node);
-uint64_t *stat_add_counter(SpiceServer *reds, StatNodeRef parent, const char *name, int visible);
-void stat_remove_counter(SpiceServer *reds, uint64_t *counter);
-
-#define stat_inc_counter(reds, counter, value) {  \
-    if (counter) {                          \
-        *(counter) += (value);              \
-    }                                       \
-}
+    uint64_t *counter;
+#endif
+} RedStatCounter;
+
+typedef struct {
+#ifdef RED_STATISTICS
+    uint32_t ref;
+#endif
+} RedStatNode;
+
+#ifdef RED_STATISTICS
+void stat_init_node(RedStatNode *node, SpiceServer *reds,
+                    const RedStatNode *parent, const char *name, int visible);
+void stat_remove_node(SpiceServer *reds, RedStatNode *node);
+void stat_init_counter(RedStatCounter *counter, SpiceServer *reds,
+                       const RedStatNode *parent, const char *name, int visible);
+void stat_remove_counter(SpiceServer *reds, RedStatCounter *counter);
 
 #else
-#define stat_add_node(r, p, n, v) INVALID_STAT_REF
-#define stat_remove_node(r, n)
-#define stat_add_counter(r, p, n, v) NULL
-#define stat_remove_counter(r, c)
-#define stat_inc_counter(r, c, v)
+
+static inline void
+stat_init_node(RedStatNode *node, SpiceServer *reds,
+               const RedStatNode *parent, const char *name, int visible)
+{
+}
+
+static inline void
+stat_remove_node(SpiceServer *reds, RedStatNode *node)
+{
+}
+
+static inline void
+stat_init_counter(RedStatCounter *counter, SpiceServer *reds,
+                  const RedStatNode *parent, const char *name, int visible)
+{
+}
+
+static inline void
+stat_remove_counter(SpiceServer *reds, RedStatCounter *counter)
+{
+}
 #endif /* RED_STATISTICS */
 
+static inline void
+stat_inc_counter(RedStatCounter counter, uint64_t value)
+{
+#ifdef RED_STATISTICS
+    if (counter.counter) {
+        *(counter.counter) += value;
+    }
+#endif
+}
+
 typedef uint64_t stat_time_t;
 
 static inline stat_time_t stat_now(clockid_t clock_id)


More information about the Spice-commits mailing list