[Spice-devel] [PATCH spice-server 1/2] Improve statistic code interface
Frediano Ziglio
fziglio at redhat.com
Fri Dec 2 15:24:19 UTC 2016
Use new structures and functions to implement statistic code.
Instead of using macro use inline functions.
Inline functions are more type safe.
If statistics are disabled structure and functions became empty;
this allow the code to work and compile with either statistic
disabled or enabled not requiring extra compilation to understand
if the code will still work if these options are enabled.
Also this way reduce the preprocessor usage.
The reds option were removed from stat_inc_counter as not used
(if parameter were used code wouldn't compile).
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/dcc-send.c | 6 ++--
server/display-channel-private.h | 8 ++---
server/display-channel.c | 18 +++++------
server/red-channel.c | 27 +++++++----------
server/red-channel.h | 4 +--
server/red-worker.c | 24 +++++++--------
server/reds.c | 28 ++++++++++++-----
server/stat.h | 65 ++++++++++++++++++++++++++++++----------
8 files changed, 105 insertions(+), 75 deletions(-)
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 4c739c8..e2d2b37 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);
}
}
@@ -384,7 +384,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 38330da..f5ad6c8 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -65,11 +65,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 52f0a3d..358fd99 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2037,18 +2037,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 fb2c8c1..03443ba 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -101,10 +101,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 {
@@ -212,7 +210,7 @@ static void red_channel_on_output(void *opaque, int n)
red_channel_client_on_output(opaque, n);
#ifdef RED_STATISTICS
self = red_channel_client_get_channel((RedChannelClient *)opaque);
- stat_inc_counter(self->priv->reds, self->priv->out_bytes_counter, n);
+ stat_inc_counter(self->priv->out_bytes_counter, n);
#endif
}
@@ -408,24 +406,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 2c99139..b201daf 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -219,7 +219,7 @@ RedChannel *red_channel_create_parser(int size,
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
@@ -314,7 +314,7 @@ int red_channel_config_socket(RedChannel *self, RedChannelClient *rcc);
void red_channel_on_disconnect(RedChannel *self, RedChannelClient *rcc);
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);
/* FIXME: do these even need to be in RedChannel? It's really only used in
* RedChannelClient. Needs refactoring */
diff --git a/server/red-worker.c b/server/red-worker.c
index d699bd6..212527d 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;
@@ -195,7 +193,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: {
@@ -652,7 +650,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);
}
@@ -1344,13 +1342,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),
@@ -1374,7 +1370,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);
@@ -1385,7 +1381,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 75f90c5..c9039d6 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -350,25 +350,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)
--
2.9.3
More information about the Spice-devel
mailing list