Re: [PATCH 01/17] Add RedsState arg to all stat functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> 
> ---
>  server/dcc-send.c        |  6 +++---
>  server/display-channel.c | 12 ++++++------
>  server/main-channel.c    |  2 +-
>  server/red-channel.c     |  4 ++--
>  server/red-worker.c      | 12 ++++++------
>  server/reds.c            | 22 +++++++++++-----------
>  server/reds.h            |  3 ---
>  server/stat.h            | 22 ++++++++++++----------
>  8 files changed, 41 insertions(+), 42 deletions(-)
> 
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 3af5760..ca7e7e2 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -207,13 +207,13 @@ static void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
>                  io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
>                  dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++]
>                  =
>                                                                                 image->descriptor.id;
> -                stat_inc_counter(display_channel->add_to_cache_counter, 1);
> +                reds_stat_inc_counter(reds,
> display_channel->add_to_cache_counter, 1);

Before I forget, I would rather avoid to add reds_ prefix to these functions/macros.

>              }
>          }
>      }
>  
>      if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
> -        stat_inc_counter(display_channel->non_cache_counter, 1);
> +        reds_stat_inc_counter(reds, display_channel->non_cache_counter, 1);
>      }
>  }
>  
> @@ -376,7 +376,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(display->cache_hits_counter, 1);
> +                reds_stat_inc_counter(reds, display->cache_hits_counter, 1);
>                  pthread_mutex_unlock(&dcc->pixmap_cache->lock);
>                  return FILL_BITS_TYPE_CACHE;
>              } else {
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 78c984f..5b496ae 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2047,12 +2047,12 @@ DisplayChannel* display_channel_new(RedWorker
> *worker, int migrate, int stream_v
>      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,
> -                                                           "cache_hits",
> TRUE);
> -    display->add_to_cache_counter = stat_add_counter(channel->stat,
> -                                                             "add_to_cache",
> TRUE);
> -    display->non_cache_counter = stat_add_counter(channel->stat,
> -                                                          "non_cache",
> TRUE);
> +    display->cache_hits_counter = reds_stat_add_counter(reds, channel->stat,
> +                                                        "cache_hits", TRUE);
> +    display->add_to_cache_counter = reds_stat_add_counter(reds,
> channel->stat,
> +                                                          "add_to_cache",
> TRUE);
> +    display->non_cache_counter = reds_stat_add_counter(reds, channel->stat,
> +                                                       "non_cache", TRUE);
>  #endif
>      stat_compress_init(&display->lz_stat, "lz", stat_clock);
>      stat_compress_init(&display->glz_stat, "glz", stat_clock);
> diff --git a/server/main-channel.c b/server/main-channel.c
> index d69095d..8b1d7d4 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -985,7 +985,7 @@ static int main_channel_handle_parsed(RedChannelClient
> *rcc, uint32_t size, uint
>              red_channel_client_handle_message(rcc, size, type, message);
>          }
>  #ifdef RED_STATISTICS
> -        reds_update_stat_value(roundtrip);
> +        reds_update_stat_value(reds, roundtrip);
>  #endif
>          break;
>      }
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 190779a..a6069a9 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -396,7 +396,7 @@ static void red_channel_client_on_output(void *opaque,
> int n)
>      if (rcc->connectivity_monitor.timer) {
>          rcc->connectivity_monitor.out_bytes += n;
>      }
> -    stat_inc_counter(rcc->channel->out_bytes_counter, n);
> +    reds_stat_inc_counter(reds, rcc->channel->out_bytes_counter, n);
>  }
>  
>  static void red_channel_client_on_input(void *opaque, int n)
> @@ -1162,7 +1162,7 @@ void red_channel_set_stat_node(RedChannel *channel,
> StatNodeRef stat)
>  
>  #ifdef RED_STATISTICS
>      channel->stat = stat;
> -    channel->out_bytes_counter = stat_add_counter(stat, "out_bytes", TRUE);
> +    channel->out_bytes_counter = reds_stat_add_counter(reds, stat,
> "out_bytes", TRUE);
>  #endif
>  }
>  
> diff --git a/server/red-worker.c b/server/red-worker.c
> index e7d0671..7e5742f 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -244,7 +244,7 @@ static int red_process_display(RedWorker *worker, int
> *ring_is_empty)
>              red_record_qxl_command(worker->record_fd, &worker->mem_slots,
>              ext_cmd,
>                                     stat_now(CLOCK_MONOTONIC));
>  
> -        stat_inc_counter(worker->command_counter, 1);
> +        reds_stat_inc_counter(reds, worker->command_counter, 1);
>          worker->display_poll_tries = 0;
>          switch (ext_cmd.cmd.type) {
>          case QXL_CMD_DRAW: {
> @@ -521,7 +521,7 @@ CommonChannel *red_worker_new_channel(RedWorker *worker,
> int size,
>                                          channel_cbs,
>                                          migration_flags);
>      spice_return_val_if_fail(channel, NULL);
> -    red_channel_set_stat_node(channel, stat_add_node(worker->stat, name,
> TRUE));
> +    red_channel_set_stat_node(channel, reds_stat_add_node(reds,
> worker->stat, name, TRUE));
>  
>      common = (CommonChannel *)channel;
>      common->worker = worker;
> @@ -856,7 +856,7 @@ static void handle_dev_wakeup(void *opaque, void
> *payload)
>  {
>      RedWorker *worker = opaque;
>  
> -    stat_inc_counter(worker->wakeup_counter, 1);
> +    reds_stat_inc_counter(reds, worker->wakeup_counter, 1);
>      red_dispatcher_clear_pending(worker->red_dispatcher,
>      RED_DISPATCHER_PENDING_WAKEUP);
>  }
>  
> @@ -1541,9 +1541,9 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> RedDispatcher *red_dispatcher)
>  #ifdef RED_STATISTICS
>      char worker_str[20];
>      sprintf(worker_str, "display[%d]", worker->qxl->id);
> -    worker->stat = stat_add_node(INVALID_STAT_REF, worker_str, TRUE);
> -    worker->wakeup_counter = stat_add_counter(worker->stat, "wakeups",
> TRUE);
> -    worker->command_counter = stat_add_counter(worker->stat, "commands",
> TRUE);
> +    worker->stat = reds_stat_add_node(reds, INVALID_STAT_REF, worker_str,
> TRUE);
> +    worker->wakeup_counter = reds_stat_add_counter(reds, worker->stat,
> "wakeups", TRUE);
> +    worker->command_counter = reds_stat_add_counter(reds, worker->stat,
> "commands", TRUE);
>  #endif
>  
>      worker->dispatch_watch =
> diff --git a/server/reds.c b/server/reds.c
> index 16327b5..c33aded 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -216,7 +216,7 @@ static void reds_link_free(RedLinkInfo *link)
>  
>  #ifdef RED_STATISTICS
>  
> -static void insert_stat_node(StatNodeRef parent, StatNodeRef ref)
> +static void reds_insert_stat_node(RedsState *reds, StatNodeRef parent,
> StatNodeRef ref)
>  {
>      SpiceStatNode *node = &reds->stat->nodes[ref];
>      uint32_t pos = INVALID_STAT_REF;
> @@ -243,7 +243,7 @@ static void insert_stat_node(StatNodeRef parent,
> StatNodeRef ref)
>      }
>  }
>  
> -StatNodeRef stat_add_node(StatNodeRef parent, const char *name, int visible)
> +StatNodeRef reds_stat_add_node(RedsState *reds, StatNodeRef parent, const
> char *name, int visible)
>  {
>      StatNodeRef ref;
>      SpiceStatNode *node;
> @@ -280,12 +280,12 @@ StatNodeRef stat_add_node(StatNodeRef parent, const
> char *name, int visible)
>      node->value = 0;
>      node->flags = SPICE_STAT_NODE_FLAG_ENABLED | (visible ?
>      SPICE_STAT_NODE_FLAG_VISIBLE : 0);
>      g_strlcpy(node->name, name, sizeof(node->name));
> -    insert_stat_node(parent, ref);
> +    reds_insert_stat_node(reds, parent, ref);
>      pthread_mutex_unlock(&reds->stat_lock);
>      return ref;
>  }
>  
> -static void stat_remove(SpiceStatNode *node)
> +static void reds_stat_remove(RedsState *reds, SpiceStatNode *node)
>  {
>      pthread_mutex_lock(&reds->stat_lock);
>      node->flags &= ~SPICE_STAT_NODE_FLAG_ENABLED;
> @@ -294,14 +294,14 @@ static void stat_remove(SpiceStatNode *node)
>      pthread_mutex_unlock(&reds->stat_lock);
>  }
>  
> -void stat_remove_node(StatNodeRef ref)
> +void reds_stat_remove_node(RedsState *reds, StatNodeRef ref)
>  {
> -    stat_remove(&reds->stat->nodes[ref]);
> +    reds_stat_remove(reds, &reds->stat->nodes[ref]);
>  }
>  
> -uint64_t *stat_add_counter(StatNodeRef parent, const char *name, int
> visible)
> +uint64_t *reds_stat_add_counter(RedsState *reds, StatNodeRef parent, const
> char *name, int visible)
>  {
> -    StatNodeRef ref = stat_add_node(parent, name, visible);
> +    StatNodeRef ref = reds_stat_add_node(reds, parent, name, visible);
>      SpiceStatNode *node;
>  
>      if (ref == INVALID_STAT_REF) {
> @@ -312,12 +312,12 @@ uint64_t *stat_add_counter(StatNodeRef parent, const
> char *name, int visible)
>      return &node->value;
>  }
>  
> -void stat_remove_counter(uint64_t *counter)
> +void reds_stat_remove_counter(RedsState *reds, uint64_t *counter)
>  {
> -    stat_remove((SpiceStatNode *)(counter - offsetof(SpiceStatNode,
> value)));
> +    reds_stat_remove(reds, (SpiceStatNode *)(counter -
> offsetof(SpiceStatNode, value)));
>  }
>  
> -void reds_update_stat_value(uint32_t value)
> +void reds_update_stat_value(RedsState *reds, uint32_t value)
>  {
>      RedsStatValue *stat_value = &reds->roundtrip_stat;
>  
> diff --git a/server/reds.h b/server/reds.h
> index d9f4e83..6caed73 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -85,9 +85,6 @@ typedef struct MainMigrateData MainMigrateData;
>  void reds_marshall_migrate_data(RedsState *reds, SpiceMarshaller *m);
>  void reds_fill_channels(RedsState *reds, SpiceMsgChannels *channels_info);
>  int reds_get_n_channels(RedsState *reds);
> -#ifdef RED_STATISTICS
> -void reds_update_stat_value(uint32_t value);
> -#endif
>  
>  /* callbacks from main channel messages */
>  
> diff --git a/server/stat.h b/server/stat.h
> index 6cb0046..ad0ea96 100644
> --- a/server/stat.h
> +++ b/server/stat.h
> @@ -23,25 +23,27 @@
>  
>  typedef uint32_t StatNodeRef;
>  #define INVALID_STAT_REF (~(StatNodeRef)0)
> +typedef struct RedsState RedsState;
>  

I don't like this typedef here, is out of "standard".
We usually typedef in the same header we define it, unless is private.

>  #ifdef RED_STATISTICS
> -StatNodeRef stat_add_node(StatNodeRef parent, const char *name, int
> visible);
> -void stat_remove_node(StatNodeRef node);
> -uint64_t *stat_add_counter(StatNodeRef parent, const char *name, int
> visible);
> -void stat_remove_counter(uint64_t *counter);
> +StatNodeRef reds_stat_add_node(RedsState *reds, StatNodeRef parent, const
> char *name, int visible);
> +void reds_stat_remove_node(RedsState *reds, StatNodeRef node);
> +uint64_t *reds_stat_add_counter(RedsState *reds, StatNodeRef parent, const
> char *name, int visible);
> +void reds_stat_remove_counter(RedsState *reds, uint64_t *counter);
> +void reds_update_stat_value(RedsState *reds, uint32_t value);
>  
> -#define stat_inc_counter(counter, value) {  \
> +#define reds_stat_inc_counter(reds, counter, value) {  \
>      if (counter) {                          \
>          *(counter) += (value);              \
>      }                                       \
>  }
>  
>  #else
> -#define stat_add_node(p, n, v) INVALID_STAT_REF
> -#define stat_remove_node(n)
> -#define stat_add_counter(p, n, v) NULL
> -#define stat_remove_counter(c)
> -#define stat_inc_counter(c, v)
> +#define reds_stat_add_node(r, p, n, v) INVALID_STAT_REF
> +#define reds_stat_remove_node(r, n)
> +#define reds_stat_add_counter(r, p, n, v) NULL
> +#define reds_stat_remove_counter(r, c)
> +#define reds_stat_inc_counter(r, c, v)
>  #endif /* RED_STATISTICS */
>  
>  typedef uint64_t stat_time_t;

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]