A few weeks back I promised Tim that I would come up with more credible numbers for http://bugzilla.gnome.org/show_bug.cgi?id=121027 which was previously discussed here http://mail.gnome.org/archives/gtk-devel-list/2003-September/msg00049.html So here is a benchmark. The benchmark contains an 'application' loosely modelled after Gnumeric. with lots of widgets in it. One of the widgets is a GtkHPaned, and the benchmark changes the position of the handle 2000 times causing lots of resizing and redrawing. The results: Without patch: elapsed time: 49.964 (40.029 resizes per second) With patch: elapsed time: 44.575 (44.868 resizes per second) so the patch is a clear improvement: 12.1% better framerate. The objections to the patch still stand: it adds an ugly and unsafe piece of API to glib. Some possible solutions to that: * Make the API libgtk_only This would allow us to get rid of the API later if something better is found. * Owen mentions in http://mail.gnome.org/archives/gtk-devel-list/2003-September/msg00159.html that the API could be a g_signal_emit_slow() function that returns FALSE if you can bypass the signal system. * Try to do the speedup entirely within g_signal_emit() I think it might be possible to skip both marshalling and argument collection by using a platform specific hack to convert a vararg list to a regular function call in the case where we can determine that it will work. I am not totally sure this will work, though. Attaching benchmark and patches. Søren
Attachment:
benchmark.tar.gz
Description: Benchmark
? gfreelist.c ? gfreelist.h ? glib-diff ? patches ? glib/gfreelist.c ? glib/gfreelist.h ? glib/memdiff ? gobject/core.469 ? gobject/glib-signal.patch ? gobject/paramspec.patch ? gobject/signal.patch Index: gobject/gsignal.c =================================================================== RCS file: /cvs/gnome/glib/gobject/gsignal.c,v retrieving revision 1.52 diff -u -r1.52 gsignal.c --- gobject/gsignal.c 19 Aug 2003 02:15:40 -0000 1.52 +++ gobject/gsignal.c 5 Sep 2003 14:21:27 -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; + + /* 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; + } + + if (!signal_check_class_closure_only (node, instance, detail)) + 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: gobject/gsignal.h =================================================================== RCS file: /cvs/gnome/glib/gobject/gsignal.h,v retrieving revision 1.35 diff -u -r1.35 gsignal.h --- gobject/gsignal.h 3 Dec 2002 23:54:54 -0000 1.35 +++ gobject/gsignal.h 5 Sep 2003 14:21:28 -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/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 29 Aug 2003 14:13:00 -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 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 29 Aug 2003 14:13:00 -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