GNOME.org
 

[glib/signal-performance: 9/10] Optimize single-handler va_marshaller case

commit f02ec2f2de3c9863da36be951662ddf4080adfaa
Author: Alexander Larsson <alexl redhat com>
Date:   Wed Feb 22 19:36:05 2012 +0100

    Optimize single-handler va_marshaller case
    
    When there is only one closure handling a signal emission and
    it doesn't have a bunch of complicated features enabled we
    can short circuit the va_args collection into GValues and call the
    callback via the va_marshaller directly.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=661140

 gobject/gsignal.c         |  344 ++++++++++++++++++++++++++++++++-------------
 gobject/gvaluecollector.h |   30 ++++
 2 files changed, 274 insertions(+), 100 deletions(-)
---
diff --git a/gobject/gsignal.c b/gobject/gsignal.c
index 5b5d301..54542c1 100644
--- a/gobject/gsignal.c
+++ b/gobject/gsignal.c
@@ -106,7 +106,9 @@
 
 #define REPORT_BUG      "please report occurrence circumstances to gtk-devel-list gnome org"
 #ifdef	G_ENABLE_DEBUG
-#define IF_DEBUG(debug_type, cond)	if ((_g_type_debug_flags & G_TYPE_DEBUG_ ## debug_type) || cond)
+#define COND_DEBUG(debug_type, cond)	((_g_type_debug_flags & G_TYPE_DEBUG_ ## debug_type) || (cond))
+#define IF_DEBUG(debug_type, cond)	if (COND_DEBUG(debug_type, cond))
+
 static volatile gpointer g_trace_instance_signals = NULL;
 static volatile gpointer g_trap_instance_signals = NULL;
 #endif	/* G_ENABLE_DEBUG */
@@ -181,6 +183,7 @@ static	      gboolean		signal_emit_unlocked_R	(SignalNode	 *node,
 							 const GValue	 *instance_and_params);
 static const gchar *            type_debug_name         (GType            type);
 static void                     node_check_deprecated   (const SignalNode *node);
+static void                     node_update_single_va_closure (SignalNode *node);
 
 
 /* --- structures --- */
@@ -205,9 +208,10 @@ struct _SignalNode
   guint              destroyed : 1;
   
   /* reinitializable portion */
-  guint		     test_class_offset : 12;
   guint              flags : 9;
   guint              n_params : 8;
+  guint              single_va_closure_is_valid : 1;
+  guint              single_va_closure_is_after : 1;
   GType		    *param_types; /* mangled with G_SIGNAL_TYPE_STATIC_SCOPE flag */
   GType		     return_type; /* mangled with G_SIGNAL_TYPE_STATIC_SCOPE flag */
   GBSearchArray     *class_closure_bsa;
@@ -215,9 +219,11 @@ struct _SignalNode
   GSignalCMarshaller c_marshaller;
   GSignalCVaMarshaller va_marshaller;
   GHookList         *emission_hooks;
+
+  GClosure *single_va_closure;
 };
-#define	MAX_TEST_CLASS_OFFSET	(4096)	/* 2^12, 12 bits for test_class_offset */
-#define	TEST_CLASS_MAGIC	(1)	/* indicates NULL class closure, candidate for NOP optimization */
+
+#define	SINGLE_VA_CLOSURE_EMPTY_MAGIC GINT_TO_POINTER(1)	/* indicates single_va_closure is valid but empty */
 
 struct _SignalKey
 {
@@ -687,6 +693,47 @@ handler_insert (guint    signal_id,
     hlist->tail_after = handler;
 }
 
+static void
+node_update_single_va_closure (SignalNode *node)
+{
+  GClosure *closure = NULL;
+  gboolean is_after = FALSE;
+
+  /* Fast path single-handler without boxing the arguments in GValues */
+  if (G_TYPE_IS_OBJECT (node->itype) &&
+      (node->flags & (G_SIGNAL_NO_RECURSE|G_SIGNAL_MUST_COLLECT)) == 0 &&
+      (node->emission_hooks == NULL || node->emission_hooks->hooks == NULL))
+    {
+      GSignalFlags run_type;
+      ClassClosure * cc; 
+      GBSearchArray *bsa = node->class_closure_bsa;
+
+      if (bsa == NULL || bsa->n_nodes == 0)
+	closure = SINGLE_VA_CLOSURE_EMPTY_MAGIC;
+      else if (bsa->n_nodes == 1)
+	{
+	  /* Look for default class closure (can't support non-default as it
+	     chains up using GValues */
+	  cc = g_bsearch_array_get_nth (bsa, &g_class_closure_bconfig, 0);
+	  if (cc->instance_type == 0)
+	    {
+	      run_type = node->flags & (G_SIGNAL_RUN_FIRST|G_SIGNAL_RUN_LAST|G_SIGNAL_RUN_CLEANUP);
+	      /* Only support *one* of run-first or run-last, not multiple or cleanup */
+	      if (run_type == G_SIGNAL_RUN_FIRST ||
+		  run_type == G_SIGNAL_RUN_LAST)
+		{
+		  closure = cc->closure;
+		  is_after = (run_type == G_SIGNAL_RUN_LAST);
+		}
+	    }
+	}
+    }
+
+  node->single_va_closure_is_valid = TRUE;
+  node->single_va_closure = closure;
+  node->single_va_closure_is_after = is_after;
+}
+
 static inline void
 emission_push (Emission **emission_list_p,
 	       Emission  *emission)
@@ -943,6 +990,7 @@ g_signal_add_emission_hook (guint               signal_id,
   node->emission_hooks->seq_id = seq_hook_id;
   g_hook_append (node->emission_hooks, hook);
   seq_hook_id = node->emission_hooks->seq_id;
+
   SIGNAL_UNLOCK ();
 
   return hook->hook_id;
@@ -971,6 +1019,9 @@ g_signal_remove_emission_hook (guint  signal_id,
     g_warning ("%s: invalid signal id `%u'", G_STRLOC, signal_id);
   else if (!node->emission_hooks || !g_hook_destroy (node->emission_hooks, hook_id))
     g_warning ("%s: signal \"%s\" had no hook (%lu) to remove", G_STRLOC, node->name, hook_id);
+
+  node->single_va_closure_is_valid = FALSE;
+
   SIGNAL_UNLOCK ();
 }
 
@@ -1346,19 +1397,6 @@ g_signal_new (const gchar	 *signal_name,
 
   va_end (args);
 
-  /* optimize NOP emissions with NULL class handlers */
-  if (signal_id && G_TYPE_IS_INSTANTIATABLE (itype) && return_type == G_TYPE_NONE &&
-      class_offset && class_offset < MAX_TEST_CLASS_OFFSET &&
-      ~signal_flags & G_SIGNAL_MUST_COLLECT)
-    {
-      SignalNode *node;
-
-      SIGNAL_LOCK ();
-      node = LOOKUP_SIGNAL_NODE (signal_id);
-      node->test_class_offset = class_offset;
-      SIGNAL_UNLOCK ();
-    }
- 
   return signal_id;
 }
 
@@ -1481,8 +1519,7 @@ signal_add_class_closure (SignalNode *node,
 {
   ClassClosure key;
 
-  /* can't optimize NOP emissions with overridden class closures */
-  node->test_class_offset = 0;
+  node->single_va_closure_is_valid = FALSE;
 
   if (!node->class_closure_bsa)
     node->class_closure_bsa = g_bsearch_array_create (&g_class_closure_bconfig);
@@ -1636,9 +1673,9 @@ g_signal_newv (const gchar       *signal_name,
       TRACE(GOBJECT_SIGNAL_NEW(signal_id, name, itype));
     }
   node->destroyed = FALSE;
-  node->test_class_offset = 0;
 
   /* setup reinitializable portion */
+  node->single_va_closure_is_valid = FALSE;
   node->flags = signal_flags & G_SIGNAL_FLAGS_MASK;
   node->n_params = n_params;
   node->param_types = g_memdup (param_types, sizeof (GType) * n_params);
@@ -1708,13 +1745,7 @@ g_signal_newv (const gchar       *signal_name,
   node->emission_hooks = NULL;
   if (class_closure)
     signal_add_class_closure (node, 0, class_closure);
-  else if (G_TYPE_IS_INSTANTIATABLE (itype) &&
-           return_type == G_TYPE_NONE &&
-           ~signal_flags & G_SIGNAL_MUST_COLLECT)
-    {
-      /* optimize NOP emissions */
-      node->test_class_offset = TEST_CLASS_MAGIC;
-    }
+
   SIGNAL_UNLOCK ();
 
   g_free (name);
@@ -1744,6 +1775,9 @@ g_signal_set_va_marshaller (guint              signal_id,
 	    _g_closure_set_va_marshal (cc->closure, va_marshaller);
 	}
     }
+
+  node->single_va_closure_is_valid = FALSE;
+
   SIGNAL_UNLOCK ();
 }
 
@@ -1817,7 +1851,7 @@ signal_destroy_R (SignalNode *signal_node)
   signal_node->destroyed = TRUE;
   
   /* reentrancy caution, zero out real contents first */
-  signal_node->test_class_offset = 0;
+  signal_node->single_va_closure_is_valid = FALSE;
   signal_node->n_params = 0;
   signal_node->param_types = NULL;
   signal_node->return_type = 0;
@@ -2887,50 +2921,6 @@ g_signal_has_handler_pending (gpointer instance,
   return has_pending;
 }
 
-static inline gboolean
-signal_check_skip_emission (SignalNode *node,
-			    gpointer    instance,
-			    GQuark      detail)
-{
-  HandlerList *hlist;
-
-  /* are we able to check for NULL class handlers? */
-  if (!node->test_class_offset)
-    return FALSE;
-
-  /* are there emission hooks pending? */
-  if (node->emission_hooks && node->emission_hooks->hooks)
-    return FALSE;
-
-  /* is there a non-NULL class handler? */
-  if (node->test_class_offset != TEST_CLASS_MAGIC)
-    {
-      GTypeClass *class = G_TYPE_INSTANCE_GET_CLASS (instance, G_TYPE_FROM_INSTANCE (instance), GTypeClass);
-
-      if (G_STRUCT_MEMBER (gpointer, class, node->test_class_offset))
-	return FALSE;
-    }
-
-  /* are signals being debugged? */
-#ifdef  G_ENABLE_DEBUG
-  IF_DEBUG (SIGNALS, g_trace_instance_signals || g_trap_instance_signals)
-    return FALSE;
-#endif /* G_ENABLE_DEBUG */
-
-  /* is this a no-recurse signal already in emission? */
-  if (node->flags & G_SIGNAL_NO_RECURSE &&
-      emission_find (g_restart_emissions, node->signal_id, detail, instance))
-    return FALSE;
-
-  /* do we have pending handlers? */
-  hlist = handler_list_lookup (node->signal_id, instance);
-  if (hlist && hlist->handlers)
-    return FALSE;
-
-  /* none of the above, no emission required */
-  return TRUE;
-}
-
 /**
  * g_signal_emitv:
  * @instance_and_params: (array): argument list for the signal emission.
@@ -3021,18 +3011,49 @@ g_signal_emitv (const GValue *instance_and_params,
 #endif	/* G_ENABLE_DEBUG */
 
   /* optimize NOP emissions */
-  if (signal_check_skip_emission (node, instance, detail))
+  if (!node->single_va_closure_is_valid)
+    node_update_single_va_closure (node);
+
+  if (node->single_va_closure != NULL &&
+      (node->single_va_closure == SINGLE_VA_CLOSURE_EMPTY_MAGIC ||
+       _g_closure_is_void (node->single_va_closure, instance))
+#ifdef	G_ENABLE_DEBUG
+      && !COND_DEBUG (SIGNALS, g_trace_instance_signals != instance &&
+		      g_trap_instance_signals == instance)
+#endif	/* G_ENABLE_DEBUG */
+      )
     {
-      /* nothing to do to emit this signal */
-      SIGNAL_UNLOCK ();
-      /* g_printerr ("omitting emission of \"%s\"\n", node->name); */
-      return;
+      HandlerList* hlist = handler_list_lookup (node->signal_id, instance);
+      if (hlist == NULL || hlist->handlers == NULL)
+	{
+	  /* nothing to do to emit this signal */
+	  SIGNAL_UNLOCK ();
+	  /* g_printerr ("omitting emission of \"%s\"\n", node->name); */
+	  return;
+	}
     }
 
   SIGNAL_UNLOCK ();
   signal_emit_unlocked_R (node, detail, instance, return_value, instance_and_params);
 }
 
+static inline gboolean
+accumulate (GSignalInvocationHint *ihint,
+	    GValue                *return_accu,
+	    GValue	          *handler_return,
+	    SignalAccumulator     *accumulator)
+{
+  gboolean continue_emission;
+
+  if (!accumulator)
+    return TRUE;
+
+  continue_emission = accumulator->func (ihint, return_accu, handler_return, accumulator->data);
+  g_value_reset (handler_return);
+
+  return continue_emission;
+}
+
 /**
  * g_signal_emit_valist:
  * @instance: the instance the signal is being emitted on.
@@ -3079,13 +3100,153 @@ g_signal_emit_valist (gpointer instance,
     }
 #endif  /* !G_DISABLE_CHECKS */
 
-  /* optimize NOP emissions */
-  if (signal_check_skip_emission (node, instance, detail))
+  if (!node->single_va_closure_is_valid)
+    node_update_single_va_closure (node);
+
+  if (node->single_va_closure != NULL
+#ifdef	G_ENABLE_DEBUG
+      && !COND_DEBUG (SIGNALS, g_trace_instance_signals != instance &&
+		      g_trap_instance_signals == instance)
+#endif	/* G_ENABLE_DEBUG */
+      )
     {
-      /* nothing to do to emit this signal */
-      SIGNAL_UNLOCK ();
-      /* g_printerr ("omitting emission of \"%s\"\n", node->name); */
-      return;
+      HandlerList* hlist = handler_list_lookup (node->signal_id, instance);
+      Handler *l;
+      GClosure *closure = NULL;
+      gboolean fastpath = TRUE;
+      GSignalFlags run_type = G_SIGNAL_RUN_FIRST;
+
+      if (node->single_va_closure != SINGLE_VA_CLOSURE_EMPTY_MAGIC &&
+	  !_g_closure_is_void (node->single_va_closure, instance))
+	{
+	  if (_g_closure_supports_invoke_va (node->single_va_closure))
+	    {
+	      closure = node->single_va_closure;
+	      if (node->single_va_closure_is_after)
+		run_type = G_SIGNAL_RUN_LAST;
+	      else
+		run_type = G_SIGNAL_RUN_FIRST;
+	    }
+	  else
+	    fastpath = FALSE;
+	}
+
+      for (l = hlist ? hlist->handlers : NULL; fastpath && l != NULL; l = l->next)
+	{
+	  if (!l->block_count &&
+	      (!l->detail || l->detail == detail))
+	    {
+	      if (closure != NULL || !_g_closure_supports_invoke_va (l->closure))
+		{
+		  fastpath = FALSE;
+		  break;
+		}
+	      else
+		{
+		  closure = l->closure;
+		  if (l->after)
+		    run_type = G_SIGNAL_RUN_LAST;
+		  else
+		    run_type = G_SIGNAL_RUN_FIRST;
+		}
+	    }
+	}
+
+      if (fastpath && closure == NULL && node->return_type == G_TYPE_NONE)
+	{
+	  SIGNAL_UNLOCK ();
+	  return;
+	}
+
+      if (fastpath)
+	{
+	  SignalAccumulator *accumulator;
+	  Emission emission;
+	  GValue *return_accu, accu = G_VALUE_INIT;
+	  guint signal_id;
+	  GValue emission_return = G_VALUE_INIT;
+          GType rtype = node->return_type & ~G_SIGNAL_TYPE_STATIC_SCOPE;
+	  gboolean static_scope = node->return_type & G_SIGNAL_TYPE_STATIC_SCOPE;
+
+	  signal_id = node->signal_id;
+	  accumulator = node->accumulator;
+	  if (rtype == G_TYPE_NONE)
+	    return_accu = NULL;
+	  else if (accumulator)
+	    return_accu = &accu;
+	  else
+	    return_accu = &emission_return;
+
+	  emission.instance = instance;
+	  emission.ihint.signal_id = node->signal_id;
+	  emission.ihint.detail = detail;
+	  emission.ihint.run_type = run_type;
+	  emission.state = EMISSION_RUN;
+	  emission.chain_type = G_TYPE_FROM_INSTANCE (instance);
+	  emission_push (&g_recursive_emissions, &emission);
+
+	  SIGNAL_UNLOCK ();
+
+	  TRACE(GOBJECT_SIGNAL_EMIT(signal_id, detail, instance, G_TYPE_FROM_INSTANCE (instance)));
+
+	  if (rtype != G_TYPE_NONE)
+	    g_value_init (&emission_return, rtype);
+
+	  if (accumulator)
+	    g_value_init (&accu, rtype);
+
+	  if (closure != NULL)
+	    {
+	      g_object_ref (instance);
+	      _g_closure_invoke_va (closure,
+				    return_accu,
+				    instance,
+				    var_args,
+				    node->n_params,
+				    node->param_types);
+	      accumulate (&emission.ihint, &emission_return, &accu, accumulator);
+	      g_object_unref (instance);
+	    }
+
+	  SIGNAL_LOCK ();
+
+	  emission.chain_type = G_TYPE_NONE;
+	  emission_pop (&g_recursive_emissions, &emission);
+
+	  SIGNAL_UNLOCK ();
+
+	  if (accumulator)
+	    g_value_unset (&accu);
+
+	  if (rtype != G_TYPE_NONE)
+	    {
+	      gchar *error = NULL;
+	      for (i = 0; i < node->n_params; i++)
+		{
+		  GType ptype = node->param_types[i] & ~G_SIGNAL_TYPE_STATIC_SCOPE;
+		  G_VALUE_COLLECT_SKIP (ptype, var_args);
+		}
+
+	      G_VALUE_LCOPY (&emission_return,
+			     var_args,
+			     static_scope ? G_VALUE_NOCOPY_CONTENTS : 0,
+			     &error);
+	      if (!error)
+		g_value_unset (&emission_return);
+	      else
+		{
+		  g_warning ("%s: %s", G_STRLOC, error);
+		  g_free (error);
+		  /* we purposely leak the value here, it might not be
+		   * in a sane state if an error condition occurred
+		   */
+		}
+	    }
+	  
+	  TRACE(GOBJECT_SIGNAL_EMIT_END(signal_id, detail, instance, G_TYPE_FROM_INSTANCE (instance)));
+
+	  return;
+	}
     }
 
   n_params = node->n_params;
@@ -3227,23 +3388,6 @@ g_signal_emit_by_name (gpointer     instance,
     g_warning ("%s: signal name `%s' is invalid for instance `%p'", G_STRLOC, detailed_signal, instance);
 }
 
-static inline gboolean
-accumulate (GSignalInvocationHint *ihint,
-	    GValue                *return_accu,
-	    GValue	          *handler_return,
-	    SignalAccumulator     *accumulator)
-{
-  gboolean continue_emission;
-
-  if (!accumulator)
-    return TRUE;
-
-  continue_emission = accumulator->func (ihint, return_accu, handler_return, accumulator->data);
-  g_value_reset (handler_return);
-
-  return continue_emission;
-}
-
 static gboolean
 signal_emit_unlocked_R (SignalNode   *node,
 			GQuark	      detail,
diff --git a/gobject/gvaluecollector.h b/gobject/gvaluecollector.h
index 9bdf482..6d5190e 100644
--- a/gobject/gvaluecollector.h
+++ b/gobject/gvaluecollector.h
@@ -158,6 +158,36 @@ G_STMT_START {										\
   G_VALUE_COLLECT_INIT(value, _value_type, var_args, flags, __error);			\
 } G_STMT_END
 
+#define G_VALUE_COLLECT_SKIP(_value_type, var_args)					\
+G_STMT_START {										\
+  GTypeValueTable *_vtable = g_type_value_table_peek (_value_type);			\
+  gchar *_collect_format = _vtable->collect_format;					\
+                                                                                        \
+  while (*_collect_format)								\
+    {											\
+      switch (*_collect_format++)							\
+	{										\
+	case G_VALUE_COLLECT_INT:							\
+	  va_arg ((var_args), gint);							\
+	  break;									\
+	case G_VALUE_COLLECT_LONG:							\
+	  va_arg ((var_args), glong);							\
+	  break;									\
+	case G_VALUE_COLLECT_INT64:							\
+	  va_arg ((var_args), gint64);							\
+	  break;									\
+	case G_VALUE_COLLECT_DOUBLE:							\
+	  va_arg ((var_args), gdouble);							\
+	  break;									\
+	case G_VALUE_COLLECT_POINTER:							\
+	  va_arg ((var_args), gpointer);						\
+	  break;									\
+	default:									\
+	  g_assert_not_reached ();							\
+	}										\
+    }											\
+} G_STMT_END
+
 /**
  * G_VALUE_LCOPY:
  * @value: a #GValue return location. @value is supposed to be initialized
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]