Re: Fast handling of class-closure-only signals

Tim Janik <timj gtk org> writes:

> so i suspect you're trying to optimize in an area where no one will ever
> notice the benefits of your optimizations (and that is at a significant
> cost in signal emission complexity).
> what i'd like to see to support your claims is profiling data showing that
> significant _time_ can be saved during signal emissions, and that
> this time is actually user visible.

I instrumented (patch attached) the part of gtk+ that does the bulk of
the work during opaque resize, gtk_container_idle_sizer(), to track
the average amount of time it took to execute that function.

Then I used the gnome-test-tool to record a lot of opaque resizing of
gnumeric. I replayed the resizing seven times and took the

Then I applied the patches, the glib one modified to check for
non-NULL class closure as the first thing in the NOP case, as you
suggested, and ran the script seven times again and took the average.


        0.0251s spent inside gtk_container_idle_sizer()


        0.0227s spent inside gtk_container_idle_sizer()

a speedup of 9.6%.

Profiling opaque resizing of Gnumeric usually shows
gtk_container_idle_sizer() to use about 80% of the total time, so the
effective speed-up would be 7.7%.

There are several error sources here, and in fact I think the real
number is probably higher, because the signal emission overhead I see
on profiles is often closer to 20% than to 10%.

> a few comments on the patches:
> - signal_check_skip_emission() does cheap and frequently failing
>   checks first, to reduce the cost of doing the check at all.
>   in your patched signal_check_skip_emission(), you're delaying
>   the check for a non-NULL class pointer until after all the expensive
>   checks have been performed, this introduces a performance penalty
>   in the very common case that there's a non-NULL class function pointer.
>   (if you do further profiling, please fix this first)

I moved the check for non-NULL class pointer to the front of the
function before measuring.

> - you're bypassing the return_type==G_TYPE_NONE check for class closures.
>   for the NOP emission optimization, the check is there because
>   normal emissions can't be skipped if return types are present since
>   that'd skip the return value initialization step which results in
>   bogus return values being returned.
>   also, if accumulators are present, we have to emit the signal
>   regardless, since the signal code may not guess the accumulator code
>   to be unimportant enough to simply be skipped.
>   while the first issue is probably being taken care of by the user
>   calling the class-function manually, i can only hope he's also fully
>   aware of any side effects the signal accumulator may have.

In this case the signal accumulator is just the standard boolean
accumulator which should be safe to leave out.


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	5 Sep 2003 18:35:32 -0000
@@ -1087,11 +1087,34 @@
   return GTK_IS_RESIZE_CONTAINER (widget) ? (GtkContainer*) widget : NULL;
+static gdouble avg = -1.0;
+static int n_samples;
+static void
+print_data (void)
+    g_print ("average time per gtk_container_idle_sizer(): %f\n", avg);
 static gboolean
 gtk_container_idle_sizer (gpointer data)
+  static GTimer *timer;
+  gdouble elapsed;
+  gboolean do_time;
+  if (!timer)
+  {
+      timer = g_timer_new ();
+      g_atexit (print_data); 
+  }
+  g_timer_reset (timer);
+  do_time = container_resize_queue != NULL;
   /* we may be invoked with a container_resize_queue of NULL, because
    * queue_resize could have been adding an extra idle function while
    * the queue still got processed. we better just ignore such case
@@ -1114,6 +1137,22 @@
   gdk_window_process_all_updates ();
+  if (do_time)
+    {
+      elapsed = g_timer_elapsed (timer, NULL);
+      if (avg < 0.0)
+      {
+	  avg = elapsed;
+	  n_samples = 1;
+      }
+      else
+      {
+	  n_samples++;
+	  avg = ((avg * (n_samples - 1)) + elapsed) / n_samples;
+      }
+    }
   return FALSE;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]