Re: a small patch for gtkstyle.c

[Owen Taylor <otaylor redhat com>]

Sven Neumann <sven gimp org> writes:

> Hi,
> 
> here's a small patch that removes the calculation of the 
> return value from sanitize_size() which was never used
> and inlines this function. OK to commit?
 
Removing the return value is OK. As far as inlining goes,
I'd rather avoid that; unless the function is so small
that inlining reduces code size, or unless we are talking
about tight loops where a lot of time is spent, it's
not clear that it will be a performance win; sure, you
reduce the instruction count, but you also increase
cache misses. (And overhead for loading the code, etc.)

And, if it's a good idea to inline the function the compiler
should be able to figure it out :-)

It doesn't matter here much one way or the other, of course,
but I'd rather avoid spewing 'inline' all over our code
and just add it in specific places where profiling indicates
large amounts of time are being spent.

> I'm also wondering if you'd accept a patch that changes
> places where gdk_draw_line() is called multiple times
> with the same GC into a single call of gdk_draw_segments().
> This would result in the same output (since gdk_draw_line
> is implemented through gdk_draw_segments but probably
> better performance. What do you think?

Well, it wouldn't make for significantly better performance
on X, since Xlib coalesces consecutive XDrawLine calls into
a single request. I suspect that for non-network windowing
systems there also won't be a lot of difference, just
because we aren't talking about all that many lines, and
all you save are stack frames.

Regards,
                                        Owen

> Index: gtk/gtkstyle.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkstyle.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 gtkstyle.c
> --- gtk/gtkstyle.c	2002/01/15 16:35:01	1.98
> +++ gtk/gtkstyle.c	2002/01/27 14:49:25
> @@ -1945,24 +1945,17 @@ gtk_default_render_icon (GtkStyle       
>    return stated;
>  }
>  
> -static gboolean
> +static inline void
>  sanitize_size (GdkWindow *window,
>  	       gint      *width,
>  	       gint      *height)
>  {
> -  gboolean set_bg = FALSE;
> -
>    if ((*width == -1) && (*height == -1))
> -    {
> -      set_bg = GDK_IS_WINDOW (window);
> -      gdk_window_get_size (window, width, height);
> -    }
> +    gdk_window_get_size (window, width, height);
>    else if (*width == -1)
>      gdk_window_get_size (window, width, NULL);
>    else if (*height == -1)
>      gdk_window_get_size (window, NULL, height);
> -
> -  return set_bg;
>  }
>  
>  static void
> _______________________________________________
> gtk-devel-list mailing list
> gtk-devel-list gnome org
> http://mail.gnome.org/mailman/listinfo/gtk-devel-list