Merge pull request #436 from alltilla/filterx-optimize-followup

filterx: optimize followup

lib/filterx/expr-condition.c

@@ -164,7 +164,7 @@ _optimize(FilterXExpr *s)
   _optimize_branches(s);
 
   if (!filterx_expr_is_literal(self->condition))
-    return FALSE;
+    return NULL;
 
   FilterXObject *condition_value = filterx_expr_eval(self->condition);
 

lib/filterx/expr-function.c

@@ -278,7 +278,28 @@ filterx_function_free_method(FilterXFunction *s)
   filterx_expr_free_method(&s->super);
 }
 
-static void
+static inline FilterXExpr *
+_function_optimize(FilterXExpr *s)
+{
+  FilterXFunction *self = (FilterXFunction *) s;
+  return filterx_function_optimize_method(self);
+}
+
+static inline gboolean
+_function_init(FilterXExpr *s, GlobalConfig *cfg)
+{
+  FilterXFunction *self = (FilterXFunction *) s;
+  return filterx_function_init_method(self, cfg);
+}
+
+static inline void
+_function_deinit(FilterXExpr *s, GlobalConfig *cfg)
+{
+  FilterXFunction *self = (FilterXFunction *) s;
+  return filterx_function_deinit_method(self, cfg);
+}
+
+static inline void
 _function_free(FilterXExpr *s)
 {
   FilterXFunction *self = (FilterXFunction *) s;
@@ -290,9 +311,18 @@ filterx_function_init_instance(FilterXFunction *s, const gchar *function_name)
 {
   filterx_expr_init_instance(&s->super);
   s->function_name = g_strdup_printf("%s()", function_name);
+  s->super.optimize = _function_optimize;
+  s->super.init = _function_init;
+  s->super.deinit = _function_deinit;
   s->super.free_fn = _function_free;
 }
 
+FilterXExpr *
+filterx_generator_function_optimize_method(FilterXGeneratorFunction *s)
+{
+  return filterx_generator_optimize_method(&s->super.super);
+}
+
 gboolean
 filterx_generator_function_init_method(FilterXGeneratorFunction *s, GlobalConfig *cfg)
 {
@@ -312,7 +342,28 @@ filterx_generator_function_free_method(FilterXGeneratorFunction *s)
   filterx_generator_free_method(&s->super.super);
 }
 
-static void
+static inline FilterXExpr *
+_generator_function_optimize(FilterXExpr *s)
+{
+  FilterXGeneratorFunction *self = (FilterXGeneratorFunction *) s;
+  return filterx_generator_function_optimize_method(self);
+}
+
+static inline gboolean
+_generator_function_init(FilterXExpr *s, GlobalConfig *cfg)
+{
+  FilterXGeneratorFunction *self = (FilterXGeneratorFunction *) s;
+  return filterx_generator_function_init_method(self, cfg);
+}
+
+static inline void
+_generator_function_deinit(FilterXExpr *s, GlobalConfig *cfg)
+{
+  FilterXGeneratorFunction *self = (FilterXGeneratorFunction *) s;
+  filterx_generator_function_deinit_method(self, cfg);
+}
+
+static inline void
 _generator_function_free(FilterXExpr *s)
 {
   FilterXGeneratorFunction *self = (FilterXGeneratorFunction *) s;
@@ -324,6 +375,9 @@ filterx_generator_function_init_instance(FilterXGeneratorFunction *s, const gcha
 {
   filterx_generator_init_instance(&s->super.super);
   s->function_name = g_strdup_printf("%s()", function_name);
+  s->super.super.optimize = _generator_function_optimize;
+  s->super.super.init = _generator_function_init;
+  s->super.super.deinit = _generator_function_deinit;
   s->super.super.free_fn = _generator_function_free;
 }
 

lib/filterx/expr-function.h

@@ -74,6 +74,8 @@ FilterXExpr *filterx_function_optimize_method(FilterXFunction *s);
 gboolean filterx_function_init_method(FilterXFunction *s, GlobalConfig *cfg);
 void filterx_function_deinit_method(FilterXFunction *s, GlobalConfig *cfg);
 void filterx_function_free_method(FilterXFunction *s);
+
+FilterXExpr *filterx_generator_function_optimize_method(FilterXGeneratorFunction *s);
 gboolean filterx_generator_function_init_method(FilterXGeneratorFunction *s, GlobalConfig *cfg);
 void filterx_generator_function_deinit_method(FilterXGeneratorFunction *s, GlobalConfig *cfg);
 void filterx_generator_function_init_instance(FilterXGeneratorFunction *s, const gchar *function_name);

lib/filterx/expr-null-coalesce.c

@@ -59,26 +59,34 @@ _eval(FilterXExpr *s)
   return lhs_object;
 }
 
-FilterXExpr *
-filterx_null_coalesce_new(FilterXExpr *lhs, FilterXExpr *rhs)
+static FilterXExpr *
+_optimize(FilterXExpr *s)
 {
-  if (filterx_expr_is_literal(lhs))
-    {
-      FilterXObject *lhs_object = filterx_expr_eval(lhs);
-      if (!lhs_object || filterx_object_is_type(lhs_object, &FILTERX_TYPE_NAME(null)))
-        {
-          filterx_object_unref(lhs_object);
-          filterx_expr_unref(lhs);
-          return rhs;
-        }
+  FilterXNullCoalesce *self = (FilterXNullCoalesce *) s;
+
+  if (filterx_binary_op_optimize_method(s))
+    g_assert_not_reached();
+
+  if (!filterx_expr_is_literal(self->super.lhs))
+    return NULL;
 
+  FilterXObject *lhs_object = filterx_expr_eval(self->super.lhs);
+  if (!lhs_object || filterx_object_is_type(lhs_object, &FILTERX_TYPE_NAME(null)))
+    {
       filterx_object_unref(lhs_object);
-      filterx_expr_unref(rhs);
-      return lhs;
+      return filterx_expr_ref(self->super.rhs);
     }
 
+  filterx_object_unref(lhs_object);
+  return filterx_expr_ref(self->super.lhs);
+}
+
+FilterXExpr *
+filterx_null_coalesce_new(FilterXExpr *lhs, FilterXExpr *rhs)
+{
   FilterXNullCoalesce *self = g_new0(FilterXNullCoalesce, 1);
   filterx_binary_op_init_instance(&self->super, "null_coalesce", lhs, rhs);
   self->super.super.eval = _eval;
+  self->super.super.optimize = _optimize;
   return &self->super.super;
 }

lib/filterx/filterx-metrics-labels.c

@@ -99,6 +99,23 @@ _label_cmp(gconstpointer a, gconstpointer b)
   return strcmp(lhs->name, rhs->name);
 }
 
+static void
+_label_optimize(FilterXMetricsLabel *self)
+{
+  self->value.expr = filterx_expr_optimize(self->value.expr);
+
+  if (!filterx_expr_is_literal(self->value.expr))
+    return;
+
+  ScratchBuffersMarker marker;
+  scratch_buffers_mark(&marker);
+  self->value.str = g_strdup(_format_value_expr(self->value.expr));
+  scratch_buffers_reclaim_marked(marker);
+
+  filterx_expr_unref(self->value.expr);
+  self->value.expr = NULL;
+}
+
 static gboolean
 _label_init(FilterXMetricsLabel *self, GlobalConfig *cfg)
 {
@@ -140,23 +157,6 @@ _init_label_name(FilterXExpr *name)
   return str;
 }
 
-static gboolean
-_init_label_value(FilterXMetricsLabel *self, FilterXExpr *value)
-{
-  if (!filterx_expr_is_literal(value))
-    {
-      self->value.expr = filterx_expr_ref(value);
-      return TRUE;
-    }
-
-  ScratchBuffersMarker marker;
-  scratch_buffers_mark(&marker);
-  self->value.str = g_strdup(_format_value_expr(value));
-  scratch_buffers_reclaim_marked(marker);
-
-  return !!self->value.str;
-}
-
 static FilterXMetricsLabel *
 _label_new(FilterXExpr *name, FilterXExpr *value)
 {
@@ -166,8 +166,7 @@ _label_new(FilterXExpr *name, FilterXExpr *value)
   if (!self->name)
     goto error;
 
-  if (!_init_label_value(self, value))
-    goto error;
+  self->value.expr = filterx_expr_ref(value);
 
   return self;
 
@@ -317,6 +316,34 @@ filterx_metrics_labels_format(FilterXMetricsLabels *self, DynMetricsStore *store
   return TRUE;
 }
 
+static gboolean
+_calculate_constness(FilterXMetricsLabels *self)
+{
+  if (self->expr)
+    return FALSE;
+
+  gboolean is_const = TRUE;
+  g_ptr_array_foreach(self->literal_labels, _check_label_is_const, &is_const);
+  return is_const;
+}
+
+void
+filterx_metrics_labels_optimize(FilterXMetricsLabels *self)
+{
+  self->expr = filterx_expr_optimize(self->expr);
+
+  if (self->literal_labels)
+    {
+      for (guint i = 0; i < self->literal_labels->len; i++)
+        {
+          FilterXMetricsLabel *label = g_ptr_array_index(self->literal_labels, i);
+          _label_optimize(label);
+        }
+    }
+
+  self->is_const = _calculate_constness(self);
+}
+
 gboolean
 filterx_metrics_labels_init(FilterXMetricsLabels *self, GlobalConfig *cfg)
 {
@@ -419,17 +446,6 @@ _init_labels(FilterXMetricsLabels *self, FilterXExpr *labels)
   return TRUE;
 }
 
-static gboolean
-_calculate_constness(FilterXMetricsLabels *self)
-{
-  if (self->expr)
-    return FALSE;
-
-  gboolean is_const = TRUE;
-  g_ptr_array_foreach(self->literal_labels, _check_label_is_const, &is_const);
-  return is_const;
-}
-
 FilterXMetricsLabels *
 filterx_metrics_labels_new(FilterXExpr *labels)
 {
@@ -441,7 +457,7 @@ filterx_metrics_labels_new(FilterXExpr *labels)
       return NULL;
     }
 
-  self->is_const = _calculate_constness(self);
+  self->is_const = FALSE;
 
   return self;
 }

lib/filterx/filterx-metrics-labels.h

@@ -31,6 +31,7 @@
 typedef struct _FilterXMetricsLabels FilterXMetricsLabels;
 
 FilterXMetricsLabels *filterx_metrics_labels_new(FilterXExpr *labels);
+void filterx_metrics_labels_optimize(FilterXMetricsLabels *self);
 gboolean filterx_metrics_labels_init(FilterXMetricsLabels *self, GlobalConfig *cfg);
 void filterx_metrics_labels_deinit(FilterXMetricsLabels *self, GlobalConfig *cfg);
 void filterx_metrics_labels_free(FilterXMetricsLabels *self);

lib/filterx/filterx-metrics.c

@@ -104,7 +104,31 @@ _is_const(FilterXMetrics *self)
 }
 
 static void
-_optimize(FilterXMetrics *self)
+_optimize_key(FilterXMetrics *self)
+{
+  self->key.expr = filterx_expr_optimize(self->key.expr);
+
+  if (!filterx_expr_is_literal(self->key.expr))
+    return;
+
+  FilterXObject *key_obj = filterx_expr_eval_typed(self->key.expr);
+  if (!filterx_object_is_type(key_obj, &FILTERX_TYPE_NAME(string)))
+    {
+      filterx_object_unref(key_obj);
+      return;
+    }
+
+  /* There are no literal message values, so we don't need to call extract_string() here. */
+  self->key.str = g_strdup(filterx_string_get_value_ref(key_obj, NULL));
+
+  filterx_expr_unref(self->key.expr);
+  self->key.expr = NULL;
+
+  filterx_object_unref(key_obj);
+}
+
+static void
+_optimize_counter(FilterXMetrics *self)
 {
   DynMetricsStore *store = dyn_metrics_cache();
 
@@ -136,6 +160,14 @@ _optimize(FilterXMetrics *self)
   stats_unlock();
 }
 
+void
+filterx_metrics_optimize(FilterXMetrics *self)
+{
+  _optimize_key(self);
+  filterx_metrics_labels_optimize(self->labels);
+  _optimize_counter(self);
+}
+
 gboolean
 filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **counter)
 {
@@ -182,8 +214,6 @@ filterx_metrics_init(FilterXMetrics *self, GlobalConfig *cfg)
       return FALSE;
     }
 
-  _optimize(self);
-
   return TRUE;
 }
 
@@ -213,36 +243,6 @@ filterx_metrics_free(FilterXMetrics *self)
   g_free(self);
 }
 
-static gboolean
-_init_key(FilterXMetrics *self, FilterXExpr *key)
-{
-  if (!filterx_expr_is_literal(key))
-    {
-      self->key.expr = filterx_expr_ref(key);
-      return TRUE;
-    }
-
-  FilterXObject *key_obj = filterx_expr_eval_typed(key);
-  if (!filterx_object_is_type(key_obj, &FILTERX_TYPE_NAME(string)))
-    {
-      filterx_eval_push_error_info("failed to init metrics key, key must be a string", key,
-                                   g_strdup_printf("got %s instread", key_obj->type->name), TRUE);
-      filterx_object_unref(key_obj);
-      return FALSE;
-    }
-
-  /* There are no literal message values, so we don't need to call extract_string() here. */
-  self->key.str = g_strdup(filterx_string_get_value_ref(key_obj, NULL));
-  return TRUE;
-}
-
-static gboolean
-_init_labels(FilterXMetrics *self, FilterXExpr *labels)
-{
-  self->labels = filterx_metrics_labels_new(labels);
-  return !!self->labels;
-}
-
 FilterXMetrics *
 filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels)
 {
@@ -251,11 +251,10 @@ filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels)
   g_assert(key);
 
   self->level = level;
+  self->key.expr = filterx_expr_ref(key);
 
-  if (!_init_key(self, key))
-    goto error;
-
-  if (!_init_labels(self, labels))
+  self->labels = filterx_metrics_labels_new(labels);
+  if (!self->labels)
     goto error;
 
   return self;

lib/filterx/filterx-metrics.h

@@ -34,6 +34,7 @@ gboolean filterx_metrics_is_enabled(FilterXMetrics *self);
 gboolean filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **counter);
 
 FilterXMetrics *filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels);
+void filterx_metrics_optimize(FilterXMetrics *self);
 gboolean filterx_metrics_init(FilterXMetrics *self, GlobalConfig *cfg);
 void filterx_metrics_deinit(FilterXMetrics *self, GlobalConfig *cfg);
 void filterx_metrics_free(FilterXMetrics *self);