tracers: code cleanups

Move static variables to instance variables. Add finalize methods. Remove code
that is commented out. Cleanup locking code.

plugins/tracers/gstlatency.c

@@ -113,8 +113,7 @@ log_latency (const GstStructure * data, GstPad * sink_pad, guint64 sink_ts)
 }
 
 static void
-send_latency_probe (GstLatencyTracer * self, GstElement * parent, GstPad * pad,
+send_latency_probe (GstElement * parent, GstPad * pad, guint64 ts)
-    guint64 ts)
 {
   if (parent && (!GST_IS_BIN (parent)) &&
       GST_OBJECT_FLAG_IS_SET (parent, GST_ELEMENT_FLAG_SOURCE)) {
@@ -134,7 +133,7 @@ do_push_buffer_pre (GstTracer * self, va_list var_args)
   GstPad *pad = va_arg (var_args, GstPad *);
   GstElement *parent = get_real_pad_parent (pad);
 
-  send_latency_probe ((GstLatencyTracer *) self, parent, pad, ts);
+  send_latency_probe (parent, pad, ts);
 }
 
 static void
@@ -145,12 +144,11 @@ do_pull_range_pre (GstTracer * self, va_list var_args)
   GstPad *peer_pad = GST_PAD_PEER (pad);
   GstElement *parent = get_real_pad_parent (peer_pad);
 
-  send_latency_probe ((GstLatencyTracer *) self, parent, peer_pad, ts);
+  send_latency_probe (parent, peer_pad, ts);
 }
 
 static void
-calculate_latency (GstLatencyTracer * self, GstElement * parent, GstPad * pad,
+calculate_latency (GstElement * parent, GstPad * pad, guint64 ts)
-    guint64 ts)
 {
   if (parent && (!GST_IS_BIN (parent)) &&
       GST_OBJECT_FLAG_IS_SET (parent, GST_ELEMENT_FLAG_SINK)) {
@@ -169,7 +167,7 @@ do_push_buffer_post (GstTracer * self, va_list var_args)
   GstPad *peer_pad = GST_PAD_PEER (pad);
   GstElement *parent = get_real_pad_parent (peer_pad);
 
-  calculate_latency ((GstLatencyTracer *) self, parent, peer_pad, ts);
+  calculate_latency (parent, peer_pad, ts);
 }
 
 static void
@@ -179,7 +177,7 @@ do_pull_range_post (GstTracer * self, va_list var_args)
   GstPad *pad = va_arg (var_args, GstPad *);
   GstElement *parent = get_real_pad_parent (pad);
 
-  calculate_latency ((GstLatencyTracer *) self, parent, pad, ts);
+  calculate_latency (parent, pad, ts);
 }
 
 static void

plugins/tracers/gstrusage.c

@@ -42,33 +42,20 @@
 GST_DEBUG_CATEGORY_STATIC (gst_rusage_debug);
 #define GST_CAT_DEFAULT gst_rusage_debug
 
+G_LOCK_DEFINE (_proc);
+
 #define _do_init \
     GST_DEBUG_CATEGORY_INIT (gst_rusage_debug, "rusage", 0, "rusage tracer");
 #define gst_rusage_tracer_parent_class parent_class
 G_DEFINE_TYPE_WITH_CODE (GstRUsageTracer, gst_rusage_tracer, GST_TYPE_TRACER,
     _do_init);
 
-/* we remember x measurements per self->window */
+/* remember x measurements per self->window */
 #define WINDOW_SUBDIV 100
 
-/* for ts calibration */
+/* number of cpus to scale cpu-usage in threads */
-static gpointer main_thread_id = NULL;
-static guint64 tproc_base = G_GINT64_CONSTANT (0);
 static glong num_cpus = 1;
 
-typedef struct
-{
-  GstClockTime ts;
-  GstClockTime val;
-} GstTraceValue;
-
-typedef struct
-{
-  GstClockTime window;
-  GMutex lock;
-  GQueue values;                /* GstTraceValue* */
-} GstTraceValues;
-
 typedef struct
 {
   /* time spend in this thread */
@@ -76,8 +63,6 @@ typedef struct
   GstTraceValues *tvs_thread;
 } GstThreadStats;
 
-static GstTraceValues *tvs_proc;
-
 /* data helper */
 
 static void
@@ -91,7 +76,6 @@ make_trace_values (GstClockTime window)
 {
   GstTraceValues *self = g_slice_new0 (GstTraceValues);
   self->window = window;
-  g_mutex_init (&self->lock);
   g_queue_init (&self->values);
   return self;
 }
@@ -100,7 +84,6 @@ static void
 free_trace_values (GstTraceValues * self)
 {
   g_queue_free_full (&self->values, free_trace_value);
-  g_mutex_clear (&self->lock);
   g_slice_free (GstTraceValues, self);
 }
 
@@ -221,21 +204,21 @@ do_stats (GstTracer * obj, va_list var_args)
    * the time is larger than ts, as our base-ts is taken after the process has
    * started.
    */
-  if (G_UNLIKELY (thread_id == main_thread_id)) {
+  if (G_UNLIKELY (thread_id == self->main_thread_id)) {
-    main_thread_id = NULL;
+    self->main_thread_id = NULL;
     /* when the registry gets updated, the tproc is less than the debug time ? */
     /* TODO(ensonic): we still see cases where tproc overtakes ts, especially
      * when with sync=false, can this be due to multiple cores in use? */
     if (tproc > ts) {
-      tproc_base = tproc - ts;
+      self->tproc_base = tproc - ts;
       GST_DEBUG ("rusage: calibrating by %" G_GUINT64_FORMAT ", thread: %"
           G_GUINT64_FORMAT ", proc: %" G_GUINT64_FORMAT,
-          tproc_base, stats->tthread, tproc);
+          self->tproc_base, stats->tthread, tproc);
-      stats->tthread -= tproc_base;
+      stats->tthread -= self->tproc_base;
     }
   }
   /* we always need to correct proc time */
-  tproc -= tproc_base;
+  tproc -= self->tproc_base;
 
   /* FIXME: how can we take cpu-frequency scaling into account?
    * - looking at /sys/devices/system/cpu/cpu0/cpufreq/
@@ -260,9 +243,9 @@ do_stats (GstTracer * obj, va_list var_args)
 
   avg_cpuload = (guint) gst_util_uint64_scale (tproc / num_cpus,
       G_GINT64_CONSTANT (1000), ts);
-  g_mutex_lock (&tvs_proc->lock);
+  G_LOCK (_proc);
-  update_trace_value (tvs_proc, ts, tproc, &dts, &dtproc);
+  update_trace_value (self->tvs_proc, ts, tproc, &dts, &dtproc);
-  g_mutex_unlock (&tvs_proc->lock);
+  G_UNLOCK (_proc);
   cur_cpuload = (guint) gst_util_uint64_scale (dtproc / num_cpus,
       G_GINT64_CONSTANT (1000), dts);
   gst_tracer_log_trace (gst_structure_new ("proc-rusage", 
@@ -276,9 +259,33 @@ do_stats (GstTracer * obj, va_list var_args)
 
 /* tracer class */
 
+static void
+gst_rusage_tracer_finalize (GObject * obj)
+{
+  GstRUsageTracer *self = GST_RUSAGE_TRACER (obj);
+
+  g_hash_table_destroy (self->threads);
+  free_trace_values (self->tvs_proc);
+
+  G_OBJECT_CLASS (parent_class)->finalize (obj);
+}
+
 static void
 gst_rusage_tracer_class_init (GstRUsageTracerClass * klass)
 {
+  GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
+
+  gobject_class->finalize = gst_rusage_tracer_finalize;
+
+  if ((num_cpus = sysconf (_SC_NPROCESSORS_ONLN)) == -1) {
+    GST_WARNING ("failed to get number of cpus online");
+    if ((num_cpus = sysconf (_SC_NPROCESSORS_CONF)) == -1) {
+      GST_WARNING ("failed to get number of cpus, assuming 1");
+      num_cpus = 1;
+    }
+  }
+  GST_DEBUG ("rusage: num_cpus=%ld", num_cpus);
+
   /* announce trace formats */
   /* *INDENT-OFF* */
   gst_tracer_log_trace (gst_structure_new ("thread-rusage.class",
@@ -344,15 +351,8 @@ gst_rusage_tracer_init (GstRUsageTracer * self)
   gst_tracer_register_hook_id (tracer, 0, do_stats);
 
   self->threads = g_hash_table_new_full (NULL, NULL, NULL, free_thread_stats);
+  self->tvs_proc = make_trace_values (GST_SECOND);
+  self->main_thread_id = g_thread_self ();
 
-  main_thread_id = g_thread_self ();
+  GST_DEBUG ("rusage: main thread=%p", self->main_thread_id);
-  tvs_proc = make_trace_values (GST_SECOND);
-  if ((num_cpus = sysconf (_SC_NPROCESSORS_ONLN)) == -1) {
-    GST_WARNING_OBJECT (self, "failed to get number of cpus online");
-    if ((num_cpus = sysconf (_SC_NPROCESSORS_CONF)) == -1) {
-      GST_WARNING_OBJECT (self, "failed to get number of cpus, assuming 1");
-      num_cpus = 1;
-    }
-  }
-  GST_DEBUG ("rusage: main thread=%p, num_cpus=%ld", main_thread_id, num_cpus);
 }

plugins/tracers/gstrusage.h

@@ -42,6 +42,18 @@ G_BEGIN_DECLS
 typedef struct _GstRUsageTracer GstRUsageTracer;
 typedef struct _GstRUsageTracerClass GstRUsageTracerClass;
 
+typedef struct
+{
+  GstClockTime ts;
+  GstClockTime val;
+} GstTraceValue;
+
+typedef struct
+{
+  GstClockTime window;
+  GQueue values;                /* GstTraceValue* */
+} GstTraceValues;
+
 /**
  * GstRUsageTracer:
  *
@@ -52,6 +64,11 @@ struct _GstRUsageTracer {
 
   /*< private >*/        
   GHashTable *threads;
+  GstTraceValues *tvs_proc;
+
+  /* for ts calibration */
+  gpointer main_thread_id;
+  guint64 tproc_base;
 };
 
 struct _GstRUsageTracerClass {

plugins/tracers/gststats.c

@@ -37,7 +37,8 @@ GST_DEBUG_CATEGORY_STATIC (gst_stats_debug);
 #define GST_CAT_DEFAULT gst_stats_debug
 
 static GQuark data_quark;
-G_LOCK_DEFINE (_stats);
+G_LOCK_DEFINE (_elem_stats);
+G_LOCK_DEFINE (_pad_stats);
 
 #define _do_init \
     GST_DEBUG_CATEGORY_INIT (gst_stats_debug, "stats", 0, "stats tracer"); \
@@ -93,6 +94,12 @@ log_new_element_stats (GstElementStats * stats, GstElement * element)
           "is-bin", G_TYPE_BOOLEAN, GST_IS_BIN (element), NULL));
 }
 
+static void
+free_element_stats (gpointer data)
+{
+  g_slice_free (GstElementStats, data);
+}
+
 static inline GstElementStats *
 get_element_stats (GstStatsTracer * self, GstElement * element)
 {
@@ -104,16 +111,14 @@ get_element_stats (GstStatsTracer * self, GstElement * element)
     return &no_elem_stats;
   }
 
-  G_LOCK (_stats);
+  G_LOCK (_elem_stats);
   if (!(stats = g_object_get_qdata ((GObject *) element, data_quark))) {
     stats = fill_element_stats (self, element);
-    g_object_set_qdata ((GObject *) element, data_quark, stats);
+    g_object_set_qdata_full ((GObject *) element, data_quark, stats,
-    if (self->elements->len <= stats->index)
+        free_element_stats);
-      g_ptr_array_set_size (self->elements, stats->index + 1);
-    g_ptr_array_index (self->elements, stats->index) = stats;
     is_new = TRUE;
   }
-  G_UNLOCK (_stats);
+  G_UNLOCK (_elem_stats);
   if (G_UNLIKELY (stats->parent_ix == G_MAXUINT)) {
     GstElement *parent = GST_ELEMENT_PARENT (element);
     if (parent) {
@@ -127,12 +132,6 @@ get_element_stats (GstStatsTracer * self, GstElement * element)
   return stats;
 }
 
-static void
-free_element_stats (gpointer data)
-{
-  g_slice_free (GstElementStats, data);
-}
-
 /*
  * Get the element/bin owning the pad. 
  *
@@ -192,6 +191,12 @@ log_new_pad_stats (GstPadStats * stats, GstPad * pad)
           "thread-id", G_TYPE_UINT, GPOINTER_TO_UINT (g_thread_self ()), NULL));
 }
 
+static void
+free_pad_stats (gpointer data)
+{
+  g_slice_free (GstPadStats, data);
+}
+
 static GstPadStats *
 get_pad_stats (GstStatsTracer * self, GstPad * pad)
 {
@@ -203,16 +208,14 @@ get_pad_stats (GstStatsTracer * self, GstPad * pad)
     return &no_pad_stats;
   }
 
-  G_LOCK (_stats);
+  G_LOCK (_pad_stats);
   if (!(stats = g_object_get_qdata ((GObject *) pad, data_quark))) {
     stats = fill_pad_stats (self, pad);
-    g_object_set_qdata ((GObject *) pad, data_quark, stats);
+    g_object_set_qdata_full ((GObject *) pad, data_quark, stats,
-    if (self->pads->len <= stats->index)
+        free_pad_stats);
-      g_ptr_array_set_size (self->pads, stats->index + 1);
-    g_ptr_array_index (self->pads, stats->index) = stats;
     is_new = TRUE;
   }
-  G_UNLOCK (_stats);
+  G_UNLOCK (_pad_stats);
   if (G_UNLIKELY (stats->parent_ix == G_MAXUINT)) {
     GstElement *elem = get_real_pad_parent (pad);
     if (elem) {
@@ -227,12 +230,6 @@ get_pad_stats (GstStatsTracer * self, GstPad * pad)
   return stats;
 }
 
-static void
-free_pad_stats (gpointer data)
-{
-  g_slice_free (GstPadStats, data);
-}
-
 static void
 do_buffer_stats (GstStatsTracer * self, GstPad * this_pad,
     GstPadStats * this_pad_stats, GstPad * that_pad,
@@ -453,15 +450,6 @@ do_push_event_pre (GstStatsTracer * self, va_list var_args)
           "name", G_TYPE_STRING, GST_EVENT_TYPE_NAME (ev), NULL));
 }
 
-static void
-do_push_event_post (GstStatsTracer * self, va_list var_args)
-{
-#if 0
-  guint64 ts = va_arg (var_args, guint64);
-  GstPad *pad = va_arg (var_args, GstPad *);
-#endif
-}
-
 static void
 do_post_message_pre (GstStatsTracer * self, va_list var_args)
 {
@@ -477,15 +465,6 @@ do_post_message_pre (GstStatsTracer * self, va_list var_args)
           "name", G_TYPE_STRING, GST_MESSAGE_TYPE_NAME (msg), NULL));
 }
 
-static void
-do_post_message_post (GstStatsTracer * self, va_list var_args)
-{
-#if 0
-  guint64 ts = va_arg (var_args, guint64);
-  GstElement *elem = va_arg (var_args, GstElement *);
-#endif
-}
-
 static void
 do_query_pre (GstStatsTracer * self, va_list var_args)
 {
@@ -501,15 +480,6 @@ do_query_pre (GstStatsTracer * self, va_list var_args)
           "name", G_TYPE_STRING, GST_QUERY_TYPE_NAME (qry), NULL));
 }
 
-static void
-do_query_post (GstStatsTracer * self, va_list var_args)
-{
-#if 0
-  guint64 ts = va_arg (var_args, guint64);
-  GstElement *elem = va_arg (var_args, GstElement *);
-#endif
-}
-
 /* tracer class */
 
 static void
@@ -608,17 +578,8 @@ gst_stats_tracer_init (GstStatsTracer * self)
       (GstTracerHookFunction) do_pull_range_post);
   gst_tracer_register_hook (tracer, "pad-push-event-pre",
       (GstTracerHookFunction) do_push_event_pre);
-  gst_tracer_register_hook (tracer, "pad-push-event-post",
-      (GstTracerHookFunction) do_push_event_post);
   gst_tracer_register_hook (tracer, "element-post-message-pre",
       (GstTracerHookFunction) do_post_message_pre);
-  gst_tracer_register_hook (tracer, "element-post-message-post",
-      (GstTracerHookFunction) do_post_message_post);
   gst_tracer_register_hook (tracer, "element-query-pre",
       (GstTracerHookFunction) do_query_pre);
-  gst_tracer_register_hook (tracer, "element-query-post",
-      (GstTracerHookFunction) do_query_post);
-
-  self->elements = g_ptr_array_new_with_free_func (free_element_stats);
-  self->pads = g_ptr_array_new_with_free_func (free_pad_stats);
 }

plugins/tracers/gststats.h

@@ -51,8 +51,6 @@ struct _GstStatsTracer {
   GstTracer 	 parent;
 
   /*< private >*/
-  GPtrArray *elements;
-  GPtrArray *pads;
   guint num_elements, num_pads;
 };