stats: refactor vlib counters 43/35643/6
authorDamjan Marion <damarion@cisco.com>
Mon, 14 Mar 2022 12:04:38 +0000 (13:04 +0100)
committerFlorin Coras <florin.coras@gmail.com>
Mon, 14 Mar 2022 20:30:06 +0000 (20:30 +0000)
Change-Id: I09d2da73eff42c52ba1373acc99ff28f283a6725
Type: improvement
Signed-off-by: Damjan Marion <damarion@cisco.com>
src/vlib/counter.c
src/vlib/counter.h
src/vlib/stats/init.c
src/vlib/stats/provider_mem.c
src/vlib/stats/stats.c
src/vlib/stats/stats.h

index ceaf013..4f375de 100644 (file)
@@ -79,63 +79,59 @@ void
 vlib_validate_simple_counter (vlib_simple_counter_main_t * cm, u32 index)
 {
   vlib_thread_main_t *tm = vlib_get_thread_main ();
-  int i, resized = 0;
-  void *oldheap = vlib_stats_set_heap ();
+  char *name = cm->stat_segment_name ? cm->stat_segment_name : cm->name;
 
-  vec_validate (cm->counters, tm->n_vlib_mains - 1);
-  for (i = 0; i < tm->n_vlib_mains; i++)
-    if (index >= vec_len (cm->counters[i]))
-      {
-       if (vec_resize_will_expand (cm->counters[i],
-                                   index - vec_len (cm->counters[i]) +
-                                     1 /* length_increment */))
-         resized++;
+  if (name == 0)
+    {
+      if (cm->counters == 0)
+       cm->stats_entry_index = ~0;
+      vec_validate (cm->counters, tm->n_vlib_mains - 1);
+      for (int i = 0; i < tm->n_vlib_mains; i++)
        vec_validate_aligned (cm->counters[i], index, CLIB_CACHE_LINE_BYTES);
-      }
+      return;
+    }
 
-  clib_mem_set_heap (oldheap);
-  /* Avoid the epoch increase when there was no counter vector resize. */
-  if (resized)
-    vlib_stats_update_counter (cm, index, STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE);
+  if (cm->counters == 0)
+    cm->stats_entry_index = vlib_stats_add_counter_vector ("%s", name);
+
+  vlib_stats_validate (cm->stats_entry_index, tm->n_vlib_mains - 1, index);
+  cm->counters = vlib_stats_get_entry_data_pointer (cm->stats_entry_index);
 }
 
 void
 vlib_free_simple_counter (vlib_simple_counter_main_t * cm)
 {
-  int i;
-
-  vlib_stats_delete_cm (cm);
-
-  void *oldheap = vlib_stats_set_heap ();
-  for (i = 0; i < vec_len (cm->counters); i++)
-    vec_free (cm->counters[i]);
-  vec_free (cm->counters);
-  clib_mem_set_heap (oldheap);
+  if (cm->stats_entry_index == ~0)
+    {
+      for (int i = 0; i < vec_len (cm->counters); i++)
+       vec_free (cm->counters[i]);
+      vec_free (cm->counters);
+    }
+  else
+    vlib_stats_remove_entry (cm->stats_entry_index);
 }
 
 void
 vlib_validate_combined_counter (vlib_combined_counter_main_t * cm, u32 index)
 {
   vlib_thread_main_t *tm = vlib_get_thread_main ();
-  int i, resized = 0;
-  void *oldheap = vlib_stats_set_heap ();
+  char *name = cm->stat_segment_name ? cm->stat_segment_name : cm->name;
 
-  vec_validate (cm->counters, tm->n_vlib_mains - 1);
-  for (i = 0; i < tm->n_vlib_mains; i++)
-    if (index >= vec_len (cm->counters[i]))
-      {
-       if (vec_resize_will_expand (cm->counters[i],
-                                   index - vec_len (cm->counters[i]) +
-                                     1 /* length_increment */))
-         resized++;
+  if (name == 0)
+    {
+      if (cm->counters == 0)
+       cm->stats_entry_index = ~0;
+      vec_validate (cm->counters, tm->n_vlib_mains - 1);
+      for (int i = 0; i < tm->n_vlib_mains; i++)
        vec_validate_aligned (cm->counters[i], index, CLIB_CACHE_LINE_BYTES);
-      }
+      return;
+    }
 
-  clib_mem_set_heap (oldheap);
-  /* Avoid the epoch increase when there was no counter vector resize. */
-  if (resized)
-    vlib_stats_update_counter (cm, index,
-                              STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED);
+  if (cm->counters == 0)
+    cm->stats_entry_index = vlib_stats_add_counter_pair_vector ("%s", name);
+
+  vlib_stats_validate (cm->stats_entry_index, tm->n_vlib_mains - 1, index);
+  cm->counters = vlib_stats_get_entry_data_pointer (cm->stats_entry_index);
 }
 
 int
@@ -173,15 +169,14 @@ int
 void
 vlib_free_combined_counter (vlib_combined_counter_main_t * cm)
 {
-  int i;
-
-  vlib_stats_delete_cm (cm);
-
-  void *oldheap = vlib_stats_set_heap ();
-  for (i = 0; i < vec_len (cm->counters); i++)
-    vec_free (cm->counters[i]);
-  vec_free (cm->counters);
-  clib_mem_set_heap (oldheap);
+  if (cm->stats_entry_index == ~0)
+    {
+      for (int i = 0; i < vec_len (cm->counters); i++)
+       vec_free (cm->counters[i]);
+      vec_free (cm->counters);
+    }
+  else
+    vlib_stats_remove_entry (cm->stats_entry_index);
 }
 
 u32
index 56701e8..f9da576 100644 (file)
@@ -59,6 +59,7 @@ typedef struct
   counter_t **counters;         /**< Per-thread u64 non-atomic counters */
   char *name;                  /**< The counter collection's name. */
   char *stat_segment_name;    /**< Name in stat segment directory */
+  u32 stats_entry_index;
 } vlib_simple_counter_main_t;
 
 /** The number of counters (not the number of per-thread counters) */
@@ -219,6 +220,7 @@ typedef struct
   vlib_counter_t **counters;   /**< Per-thread u64 non-atomic counter pairs */
   char *name; /**< The counter collection's name. */
   char *stat_segment_name;     /**< Name in stat segment directory */
+  u32 stats_entry_index;
 } vlib_combined_counter_main_t;
 
 /** The number of counters (not the number of per-thread counters) */
index d24c158..3dba102 100644 (file)
@@ -17,7 +17,7 @@ vector_rate_collector_fn (vlib_stats_collector_data_t *d)
   f64 vector_rate = 0.0;
   u32 i, n_threads = vlib_get_n_threads ();
 
-  vlib_stats_validate_counter_vector (d->entry_index, n_threads - 1);
+  vlib_stats_validate (d->entry_index, 0, n_threads - 1);
 
   for (i = 0; i < n_threads; i++)
     {
@@ -118,7 +118,7 @@ vlib_stats_init (vlib_main_t *vm)
   reg.entry_index =
     vlib_stats_add_counter_vector ("/sys/vector_rate_per_worker");
   vlib_stats_register_collector_fn (&reg);
-  vlib_stats_validate_counter_vector (reg.entry_index, vlib_get_n_threads ());
+  vlib_stats_validate (reg.entry_index, 0, vlib_get_n_threads ());
 
   return 0;
 }
index d52ea4a..1f6d646 100644 (file)
@@ -55,7 +55,7 @@ vlib_stats_register_mem_heap (clib_mem_heap_t *heap)
   vec_add1 (memory_heaps_vec, heap);
 
   r.entry_index = idx = vlib_stats_add_counter_vector ("/mem/%s", heap->name);
-  vlib_stats_validate_counter_vector (idx, STAT_MEM_RELEASABLE);
+  vlib_stats_validate (idx, 0, STAT_MEM_RELEASABLE);
 
   /* Create symlink */
   vlib_stats_add_symlink (idx, STAT_MEM_TOTAL, "/mem/%s/used", heap->name);
index 97f8400..75c3986 100644 (file)
@@ -135,6 +135,7 @@ vlib_stats_remove_entry (u32 entry_index)
   vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
   void *oldheap;
   counter_t **c;
+  vlib_counter_t **vc;
   u32 i;
 
   if (entry_index >= vec_len (sm->directory_vector))
@@ -142,6 +143,8 @@ vlib_stats_remove_entry (u32 entry_index)
 
   oldheap = clib_mem_set_heap (sm->heap);
 
+  vlib_stats_segment_lock ();
+
   switch (e->type)
     {
     case STAT_DIR_TYPE_NAME_VECTOR:
@@ -158,6 +161,14 @@ vlib_stats_remove_entry (u32 entry_index)
       vec_free (c);
       break;
 
+    case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
+      vc = e->data;
+      e->data = 0;
+      for (i = 0; i < vec_len (vc); i++)
+       vec_free (vc[i]);
+      vec_free (vc);
+      break;
+
     case STAT_DIR_TYPE_SCALAR_INDEX:
     case STAT_DIR_TYPE_SYMLINK:
       break;
@@ -165,6 +176,8 @@ vlib_stats_remove_entry (u32 entry_index)
       ASSERT (0);
     }
 
+  vlib_stats_segment_unlock ();
+
   clib_mem_set_heap (oldheap);
   hash_unset_str_key_free (&sm->directory_vector_by_name, e->name);
 
@@ -187,44 +200,47 @@ vlib_stats_set_entry_name (vlib_stats_entry_t *e, char *s)
   s[i] = 0;
 }
 
-void
-vlib_stats_update_counter (void *cm_arg, u32 cindex,
-                          stat_directory_type_t type)
+static u32
+vlib_stats_new_entry_internal (stat_directory_type_t t, u8 *name)
 {
-  vlib_simple_counter_main_t *cm = (vlib_simple_counter_main_t *) cm_arg;
   vlib_stats_segment_t *sm = vlib_stats_get_segment ();
   vlib_stats_shared_header_t *shared_header = sm->shared_header;
-  char *stat_segment_name;
-  vlib_stats_entry_t e = { 0 };
-
-  /* Not all counters have names / hash-table entries */
-  if (!cm->name && !cm->stat_segment_name)
-    return;
+  vlib_stats_entry_t e = { .type = t };
 
   ASSERT (shared_header);
 
-  vlib_stats_segment_lock ();
-
-  /* Lookup hash-table is on the main heap */
-  stat_segment_name = cm->stat_segment_name ? cm->stat_segment_name : cm->name;
-
-  u32 vector_index = vlib_stats_find_entry_index ("%s", stat_segment_name);
-
-  /* Update the vector */
-  if (vector_index == STAT_SEGMENT_INDEX_INVALID)
+  u32 vector_index = vlib_stats_find_entry_index ("%v", name);
+  if (vector_index != STAT_SEGMENT_INDEX_INVALID) /* Already registered */
     {
-      vlib_stats_set_entry_name (&e, stat_segment_name);
-      e.type = type;
-      vector_index = vlib_stats_create_counter (&e);
+      vector_index = ~0;
+      goto done;
     }
 
-  vlib_stats_entry_t *ep = &sm->directory_vector[vector_index];
-  ep->data = cm->counters;
+  vec_add1 (name, 0);
+  vlib_stats_set_entry_name (&e, (char *) name);
+
+  vlib_stats_segment_lock ();
+  vector_index = vlib_stats_create_counter (&e);
 
-  /* Reset the client hash table pointer, since it WILL change! */
   shared_header->directory_vector = sm->directory_vector;
 
   vlib_stats_segment_unlock ();
+
+done:
+  vec_free (name);
+  return vector_index;
+}
+
+u32
+vlib_stats_add_gauge (char *fmt, ...)
+{
+  va_list va;
+  u8 *name;
+
+  va_start (va, fmt);
+  name = va_format (0, fmt, &va);
+  va_end (va);
+  return vlib_stats_new_entry_internal (STAT_DIR_TYPE_SCALAR_INDEX, name);
 }
 
 void
@@ -285,77 +301,6 @@ vlib_stats_update_error_vector (u64 *error_vector, u32 thread_index, int lock)
   clib_mem_set_heap (oldheap);
 }
 
-void
-vlib_stats_delete_cm (void *cm_arg)
-{
-  vlib_simple_counter_main_t *cm = (vlib_simple_counter_main_t *) cm_arg;
-  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
-  vlib_stats_entry_t *e;
-
-  /* Not all counters have names / hash-table entries */
-  if (!cm->name && !cm->stat_segment_name)
-    return;
-
-  vlib_stats_segment_lock ();
-
-  /* Lookup hash-table is on the main heap */
-  char *stat_segment_name =
-    cm->stat_segment_name ? cm->stat_segment_name : cm->name;
-  u32 index = vlib_stats_find_entry_index ("%s", stat_segment_name);
-
-  e = &sm->directory_vector[index];
-
-  hash_unset_str_key_free (&sm->directory_vector_by_name, e->name);
-
-  memset (e, 0, sizeof (*e));
-  e->type = STAT_DIR_TYPE_EMPTY;
-
-  vlib_stats_segment_unlock ();
-}
-
-static u32
-vlib_stats_new_entry_internal (stat_directory_type_t t, u8 *name)
-{
-  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
-  vlib_stats_shared_header_t *shared_header = sm->shared_header;
-  vlib_stats_entry_t e = { .type = t };
-
-  ASSERT (shared_header);
-
-  u32 vector_index = vlib_stats_find_entry_index ("%v", name);
-  if (vector_index != STAT_SEGMENT_INDEX_INVALID) /* Already registered */
-    {
-      vector_index = ~0;
-      goto done;
-    }
-
-  vec_add1 (name, 0);
-  vlib_stats_set_entry_name (&e, (char *) name);
-
-  vlib_stats_segment_lock ();
-  vector_index = vlib_stats_create_counter (&e);
-
-  shared_header->directory_vector = sm->directory_vector;
-
-  vlib_stats_segment_unlock ();
-
-done:
-  vec_free (name);
-  return vector_index;
-}
-
-u32
-vlib_stats_add_gauge (char *fmt, ...)
-{
-  va_list va;
-  u8 *name;
-
-  va_start (va, fmt);
-  name = va_format (0, fmt, &va);
-  va_end (va);
-  return vlib_stats_new_entry_internal (STAT_DIR_TYPE_SCALAR_INDEX, name);
-}
-
 void
 vlib_stats_set_gauge (u32 index, u64 value)
 {
@@ -435,26 +380,130 @@ vlib_stats_add_counter_vector (char *fmt, ...)
                                        name);
 }
 
+u32
+vlib_stats_add_counter_pair_vector (char *fmt, ...)
+{
+  va_list va;
+  u8 *name;
+
+  va_start (va, fmt);
+  name = va_format (0, fmt, &va);
+  va_end (va);
+  return vlib_stats_new_entry_internal (STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED,
+                                       name);
+}
+
+static int
+vlib_stats_validate_will_expand_internal (u32 entry_index, va_list *va)
+{
+  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
+  vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
+  void *oldheap;
+  int rv = 1;
+
+  oldheap = clib_mem_set_heap (sm->heap);
+  if (e->type == STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE)
+    {
+      u32 idx0 = va_arg (*va, u32);
+      u32 idx1 = va_arg (*va, u32);
+      u64 **data = e->data;
+
+      if (idx0 >= vec_max_len (data))
+       goto done;
+
+      for (u32 i = 0; i <= idx0; i++)
+       if (idx1 >= vec_max_len (data[idx0]))
+         goto done;
+    }
+  else if (e->type == STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED)
+    {
+      u32 idx0 = va_arg (*va, u32);
+      u32 idx1 = va_arg (*va, u32);
+      vlib_counter_t **data = e->data;
+
+      va_end (*va);
+
+      if (idx0 >= vec_max_len (data))
+       goto done;
+
+      for (u32 i = 0; i <= idx0; i++)
+       if (idx1 >= vec_max_len (data[idx0]))
+         goto done;
+    }
+  else
+    ASSERT (0);
+
+  rv = 0;
+done:
+  clib_mem_set_heap (oldheap);
+  return rv;
+}
+
+int
+vlib_stats_validate_will_expand (u32 entry_index, ...)
+{
+  va_list va;
+  int rv;
+
+  va_start (va, entry_index);
+  rv = vlib_stats_validate_will_expand_internal (entry_index, &va);
+  va_end (va);
+  return rv;
+}
+
 void
-vlib_stats_validate_counter_vector (u32 entry_index, u32 vector_index)
+vlib_stats_validate (u32 entry_index, ...)
 {
   vlib_stats_segment_t *sm = vlib_stats_get_segment ();
   vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
   void *oldheap;
-  counter_t **c = e->data;
+  va_list va;
+  int will_expand;
 
-  if (vec_len (c) > 0 && vec_len (c[0]) > vector_index)
-    return;
+  va_start (va, entry_index);
+  will_expand = vlib_stats_validate_will_expand_internal (entry_index, &va);
+  va_end (va);
+
+  if (will_expand)
+    vlib_stats_segment_lock ();
 
   oldheap = clib_mem_set_heap (sm->heap);
-  vlib_stats_segment_lock ();
 
-  vec_validate_aligned (c, 0, CLIB_CACHE_LINE_BYTES);
-  vec_validate_aligned (c[0], vector_index, CLIB_CACHE_LINE_BYTES);
-  e->data = c;
+  va_start (va, entry_index);
+
+  if (e->type == STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE)
+    {
+      u32 idx0 = va_arg (va, u32);
+      u32 idx1 = va_arg (va, u32);
+      u64 **data = e->data;
+
+      vec_validate_aligned (data, idx0, CLIB_CACHE_LINE_BYTES);
+
+      for (u32 i = 0; i <= idx0; i++)
+       vec_validate_aligned (data[i], idx1, CLIB_CACHE_LINE_BYTES);
+      e->data = data;
+    }
+  else if (e->type == STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED)
+    {
+      u32 idx0 = va_arg (va, u32);
+      u32 idx1 = va_arg (va, u32);
+      vlib_counter_t **data = e->data;
+
+      vec_validate_aligned (data, idx0, CLIB_CACHE_LINE_BYTES);
+
+      for (u32 i = 0; i <= idx0; i++)
+       vec_validate_aligned (data[i], idx1, CLIB_CACHE_LINE_BYTES);
+      e->data = data;
+    }
+  else
+    ASSERT (0);
+
+  va_end (va);
 
-  vlib_stats_segment_unlock ();
   clib_mem_set_heap (oldheap);
+
+  if (will_expand)
+    vlib_stats_segment_unlock ();
 }
 
 u32
index faaa37a..6db9371 100644 (file)
@@ -119,15 +119,21 @@ vlib_stats_get_entry (vlib_stats_segment_t *sm, u32 entry_index)
   return e;
 }
 
+static_always_inline void *
+vlib_stats_get_entry_data_pointer (u32 entry_index)
+{
+  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
+  vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
+  return e->data;
+}
+
 clib_error_t *vlib_stats_init (vlib_main_t *vm);
 void *vlib_stats_set_heap ();
-void vlib_stats_update_counter (void *, u32, stat_directory_type_t);
 void vlib_stats_register_error_index (u64 *em_vec, u64 index, char *fmt, ...);
 void vlib_stats_update_error_vector (u64 *error_vector, u32 thread_index,
                                     int lock);
 void vlib_stats_segment_lock (void);
 void vlib_stats_segment_unlock (void);
-void vlib_stats_delete_cm (void *);
 void vlib_stats_register_mem_heap (clib_mem_heap_t *);
 f64 vlib_stats_get_segment_update_rate (void);
 
@@ -139,9 +145,11 @@ void vlib_stats_set_gauge (u32 entry_index, u64 value);
 u32 vlib_stats_add_timestamp (char *fmt, ...);
 void vlib_stats_set_timestamp (u32 entry_index, f64 value);
 
-/* vector */
+/* counter vector */
 u32 vlib_stats_add_counter_vector (char *fmt, ...);
-void vlib_stats_validate_counter_vector (u32 entry_index, u32 vector_index);
+
+/* counter pair vector */
+u32 vlib_stats_add_counter_pair_vector (char *fmt, ...);
 
 /* string vector */
 u32 vlib_stats_add_string_vector (char *fmt, ...);
@@ -153,6 +161,8 @@ u32 vlib_stats_add_symlink (u32 entry_index, u32 vector_index, char *fmt, ...);
 void vlib_stats_rename_symlink (u64 entry_index, char *fmt, ...);
 
 /* common to all types */
+void vlib_stats_validate (u32 entry_index, ...);
+int vlib_stats_validate_will_expand (u32 entry_index, ...);
 void vlib_stats_remove_entry (u32 entry_index);
 u32 vlib_stats_find_entry_index (char *fmt, ...);
 void vlib_stats_register_collector_fn (vlib_stats_collector_reg_t *r);