[counters] First pass of fixes from Zoltan's code review.

mono · Mar 20, 2014 · 0657f61 · 0657f61
1 parent 9edc751
commit 0657f61
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 8 deletions.
diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c
@@ -2529,7 +2529,7 @@ void mono_thread_init (MonoThreadStartCB start_cb,
 	 */
 	GetCurrentProcess ();
 
-	thread_count = mono_counters_new_int (MONO_COUNTER_CAT_THREAD,"Thread Count", MONO_COUNTER_UNIT_NONE, MONO_COUNTER_VARIABLE);
+	thread_count = mono_counters_new_int (MONO_COUNTER_CAT_THREAD, "Thread Count", MONO_COUNTER_UNIT_NONE, MONO_COUNTER_VARIABLE);
 }
 
 void mono_thread_cleanup (void)

diff --git a/mono/utils/mono-counters-agent.c b/mono/utils/mono-counters-agent.c
@@ -70,6 +70,9 @@ free_agent_memory (void)
 		counter = counter->next;
 	}
 	g_slist_free (counters);
+	g_free ((char*)inspector_ip);
+	counters = NULL;
+	inspector_ip = NULL;
 }
 
 static void
@@ -123,7 +126,7 @@ parse_counters_names (const char *counters_names)
 static void
 parse_address (const char *address)
 {
-	inspector_ip = DEFAULT_ADDRESS;
+	inspector_ip = NULL;
 	inspector_port = DEFAULT_PORT;
 
 	if (address) {
@@ -136,6 +139,8 @@ parse_address (const char *address)
 
 		g_strfreev(split);
 	}
+	if (!inspector_ip)
+		inspector_ip = g_strdup (DEFAULT_ADDRESS);
 }
 
 static gboolean

diff --git a/mono/utils/mono-counters-internals.h b/mono/utils/mono-counters-internals.h
@@ -40,7 +40,7 @@ typedef enum {
 
 typedef enum {
 	MONO_COUNTER_CONSTANT = 1, /* This counter doesn't change. Agent will only send it once */
-	MONO_COUNTER_MONOTONIC, /* This counter value always increase/decreate over time */
+	MONO_COUNTER_MONOTONIC, /* This counter value always increase/decreases over time */
 	MONO_COUNTER_VARIABLE, /* This counter value can be anything on each sampling */
 
 	MONO_COUNTER_VARIANCE_MAX
@@ -96,7 +96,7 @@ typedef struct { TYPE *pointer; } CAPITALIZED_NAME ##Counter;	\
 static void mono_counters_ ## NAME ## _set (CAPITALIZED_NAME ##Counter counter, TYPE value) { (*counter.pointer) += value; }	\
 static TYPE mono_counters_ ## NAME ## _get (CAPITALIZED_NAME ##Counter counter) { return *counter.pointer; }	\
 static void mono_counters_ ## NAME ## _inc (CAPITALIZED_NAME ##Counter counter) { (*counter.pointer) += 1; }	\
-static void mono_counters_ ## NAME ## _dec (CAPITALIZED_NAME ##Counter counter) { (*counter.pointer) += 1; }	\
+static void mono_counters_ ## NAME ## _dec (CAPITALIZED_NAME ##Counter counter) { (*counter.pointer) -= 1; }	\
 static CAPITALIZED_NAME ##Counter mono_counters_new_ ## NAME (MonoCounterCategory category, const char *name, MonoCounterUnit unit, MonoCounterVariance variance) {	\
 	CAPITALIZED_NAME ##Counter c = { mono_counters_new(category, name, MONO_COUNTER_TYPE_ ## COUNTER_TYPE, unit, variance) };	\
 	return c;	\

diff --git a/mono/utils/mono-counters.c b/mono/utils/mono-counters.c
@@ -2,12 +2,15 @@
  * Copyright 2006-2010 Novell
  * Copyright 2011 Xamarin Inc
  */
-
+#include "config.h"
 #include <stdlib.h>
 #include <glib.h>
 #include "mono-counters-internals.h"
 #include "mono-proclib.h"
+
+#ifdef HAVE_UNISTD_H
 #include <unistd.h>
+#endif
 
 static MonoCounter *counters = NULL;
 static int valid_mask = 0;
@@ -65,13 +68,29 @@ mono_counters_register_full (MonoCounterCategory category, const char *name, Mon
 	return counter;
 }
 
+/*
+This function is a placeholder, it should eventually be replaced by code that allocs in the shm perfcounter arena.
+*/
 static void*
 mono_counters_alloc_space (int size)
 {
 	//FIXME actually alloc memory from perf-counters
 	return g_malloc0 (size);
 }
 
+/**
+ * mono_counters_new:
+ * @category: The category of this counter
+ * @name: The name of this counter
+ * @type: The type size of the counter
+ * @variance: The expected semantics of the values sampled.
+ *
+ * Register a new counter within the runtime.
+ *
+ * Avoid this function if possible and use the specialized helpers.
+ *
+ * Returns: The address of the counter.
+ */
 void*
 mono_counters_new (MonoCounterCategory category, const char *name, MonoCounterType type, MonoCounterUnit unit, MonoCounterVariance variance)
 {
@@ -89,7 +108,6 @@ mono_counters_new (MonoCounterCategory category, const char *name, MonoCounterTy
 	return addr;
 }
 
-
 static int
 section_to_category (int type)
 {
@@ -160,15 +178,13 @@ mono_counters_register (const char* name, int type, void *addr)
 		counter->callback_style = CB_NO_ARG;
 }
 
-
 typedef int (*IntFunc) (void);
 typedef gint64 (*LongFunc) (void);
 typedef double (*DoubleFunc) (void);
 typedef int (*IntFunc2) (void*);
 typedef gint64 (*LongFunc2) (void*);
 typedef double (*DoubleFunc2) (void*);
 
-
 #define ENTRY_FMT "%-36s: "
 static void
 dump_counter (MonoCounter *counter, FILE *outfile) {
@@ -322,6 +338,7 @@ static gint64
 sample_cpu (void *arg)
 {
 	int kind = GPOINTER_TO_INT (arg);
+	//FIXME replace getpid with something portable.
 	return mono_process_get_data (GINT_TO_POINTER (getpid ()), kind);
 }