Re: GtkBindingSignal changes

[Tim Janik <timj imendio com>]

On Wed, 4 Jan 2006, Matthias Clasen wrote:

> On Wed, 2006-01-04 at 14:42 +0100, Tim Janik wrote:
> > On Wed, 4 Jan 2006, Matthias Clasen wrote:
> >
> > > On Wed, 2006-01-04 at 13:28 +0100, Tim Janik wrote:
> > > > hi matthias,
> > > >
> > > > can you please outline the rationale for this change:
> > > >
> > > > 2005-12-27  Matthias Clasen  <mclasen redhat com>
> > > >
> > > >          * gtk/gtkbindings.h (GtkBindingSignal):
> > > >          * gtk/gtkbindings.c (binding_signal_new): Make the
> > > >          args a flexible array inside the struct, and allocate them
> > > >          together.
> > > >
> > > > i think this needs to be backed out because:
> > > >
> > > > 1) this is an incompatible ABI change (gtkbindings.h):
> > > >     -  GtkBindingArg          *args;
> > > >     +  GtkBindingArg          args[1]; /* flexible array */
> > >
> > > Hmm, so people are expected to actually make use of the fact that the
> > > internals of GtkBindingSet are declared in the header ?
> >
> > erm, isn't that the case with everything declared as non-private
> > in our public header files? ;)
> >
> > > What for ?
> >
> > i don't have a good use case at hand, but that doesn't mean it isn't
> > used out there. if we change public structures at will, or want to do
> > so, we definitely should make them private first though.
>
> I think in GTK+, how much private stuff is exposed in headers vs marked
> as private vs not shown in headers at all is pretty much a function of
> the age of the code.

can't say i agree here. i'm generally not on side of the
make-everything-useful-private vendetta that some coders like to angage in.

> Would you as the original author of the binding code agree to label the
> contents of GtkBindingSet, GtkBindingEntry and GtkBindingSignal as
> private ?

erk, hard call.
so generally no, but as i said earlier i don't have a good use case for
third parties using the GtkBinding* structures. and i can see your point
about optimizing the signal arguments. what i think still may make sense
is querying the binding keys which are set up for a class. reading or
modifying the signal and the arguments of such a binding is of no obvious
use to me though, so i guess we could move the definition of GtkBindingSignal
and GtkBindingArg to gtkbinding.c. (or at least try to, and see if it breaks
for anyone with the next releases.)

> > > > 3) the code is actually more memory inefficient now.
> > > >     the main use case are signal bindings with 0 args, which used up
> > > >     a single ->args=NULL (4bytes) pointer in GtkBindingSignal.
> > > >     with your change, sizeof (GtkBindingArg)==12 will be used instead.
> > >
> > > The numbers don't support you here. From running testgtk --bench ALL, I
> > > see:
> > >
> > > 67  binding signals with 0 args
> > > 194 binding signals with 1 arg
> > > 131 binding signals with 2 args
> > > 127 binding signals with 3 args
> >
> > hm, i can't reproduce that with HEAD atm:
> >
> > $ ./testgtk --bench ALL
> > Test                 Iters      First      Other
> > -------------------- ----- ---------- ----------
> > alpha window             1     1905.4
> > [...]
> > cursors                  1      145.7
> > Segmentation fault
>
> Hmm, works for me.
>
> > but your numbers do indeed look like an interesting finding, though
> > it's usually better to look at stats from real apps like gimp or evo.
>
> Here are the numbers for the gimp, after opening the prefs dialog and
> one image window:
>
> 0     40
> 1    149
> 2     94
> 3    169
>
> and here are some numbers for evo:
>
> 0     59
> 1    328
> 2    165
> 3    135

hm, that really is a lot more arg usage than i originally envisioned.
i'll glance at some binding signal arg uses now to see what people
need all those args for ;)


> Matthias

---
ciaoTJ