Re: [PATCH spice-server v2 2/2] red-channel: Initialize base statistic node creating the object

[Christophe Fergeau <cfergeau@xxxxxxxxxx>]

On Mon, Mar 27, 2017 at 07:06:30AM -0400, Frediano Ziglio wrote:
> > The stat counters only seem to be used once we have a
> > DisplayChannelClient though, so we could init everything after the
> > DisplayChannel has been created, ie:
> > 
> >     worker->display_channel = display_channel_new(reds, qxl, &worker->core,
> >     FALSE,
> >                                                   reds_get_streaming_video(reds),
> >                                                   reds_get_video_codecs(reds),
> >                                                   init_info.n_surfaces);
> >     channel = RED_CHANNEL(worker->display_channel);
> >     display_channel_init_stats(display_channel, &worker->stat,
> >     "display_channel");
> > 
> 
> This is the way is done currently and as I said this does not work
> as the counters are initialized in display_channel_new so before
> display_channel_init_stats is called.

If we have a display_channel_init_stats() method, then the counter
creation can be done in that method rather than in _constructed. I
assume we don't need the stat counters that early in the DisplayChannel
lifetime?

My suggestion (only compile-tested):

diff --git a/server/display-channel.c b/server/display-channel.c
index a35145c..ac70be3 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2127,14 +2127,6 @@ 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);
-    RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
-    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);
@@ -2145,6 +2137,21 @@ display_channel_constructed(GObject *object)
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
 }
 
+void display_channel_init_stat_node(DisplayChannel *self, RedStatNode *parent)
+{
+    const RedStatNode *stat = red_channel_get_stat_node(RED_CHANNEL(self));
+    RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
+
+    red_channel_init_stat_node(RED_CHANNEL(self), parent, "display_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);
+}
+
 void display_channel_process_surface_cmd(DisplayChannel *display,
                                          const RedSurfaceCmd *surface_cmd,
                                          int loadvm)
diff --git a/server/display-channel.h b/server/display-channel.h
index 4414de1..a6a5944 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -43,6 +43,7 @@
 #include "dcc.h"
 #include "image-encoders.h"
 #include "common-graphics-channel.h"
+#include "stat.h"
 
 G_BEGIN_DECLS
 
@@ -382,6 +383,8 @@ static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
     }
 }
 
+void display_channel_init_stat_node(DisplayChannel *self, RedStatNode *parent);
+
 G_END_DECLS
 
 #endif /* DISPLAY_CHANNEL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index 4876143..cf9e45e 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1370,7 +1370,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
                                                   reds_get_video_codecs(reds),
                                                   init_info.n_surfaces);
     channel = RED_CHANNEL(worker->display_channel);
-    red_channel_init_stat_node(channel, &worker->stat, "display_channel");
+    display_channel_init_stat_node(worker->display_channel, &worker->stat);
     red_channel_register_client_cbs(channel, client_display_cbs, dispatcher);
     g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
     reds_register_channel(reds, channel);

Attachment: signature.asc
Description: PGP signature

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