Re: Fast handling of class-closure-only signals

On 27 Aug 2003, Soeren Sandmann wrote:

> 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?

in particular, the function pointer of a c-class-closure (i.e.
the one used to wrap up class function pointers).

>         - 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'm not sure the speed up is that great for those signals, especially
if compared to what else the default handlers in those cases do.

and even if that were the case (huge speedups for ::expose etc.), i see
little point in speeding up signal emission for event handlers. events
simply occour comparatively rare during a programs life time and usually
programs have no problems handling them fast enough (let alone due
to the speed penalty involved in a signal emission).

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.

and even then, i'm not very fond of moving optimization logic out of
gsignal.c into user code. that's error prone, since complexity is
introduced on the "many side". i.e. there's only one gsignal.[hc] but
a multitude of other code portions using it (1:many), introducing
optimizations on the gsignal.[hc] side thusly benefits many code
portions, but forcing optimization hacks on the "many" side magnifies
a) potential for errors, and
b) work involved in order to benefit from the optimizations (since
   "many" code portions need changing).
thus, even if you encounter profiling data that shows significant
savings, i'd like to figure out where the additional time is spent,
and if we can't simply optimize that inside gsignal.c.

> 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.

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)
- 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.

> Søren


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