optimizing NOP emissions (Re: [Nautilus-list] nautilus & signals)

[Tim Janik <timj gtk org>]

On 10 Apr 2002, Michael Meeks wrote:

> 	Well it's certainly a shame that signal emissions are not faster,
> presumably 99% of the notify emissions are not actually listened to by
> anyone, tens of thousands of them - it seems that handling the case
> where there is no-one listening to or implementing a signal is not
> overly fast - we get to do a chunk of lock/unlocks, unwind the argument
> va_list, etc. etc. looks like it certainly could be optimized. Anyway,
> there's no reason why Tim should be the only one that can optimize ?

first, locks/unlocks don't need to be slow per-se, a pthread lock/unlock
pair takes 40 nano seconds on my machine (550Mhz dual processor system).
though what gthread adds on top of that is another matter, _locking_
is definitely not a problem for gsignal.c (for gtype.c it was, and we
fixed that).

you got a point in NOP emissions though. the problem here is that it's
1) fairly hard to correctly figure that an emission is not neccessary
2) expensive too some extend, i.e. extra overhead is added to all
   emissions which cannot be skipped.

so for (2), what we need is somewhat representative profiling data, if it
showes that either:
- during normal program operations, there're much more signal emissions which
  could be skipped than emissions which can not, or
- the overhead to figure whether an emission could be skipped is negligable
  compared to emitting a signal,
optimizing NOP emissions is actually worth incorporating into the standard
gobject distro.

for (1), i have appended a patch that people can check out (should better be
thoroughly tested) and profile (i don't have time to do thorough profiling
of gtk+-2.0 based third party apps at this point, and we need good profiling
data to judge over (2)).

> 	The other thing is that I must say I'm not convinced about the choice
> of overloading shed loads of signals on objects to implement complex
> functionality instead of just sub-classing it in the normal way. It
> seems to me a fairly obscure and complex way of doing something that is
> more legible, standard (and faster) to do with virtual methods and
> inheritance.

subclassing isn't necessarily faster, there's lots of extra type system
stuff involved for this, and often it is cheaper to add one or two signal
handlers to an object if minor tweaks of behaviour are desired.
however, at a certain point of required behaviour changes, subclassing
is the cleaner alternative (though it's somewhat tedious with our C based
object system, which is why many developers step away from subclassing),
and for a long time now, i more often lean towards subclassing than
extensive signal/object-data hackery than not.

> 
> 	Regards,
> 
> 		Michael.
> 

---
ciaoTJ


Index: gsignal.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gsignal.c,v
retrieving revision 1.43
diff -u -u -p -r1.43 gsignal.c
--- gsignal.c	21 Mar 2002 00:34:05 -0000	1.43
+++ gsignal.c	11 Apr 2002 21:53:15 -0000
@@ -172,6 +172,7 @@ struct _SignalNode
   guint              destroyed : 1;
   
   /* reinitializable portion */
+  guint		     test_class_offset : 12;
   guint              flags : 8;
   guint              n_params : 8;
   GType		    *param_types; /* mangled with G_SIGNAL_TYPE_STATIC_SCOPE flag */
@@ -181,6 +182,8 @@ struct _SignalNode
   GSignalCMarshaller c_marshaller;
   GHookList         *emission_hooks;
 };
+#define	MAX_TEST_CLASS_OFFSET	(4096)	/* 2^12, 12 bits for test_class_offset */
+#define	TEST_CLASS_MAGIC	(1)	/* NULL class closure, but optimizable */
 
 struct _SignalKey
 {
@@ -1120,6 +1123,18 @@ g_signal_new (const gchar	 *signal_name,
                                    return_type, n_params, args);
 
   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)
+    {
+      SignalNode *node;
+
+      SIGNAL_LOCK ();
+      node = LOOKUP_SIGNAL_NODE (signal_id);
+      node->test_class_offset = class_offset;
+      SIGNAL_UNLOCK ();
+    }
  
   return signal_id;
 }
@@ -1170,6 +1185,9 @@ signal_add_class_closure (SignalNode *no
 {
   ClassClosure key;
 
+  /* can't optimize NOP emissions with overridden class closures */
+  node->test_class_offset = 0;
+
   if (!node->class_closure_bsa)
     node->class_closure_bsa = g_bsearch_array_new (&g_class_closure_bconfig);
   key.instance_type = itype;
@@ -1285,7 +1303,8 @@ g_signal_newv (const gchar       *signal
       g_signal_key_bsa = g_bsearch_array_insert (g_signal_key_bsa, &g_signal_key_bconfig, &key, FALSE);
     }
   node->destroyed = FALSE;
-  
+  node->test_class_offset = 0;
+
   /* setup reinitializable portion */
   node->flags = signal_flags & G_SIGNAL_FLAGS_MASK;
   node->n_params = n_params;
@@ -1304,6 +1323,11 @@ g_signal_newv (const gchar       *signal
   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)
+    {
+      /* optimize NOP emissions */
+      node->test_class_offset = TEST_CLASS_MAGIC;
+    }
   SIGNAL_UNLOCK ();
 
   return signal_id;
@@ -1351,6 +1375,7 @@ signal_destroy_R (SignalNode *signal_nod
   signal_node->destroyed = TRUE;
   
   /* reentrancy caution, zero out real contents first */
+  signal_node->test_class_offset = 0;
   signal_node->n_params = 0;
   signal_node->param_types = NULL;
   signal_node->return_type = 0;
@@ -1944,6 +1969,50 @@ g_signal_has_handler_pending (gpointer i
   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;
+}
+
 void
 g_signal_emitv (const GValue *instance_and_params,
 		guint         signal_id,
@@ -2017,6 +2086,15 @@ g_signal_emitv (const GValue *instance_a
     return_value = NULL;
 #endif	/* G_ENABLE_DEBUG */
 
+  /* optimize NOP emissions */
+  if (signal_check_skip_emission (node, instance, detail))
+    {
+      /* 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);
 }
@@ -2052,6 +2130,15 @@ g_signal_emit_valist (gpointer instance,
       return;
     }
 #endif  /* !G_DISABLE_CHECKS */
+
+  /* optimize NOP emissions */
+  if (signal_check_skip_emission (node, instance, detail))
+    {
+      /* nothing to do to emit this signal */
+      SIGNAL_UNLOCK ();
+      g_printerr ("omitting emission of \"%s\"\n", node->name);
+      return;
+    }
 
   n_params = node->n_params;
   signal_return_type = node->return_type;