Merge pull request #365 from alltilla/update-metrics-perf-improvement

filterx: `update_metrics()` and `flatten()` perf improvements

lib/filterx/CMakeLists.txt

@@ -53,6 +53,7 @@ set(FILTERX_HEADERS
     filterx/object-json.h
     filterx/object-list-interface.h
     filterx/object-message-value.h
+    filterx/object-metrics-labels.h
     filterx/object-null.h
     filterx/object-primitive.h
     filterx/object-string.h
@@ -115,6 +116,7 @@ set(FILTERX_SOURCES
     filterx/object-json.c
     filterx/object-list-interface.c
     filterx/object-message-value.c
+    filterx/object-metrics-labels.c
     filterx/object-null.c
     filterx/object-primitive.c
     filterx/object-string.c

lib/filterx/Makefile.am

@@ -56,6 +56,7 @@ filterxinclude_HEADERS = 			\
 	lib/filterx/object-json.h \
 	lib/filterx/object-list-interface.h \
 	lib/filterx/object-message-value.h \
+	lib/filterx/object-metrics-labels.h \
 	lib/filterx/object-null.h \
 	lib/filterx/object-primitive.h \
 	lib/filterx/object-string.h
@@ -117,6 +118,7 @@ filterx_sources = 				\
 	lib/filterx/object-json.c \
 	lib/filterx/object-list-interface.c \
 	lib/filterx/object-message-value.c \
+	lib/filterx/object-metrics-labels.c \
 	lib/filterx/object-null.c \
 	lib/filterx/object-primitive.c \
 	lib/filterx/object-string.c

lib/filterx/filterx-globals.c

@@ -30,6 +30,7 @@
 #include "filterx/object-message-value.h"
 #include "filterx/object-list-interface.h"
 #include "filterx/object-dict-interface.h"
+#include "filterx/object-metrics-labels.h"
 #include "filterx/func-istype.h"
 #include "filterx/func-len.h"
 #include "filterx/func-vars.h"
@@ -97,6 +98,9 @@ _simple_init(void)
   g_assert(filterx_builtin_simple_function_register("bool", filterx_typecast_boolean));
   g_assert(filterx_builtin_simple_function_register("int", filterx_typecast_integer));
   g_assert(filterx_builtin_simple_function_register("double", filterx_typecast_double));
+  g_assert(filterx_builtin_simple_function_register("metrics_labels", filterx_simple_function_metrics_labels));
+  g_assert(filterx_builtin_simple_function_register("dedup_metrics_labels",
+                                                    filterx_simple_function_dedup_metrics_labels));
   g_assert(filterx_builtin_simple_function_register("len", filterx_simple_function_len));
   g_assert(filterx_builtin_simple_function_register("vars", filterx_simple_function_vars));
   g_assert(filterx_builtin_simple_function_register("lower", filterx_simple_function_lower));
@@ -248,6 +252,8 @@ filterx_global_init(void)
   filterx_type_init(&FILTERX_TYPE_NAME(datetime));
   filterx_type_init(&FILTERX_TYPE_NAME(message_value));
 
+  filterx_type_init(&FILTERX_TYPE_NAME(metrics_labels));
+
   filterx_primitive_global_init();
   filterx_null_global_init();
   filterx_builtin_functions_init();

lib/filterx/filterx-metrics-labels.c

@@ -29,6 +29,7 @@
 #include "expr-literal-generator.h"
 #include "object-string.h"
 #include "object-dict-interface.h"
+#include "object-metrics-labels.h"
 #include "metrics/dyn-metrics-cache.h"
 #include "stats/stats-cluster-single.h"
 #include "scratch-buffers.h"
@@ -203,9 +204,9 @@ filterx_metrics_labels_is_const(FilterXMetricsLabels *self)
 }
 
 static gboolean
-_format_dict_elem_to_cache(FilterXObject *key, FilterXObject *value, gpointer user_data)
+_format_dict_elem_to_store(FilterXObject *key, FilterXObject *value, gpointer user_data)
 {
-  DynMetricsStore *cache = (DynMetricsStore *) user_data;
+  DynMetricsStore *store = (DynMetricsStore *) user_data;
 
   const gchar *name_str = _format_str_obj(key);
   if (!name_str)
@@ -223,78 +224,96 @@ _format_dict_elem_to_cache(FilterXObject *key, FilterXObject *value, gpointer us
       return FALSE;
     }
 
-  StatsClusterLabel *label = dyn_metrics_store_cache_label(cache);
+  StatsClusterLabel *label = dyn_metrics_store_cache_label(store);
   label->name = name_str;
   label->value = value_str;
 
   return TRUE;
 }
 
 static gboolean
-_format_expr_to_cache(FilterXExpr *expr, DynMetricsStore *cache)
+_format_dict_to_store(FilterXObject *obj, DynMetricsStore *store, StatsClusterLabel **labels, gsize *len)
+{
+  FilterXObject *typed_obj = filterx_ref_unwrap_ro(obj);
+  if (!filterx_object_is_type(typed_obj, &FILTERX_TYPE_NAME(dict)))
+    {
+      filterx_eval_push_error_info("failed to format metrics labels, labels must be a dict", NULL,
+                                   g_strdup_printf("got %s instead", typed_obj->type->name), TRUE);
+      return FALSE;
+    }
+
+  if (!filterx_dict_iter(typed_obj, _format_dict_elem_to_store, store))
+    return FALSE;
+
+  dyn_metrics_store_sort_cached_labels(store);
+
+  *labels = dyn_metrics_store_get_cached_labels(store);
+  *len = dyn_metrics_store_get_cached_labels_len(store);
+
+  return TRUE;
+}
+
+static gboolean
+_format_expr(FilterXExpr *expr, DynMetricsStore *store, StatsClusterLabel **labels, gsize *len)
 {
   FilterXObject *obj = filterx_expr_eval_typed(expr);
   if (!obj)
     return FALSE;
 
-  gboolean success = FALSE;
+  gboolean success = TRUE;
 
   FilterXObject *typed_obj = filterx_ref_unwrap_ro(obj);
-  if (!filterx_object_is_type(typed_obj, &FILTERX_TYPE_NAME(dict)))
+  if (filterx_object_is_type(typed_obj, &FILTERX_TYPE_NAME(metrics_labels)))
     {
-      filterx_eval_push_error_info("failed to format metrics labels, labels must be a dict", expr,
-                                   g_strdup_printf("got %s instead", obj->type->name), TRUE);
+      *labels = filterx_object_metrics_labels_get_value_ref(typed_obj, len);
+      g_assert(labels);
       goto exit;
     }
 
-  success = filterx_dict_iter(typed_obj, _format_dict_elem_to_cache, cache);
-  if (!success)
-    goto exit;
-
-  dyn_metrics_store_sort_cached_labels(cache);
+  success = _format_dict_to_store(obj, store, labels, len);
 
 exit:
   filterx_object_unref(obj);
   return success;
 }
 
 static void
-_format_label_to_cache(gpointer data, gpointer user_data)
+_format_label_to_store(gpointer data, gpointer user_data)
 {
   FilterXMetricsLabel *label = (FilterXMetricsLabel *) data;
   gboolean *success = ((gpointer *) user_data)[0];
-  DynMetricsStore *cache = ((gpointer *) user_data)[1];
+  DynMetricsStore *store = ((gpointer *) user_data)[1];
 
   if (!(*success))
     return;
 
-  *success = _label_format(label, dyn_metrics_store_cache_label(cache));
+  *success = _label_format(label, dyn_metrics_store_cache_label(store));
 }
 
+/* NOTE: Names and values of the labels are stored in ScratchBuffers. */
 gboolean
-filterx_metrics_labels_format(FilterXMetricsLabels *self, StatsClusterLabel **labels, gsize *len)
+filterx_metrics_labels_format(FilterXMetricsLabels *self, DynMetricsStore *store,
+                              StatsClusterLabel **labels, gsize *len)
 {
-  DynMetricsStore *cache = dyn_metrics_cache();
-
-  dyn_metrics_store_reset_labels_cache(cache);
+  dyn_metrics_store_reset_labels_cache(store);
 
   gboolean success;
   if (self->expr)
     {
-      success = _format_expr_to_cache(self->expr, cache);
+      success = _format_expr(self->expr, store, labels, len);
     }
   else
     {
       success = TRUE;
-      gpointer user_data[] = { &success, cache };
-      g_ptr_array_foreach(self->literal_labels, _format_label_to_cache, user_data);
+      gpointer user_data[] = { &success, store };
+      g_ptr_array_foreach(self->literal_labels, _format_label_to_store, user_data);
+      *labels = dyn_metrics_store_get_cached_labels(store);
+      *len = dyn_metrics_store_get_cached_labels_len(store);
     }
 
   if (!success)
     return FALSE;
 
-  *labels = dyn_metrics_store_get_cached_labels(cache);
-  *len = dyn_metrics_store_get_cached_labels_len(cache);
   return TRUE;
 }
 

lib/filterx/filterx-metrics-labels.h

@@ -26,6 +26,7 @@
 #define FILTERX_METRICS_LABELS_H_INCLUDED
 
 #include "filterx-expr.h"
+#include "metrics/dyn-metrics-store.h"
 
 typedef struct _FilterXMetricsLabels FilterXMetricsLabels;
 
@@ -34,7 +35,8 @@ gboolean filterx_metrics_labels_init(FilterXMetricsLabels *self, GlobalConfig *c
 void filterx_metrics_labels_deinit(FilterXMetricsLabels *self, GlobalConfig *cfg);
 void filterx_metrics_labels_free(FilterXMetricsLabels *self);
 
-gboolean filterx_metrics_labels_format(FilterXMetricsLabels *self, StatsClusterLabel **labels, gsize *len);
+gboolean filterx_metrics_labels_format(FilterXMetricsLabels *self, DynMetricsStore *store,
+                                       StatsClusterLabel **labels, gsize *len);
 gboolean filterx_metrics_labels_is_const(FilterXMetricsLabels *self);
 
 #endif

lib/filterx/filterx-metrics.c

@@ -82,15 +82,15 @@ _format_sck_name(FilterXMetrics *self)
 }
 
 static gboolean
-_format_sck(FilterXMetrics *self, StatsClusterKey *sck)
+_format_sck(FilterXMetrics *self, StatsClusterKey *sck, DynMetricsStore *store)
 {
   const gchar *name = _format_sck_name(self);
   if (!name)
     return FALSE;
 
   StatsClusterLabel *labels;
   gsize labels_len;
-  if (!filterx_metrics_labels_format(self->labels, &labels, &labels_len))
+  if (!filterx_metrics_labels_format(self->labels, store, &labels, &labels_len))
     return FALSE;
 
   stats_cluster_single_key_set(sck, name, labels, labels_len);
@@ -106,6 +106,8 @@ _is_const(FilterXMetrics *self)
 static void
 _optimize(FilterXMetrics *self)
 {
+  DynMetricsStore *store = dyn_metrics_cache();
+
   stats_lock();
 
   if (!self->key.str || !filterx_metrics_labels_is_const(self->labels))
@@ -115,7 +117,7 @@ _optimize(FilterXMetrics *self)
   scratch_buffers_mark(&marker);
 
   StatsClusterKey sck;
-  if (!_format_sck(self, &sck))
+  if (!_format_sck(self, &sck, store))
     {
       msg_debug("FilterX: Failed to optimize metrics, continuing unoptimized");
       scratch_buffers_reclaim_marked(marker);
@@ -137,6 +139,8 @@ _optimize(FilterXMetrics *self)
 gboolean
 filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **counter)
 {
+  DynMetricsStore *store = dyn_metrics_cache();
+
   if (!filterx_metrics_is_enabled(self))
     {
       *counter = NULL;
@@ -155,10 +159,10 @@ filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **count
   scratch_buffers_mark(&marker);
 
   StatsClusterKey sck;
-  if (!_format_sck(self, &sck))
+  if (!_format_sck(self, &sck, store))
     goto exit;
 
-  *counter = dyn_metrics_store_retrieve_counter(dyn_metrics_cache(), &sck, self->level);
+  *counter = dyn_metrics_store_retrieve_counter(store, &sck, self->level);
   success = TRUE;
 
 exit:

lib/filterx/func-flatten.c

@@ -46,37 +46,34 @@ typedef struct FilterXFunctionFlattenKV_
   FilterXObject *value;
 } FilterXFunctionFlattenKV;
 
-static FilterXFunctionFlattenKV *
-_kv_new(FilterXObject *key, FilterXObject *value)
+static void
+_kv_init(FilterXFunctionFlattenKV *self, FilterXObject *key, FilterXObject *value)
 {
-  FilterXFunctionFlattenKV *self = g_new0(FilterXFunctionFlattenKV, 1);
   self->key = filterx_object_ref(key);
   self->value = filterx_object_ref(value);
-  return self;
 }
 
 static void
-_kv_free(FilterXFunctionFlattenKV *self)
+_kv_destroy(FilterXFunctionFlattenKV *self)
 {
   filterx_object_unref(self->key);
   filterx_object_unref(self->value);
-  g_free(self);
 }
 
 static gboolean
 _collect_modifications_from_elem(FilterXObject *key, FilterXObject *value, gpointer user_data)
 {
   FilterXFunctionFlatten *self = ((gpointer *) user_data)[0];
-  GList **flattened_kvs = ((gpointer *) user_data)[1];
-  GList **top_level_dict_keys = ((gpointer *) user_data)[2];
+  GArray *flattened_kvs = ((gpointer *) user_data)[1];
+  GPtrArray *top_level_dict_keys = ((gpointer *) user_data)[2];
   GString *key_buffer = ((gpointer *) user_data)[3];
   gboolean is_top_level = (gboolean) GPOINTER_TO_INT(((gpointer *) user_data)[4]);
 
   FilterXObject *dict = filterx_ref_unwrap_ro(value);
   if (filterx_object_is_type(dict, &FILTERX_TYPE_NAME(dict)))
     {
       if (is_top_level)
-        *top_level_dict_keys = g_list_prepend(*top_level_dict_keys, filterx_object_ref(key));
+        g_ptr_array_add(top_level_dict_keys, filterx_object_ref(key));
 
       gssize orig_len = key_buffer->len;
       if (!filterx_object_repr_append(key, key_buffer))
@@ -108,7 +105,9 @@ _collect_modifications_from_elem(FilterXObject *key, FilterXObject *value, gpoin
     }
 
   FilterXObject *flat_key = filterx_string_new(key_buffer->str, (gssize) MIN(key_buffer->len, G_MAXSSIZE));
-  *flattened_kvs = g_list_prepend(*flattened_kvs, _kv_new(flat_key, value));
+  FilterXFunctionFlattenKV kv;
+  _kv_init(&kv, flat_key, value);
+  g_array_append_val(flattened_kvs, kv);
   filterx_object_unref(flat_key);
 
   g_string_truncate(key_buffer, orig_len);
@@ -117,20 +116,19 @@ _collect_modifications_from_elem(FilterXObject *key, FilterXObject *value, gpoin
 
 static gboolean
 _collect_dict_modifications(FilterXFunctionFlatten *self, FilterXObject *dict,
-                            GList **flattened_kvs, GList **top_level_dict_keys)
+                            GArray *flattened_kvs, GPtrArray *top_level_dict_keys)
 {
   GString *key_buffer = scratch_buffers_alloc();
   gpointer user_data[] = { self, flattened_kvs, top_level_dict_keys, key_buffer, GINT_TO_POINTER(TRUE)};
   return filterx_dict_iter(dict, _collect_modifications_from_elem, user_data);
 }
 
 static gboolean
-_remove_keys(FilterXFunctionFlatten *self, FilterXObject *dict, GList *keys)
+_remove_keys(FilterXFunctionFlatten *self, FilterXObject *dict, GPtrArray *keys)
 {
-  for (GList *elem = keys; elem; elem = elem->next)
+  for (guint i = 0; i < keys->len; i++)
     {
-      FilterXObject *key = (FilterXObject *) elem->data;
-
+      FilterXObject *key = (FilterXObject *) g_ptr_array_index(keys, i);
       if (!filterx_object_unset_key(dict, key))
         return FALSE;
     }
@@ -139,11 +137,11 @@ _remove_keys(FilterXFunctionFlatten *self, FilterXObject *dict, GList *keys)
 }
 
 static gboolean
-_add_kvs(FilterXFunctionFlatten *self, FilterXObject *dict, GList *kvs)
+_add_kvs(FilterXFunctionFlatten *self, FilterXObject *dict, GArray *kvs)
 {
-  for (GList *elem = kvs; elem; elem = elem->next)
+  for (guint i = 0; i < kvs->len; i++)
     {
-      FilterXFunctionFlattenKV *kv = (FilterXFunctionFlattenKV *) elem->data;
+      FilterXFunctionFlattenKV *kv = &g_array_index(kvs, FilterXFunctionFlattenKV, i);
 
       FilterXObject *value = filterx_object_clone(kv->value);
       gboolean success = filterx_object_set_subscript(dict, kv->key, &value);
@@ -162,10 +160,11 @@ _add_kvs(FilterXFunctionFlatten *self, FilterXObject *dict, GList *kvs)
 static gboolean
 _flatten(FilterXFunctionFlatten *self, FilterXObject *dict)
 {
-  GList *flattened_kvs = NULL, *top_level_dict_keys = NULL;
+  GArray *flattened_kvs = g_array_new(FALSE, FALSE, sizeof(FilterXFunctionFlattenKV));
+  GPtrArray *top_level_dict_keys = g_ptr_array_new_with_free_func((GDestroyNotify) filterx_object_unref);
   gboolean result = FALSE;
 
-  if (!_collect_dict_modifications(self, dict, &flattened_kvs, &top_level_dict_keys))
+  if (!_collect_dict_modifications(self, dict, flattened_kvs, top_level_dict_keys))
     goto exit;
 
   if (!_remove_keys(self, dict, top_level_dict_keys))
@@ -177,8 +176,10 @@ _flatten(FilterXFunctionFlatten *self, FilterXObject *dict)
   result = TRUE;
 
 exit:
-  g_list_free_full(flattened_kvs, (GDestroyNotify) _kv_free);
-  g_list_free_full(top_level_dict_keys, (GDestroyNotify) filterx_object_unref);
+  for (guint i = 0; i < flattened_kvs->len; i++)
+    _kv_destroy(&g_array_index(flattened_kvs, FilterXFunctionFlattenKV, i));
+  g_array_free(flattened_kvs, TRUE);
+  g_ptr_array_free(top_level_dict_keys, TRUE);
   return result;
 }