GNOME.org
 

Fast handling of class-closure-only signals

Recently, Tim applied a patch to gsignal.c that avoids emitting
signals that won't have any effect.  Such signals are common during
TreeView validation, so people who have performance problems here may
want to upgrade to GLib CVS HEAD.

Anyway, I got an idea from reading his patch. You can view what the
noop patch does as:

        - Is the class closure for this signal NULL?
 
        - if yes, does anything else than the class closure need to run?

        - if no, omit the emission.

So my idea is to export API that does the check in the second step. By
using that, gtk+ and other users of GObject can do

    if (g_signal_check_class_closure_only (object, signal_id, detail))
        FOO_GET_CLASS (object)->the_handler (...);
    else
        g_signal_emit (object, signal_id, detail);

ie. directly calling the virtual function if nothing else needs to be
done.

For signals that only rarely have handlers, like "expose" or
"size_allocate", but always a class closure, this is a big speed-up.

I am attaching two patches, one for glib and one for gtk+. The glib
patch exports the function g_signal_check_class_closure_only(), and
the gtk+ patch makes use of this call to check if the event, size
allocate, and the check_resize signals can be skipped.

With these patches applied I measured with Gnumeric and Nautilus how
often the signals were skipped by this during opaque resizing. The
results are that

   for Gnumeric 96.9% of the checked signals were skipped
   for Nautilus 94.6% of the checked signals were skipped

I also noted that the time spent inside g_signal_emit() (as measured
with SpeedProf) during resize of Gnumeric dropped to around 2%.

The main drawback is that for signals that actually *do* need to be
emitted rather than directly called, the "class_closure_only" code
will run twice. But as the numbers show, that is rare.


Søren


Index: gsignal.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gsignal.c,v
retrieving revision 1.52
diff -u -r1.52 gsignal.c
--- gsignal.c	19 Aug 2003 02:15:40 -0000	1.52
+++ gsignal.c	25 Aug 2003 19:33:03 -0000
@@ -1135,7 +1135,7 @@
   va_end (args);
 
   /* optimize NOP emissions with NULL class handlers */
-  if (signal_id && G_TYPE_IS_INSTANTIATABLE (itype) && return_type == G_TYPE_NONE &&
+  if (signal_id && G_TYPE_IS_INSTANTIATABLE (itype) &&
       class_offset && class_offset < MAX_TEST_CLASS_OFFSET)
     {
       SignalNode *node;
@@ -1331,7 +1331,7 @@
   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)
+  else if (G_TYPE_IS_INSTANTIATABLE (itype))
     {
       /* optimize NOP emissions */
       node->test_class_offset = TEST_CLASS_MAGIC;
@@ -1978,9 +1978,9 @@
 }
 
 static inline gboolean
-signal_check_skip_emission (SignalNode *node,
-			    gpointer    instance,
-			    GQuark      detail)
+signal_check_class_closure_only (SignalNode *node,
+				 gpointer    instance,
+				 GQuark      detail)
 {
   HandlerList *hlist;
 
@@ -1992,15 +1992,6 @@
   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)
@@ -2017,8 +2008,47 @@
   if (hlist && hlist->handlers)
     return FALSE;
 
+  /* none of the above, only class closure need to run */
+  return TRUE;
+}
+
+static inline gboolean
+signal_check_skip_emission (SignalNode *node,
+			    gpointer    instance,
+			    GQuark      detail)
+{
+  if (node->return_type != G_TYPE_NONE)
+    return FALSE;
+  
+  if (!signal_check_class_closure_only (node, instance, detail))
+    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;
+    }
+
   /* none of the above, no emission required */
   return TRUE;
+}
+
+gboolean
+g_signal_check_class_closure_only (gpointer instance,
+				   guint    id,
+				   GQuark   detail)
+{
+  gboolean result;
+  SignalNode *node;
+  
+  SIGNAL_LOCK ();
+  node = LOOKUP_SIGNAL_NODE (id);
+  result = signal_check_class_closure_only (node, instance, detail);
+  SIGNAL_UNLOCK ();
+  return result;
 }
 
 void
Index: gsignal.h
===================================================================
RCS file: /cvs/gnome/glib/gobject/gsignal.h,v
retrieving revision 1.35
diff -u -r1.35 gsignal.h
--- gsignal.h	3 Dec 2002 23:54:54 -0000	1.35
+++ gsignal.h	25 Aug 2003 19:33:03 -0000
@@ -139,6 +139,9 @@
 void                  g_signal_emit_by_name (gpointer            instance,
 					     const gchar        *detailed_signal,
 					     ...);
+gboolean	      g_signal_check_class_closure_only (gpointer instance,
+							 guint    id,
+							 GQuark   detail);
 guint                 g_signal_lookup       (const gchar        *name,
 					     GType               itype);
 G_CONST_RETURN gchar* g_signal_name         (guint               signal_id);

Index: gtk/gtkcontainer.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkcontainer.c,v
retrieving revision 1.124
diff -u -r1.124 gtkcontainer.c
--- gtk/gtkcontainer.c	8 Jul 2003 22:49:35 -0000	1.124
+++ gtk/gtkcontainer.c	27 Aug 2003 19:32:38 -0000
@@ -1184,8 +1184,16 @@
 gtk_container_check_resize (GtkContainer *container)
 {
   g_return_if_fail (GTK_IS_CONTAINER (container));
-  
-  g_signal_emit (container, container_signals[CHECK_RESIZE], 0);
+
+  if (g_signal_check_class_closure_only (container, container_signals[CHECK_RESIZE], 0))
+    {
+      GtkContainerClass *class = GTK_CONTAINER_GET_CLASS (container);
+
+      if (class->check_resize)
+	class->check_resize (container);
+    }
+  else
+    g_signal_emit (container, container_signals[CHECK_RESIZE], 0);
 }
 
 static void
Index: gtk/gtkwidget.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkwidget.c,v
retrieving revision 1.358
diff -u -r1.358 gtkwidget.c
--- gtk/gtkwidget.c	7 Aug 2003 21:03:18 -0000	1.358
+++ gtk/gtkwidget.c	27 Aug 2003 19:32:57 -0000
@@ -2643,7 +2643,15 @@
   if (!alloc_needed && !size_changed && !position_changed)
     return;
   
-  g_signal_emit (widget, widget_signals[SIZE_ALLOCATE], 0, &real_allocation);
+  if (g_signal_check_class_closure_only (widget, widget_signals[SIZE_ALLOCATE], 0))
+    {
+      GtkWidgetClass *class = GTK_WIDGET_GET_CLASS (widget);
+
+      if (class->size_allocate)
+	class->size_allocate (widget, &real_allocation);
+    }
+  else
+    g_signal_emit (widget, widget_signals[SIZE_ALLOCATE], 0, &real_allocation);
 
   if (GTK_WIDGET_MAPPED (widget))
     {
@@ -3356,6 +3364,7 @@
 			   GdkEvent  *event)
 {
   gboolean return_val = FALSE;
+  GtkWidgetClass *class;
 
   /* We check only once for is-still-visible; if someone
    * hides the window in on of the signals on the widget,
@@ -3367,10 +3376,22 @@
 
   g_object_ref (widget);
 
-  g_signal_emit (widget, widget_signals[EVENT], 0, event, &return_val);
+  class = GTK_WIDGET_GET_CLASS (widget);
+  
+  if (g_signal_check_class_closure_only (widget,
+					 widget_signals[EVENT], 0))
+    {
+      if (class->event)
+	return_val = class->event (widget, event);
+    }
+  else
+    g_signal_emit (widget, widget_signals[EVENT], 0, event, &return_val);
+  
   return_val |= !WIDGET_REALIZED_FOR_EVENT (widget, event);
   if (!return_val)
     {
+      typedef gboolean (* EventFunc) (GtkWidget *widget, GdkEvent *event);
+      EventFunc event_func = NULL;
       gint signal_num;
 
       switch (event->type)
@@ -3382,78 +3403,104 @@
 	case GDK_2BUTTON_PRESS:
 	case GDK_3BUTTON_PRESS:
 	  signal_num = BUTTON_PRESS_EVENT;
+	  event_func = (EventFunc)class->button_press_event;
 	  break;
 	case GDK_SCROLL:
 	  signal_num = SCROLL_EVENT;
+	  event_func = (EventFunc)class->scroll_event;
 	  break;
 	case GDK_BUTTON_RELEASE:
 	  signal_num = BUTTON_RELEASE_EVENT;
+	  event_func = (EventFunc)class->button_release_event;
 	  break;
 	case GDK_MOTION_NOTIFY:
 	  signal_num = MOTION_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->motion_notify_event;
 	  break;
 	case GDK_DELETE:
 	  signal_num = DELETE_EVENT;
+	  event_func = (EventFunc)class->delete_event;
 	  break;
 	case GDK_DESTROY:
 	  signal_num = DESTROY_EVENT;
+	  event_func = (EventFunc)class->destroy_event;
 	  break;
 	case GDK_KEY_PRESS:
 	  signal_num = KEY_PRESS_EVENT;
+	  event_func = (EventFunc)class->key_press_event;
 	  break;
 	case GDK_KEY_RELEASE:
 	  signal_num = KEY_RELEASE_EVENT;
+	  event_func = (EventFunc)class->key_release_event;
 	  break;
 	case GDK_ENTER_NOTIFY:
 	  signal_num = ENTER_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->enter_notify_event;
 	  break;
 	case GDK_LEAVE_NOTIFY:
 	  signal_num = LEAVE_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->leave_notify_event;
 	  break;
 	case GDK_FOCUS_CHANGE:
 	  signal_num = event->focus_change.in ? FOCUS_IN_EVENT : FOCUS_OUT_EVENT;
+	  event_func = (EventFunc)(event->focus_change.in ?
+				   class->focus_in_event : class->focus_out_event);
 	  break;
 	case GDK_CONFIGURE:
 	  signal_num = CONFIGURE_EVENT;
+	  event_func = (EventFunc)class->configure_event;
 	  break;
 	case GDK_MAP:
 	  signal_num = MAP_EVENT;
+	  event_func = (EventFunc)class->map_event;
 	  break;
 	case GDK_UNMAP:
 	  signal_num = UNMAP_EVENT;
+	  event_func = (EventFunc)class->unmap_event;
 	  break;
 	case GDK_WINDOW_STATE:
 	  signal_num = WINDOW_STATE_EVENT;
+	  event_func = (EventFunc)class->window_state_event;
 	  break;
 	case GDK_PROPERTY_NOTIFY:
 	  signal_num = PROPERTY_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->property_notify_event;
 	  break;
 	case GDK_SELECTION_CLEAR:
 	  signal_num = SELECTION_CLEAR_EVENT;
+	  event_func = (EventFunc)class->selection_clear_event;
 	  break;
 	case GDK_SELECTION_REQUEST:
 	  signal_num = SELECTION_REQUEST_EVENT;
+	  event_func = (EventFunc)class->selection_request_event;
 	  break;
 	case GDK_SELECTION_NOTIFY:
 	  signal_num = SELECTION_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->selection_notify_event;
 	  break;
 	case GDK_PROXIMITY_IN:
 	  signal_num = PROXIMITY_IN_EVENT;
+	  event_func = (EventFunc)class->proximity_in_event;
 	  break;
 	case GDK_PROXIMITY_OUT:
 	  signal_num = PROXIMITY_OUT_EVENT;
+	  event_func = (EventFunc)class->proximity_out_event;
 	  break;
 	case GDK_NO_EXPOSE:
 	  signal_num = NO_EXPOSE_EVENT;
+	  event_func = (EventFunc)class->no_expose_event;
 	  break;
 	case GDK_CLIENT_EVENT:
 	  signal_num = CLIENT_EVENT;
+	  event_func = (EventFunc)class->client_event;
 	  break;
 	case GDK_EXPOSE:
 	  signal_num = EXPOSE_EVENT;
+	  event_func = (EventFunc)class->expose_event;
 	  break;
 	case GDK_VISIBILITY_NOTIFY:
 	  signal_num = VISIBILITY_NOTIFY_EVENT;
+	  event_func = (EventFunc)class->visibility_notify_event;
 	  break;
 	default:
 	  g_warning ("gtk_widget_event(): unhandled event type: %d", event->type);
@@ -3461,8 +3508,17 @@
 	  break;
 	}
       if (signal_num != -1)
-	g_signal_emit (widget, widget_signals[signal_num], 0, event, &return_val);
+	{
+	  if (g_signal_check_class_closure_only (widget, widget_signals[signal_num], 0))
+	    {
+	      if (event_func)
+		return_val = event_func (widget, event);
+	    }
+	  else
+	    g_signal_emit (widget, widget_signals[signal_num], 0, event, &return_val);
+	}
     }
+
   if (WIDGET_REALIZED_FOR_EVENT (widget, event))
     g_signal_emit (widget, widget_signals[EVENT_AFTER], 0, event);
   else
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]