Re: questioning glib r5316 by mclasen

[Behdad Esfahbod <behdad behdad org>]

On Mon, 2007-05-14 at 06:49 -0400, Tim Janik wrote:
> hi Matthias.
> 
> can you please explain your glib change from 2007-01-26 where you're making
> function-scoped symbols non-static without any obvious need?
> 
> i'm sorry to have to bring this up, but your ChangeLog entry is very
> uninformative on this matter:
>    +	* gutils.c: Make some structs which are used only once
>    +	non-static. 
> because it uselessly states *what* was done (the diff can tell that
> already), instead of describing *why* the change was done.
> 
> thanks for consideration.

Hi Tim,

I'll give a shot at explaining this if you don't mind.

A const struct when defined static will hopefully end up in the
readonly .data section of the binary and process and everything will be
perfect.  However, if the struct contains pointer arguments (function or
data) that are initialized to some other symbol, it will need link-time
relocation and hence cannot live in the readonly data segment anymore,
and will be moved to regular, writable, .data.  The effect is that its
memory will not be shared across processes.

Now if you make that struct non-static, it will be allocated off the
stack and initialized at run-time, and disposed when the stack frame
goes out of scope.  No per-process penalty.

In the cases that Matthias committed, the functions in question were
only run once during a process's lifecycle, so there is not much runtime
penalty being paid, in return for less link-time overhead and less
per-process memory use.


Regards,

behdad


> ---
> ciaoTJ
> 
> Index: /usr/src/gtk+head/glib/glib/gmessages.c
> ===================================================================
> --- /usr/src/gtk+head/glib/glib/gmessages.c	(revision 5315)
> +++ /usr/src/gtk+head/glib/glib/gmessages.c	(revision 5316)
> @@ -144,7 +144,7 @@
> 
>         if (val)
>   	{
> -	  static const GDebugKey keys[] = {
> +	  const GDebugKey keys[] = {
>   	    { "error", G_LOG_LEVEL_ERROR },
>   	    { "critical", G_LOG_LEVEL_CRITICAL },
>   	    { "warning", G_LOG_LEVEL_WARNING },
> @@ -1062,7 +1062,7 @@
>     val = g_getenv ("G_DEBUG");
>     if (val != NULL)
>       {
> -      static const GDebugKey keys[] = {
> +      const GDebugKey keys[] = {
>   	{"fatal_warnings", G_DEBUG_FATAL_WARNINGS},
>   	{"fatal_criticals", G_DEBUG_FATAL_CRITICALS}
>         };
> Index: /usr/src/gtk+head/glib/glib/gslice.c
> ===================================================================
> --- /usr/src/gtk+head/glib/glib/gslice.c	(revision 5315)
> +++ /usr/src/gtk+head/glib/glib/gslice.c	(revision 5316)
> @@ -277,7 +277,7 @@
>     /* don't use g_malloc/g_message here */
>     gchar buffer[1024];
>     const gchar *val = _g_getenv_nomalloc ("G_SLICE", buffer);
> -  static const GDebugKey keys[] = {
> +  const GDebugKey keys[] = {
>       { "always-malloc", 1 << 0 },
>       { "debug-blocks",  1 << 1 },
>     };
> Index: /usr/src/gtk+head/glib/glib/gmem.c
> ===================================================================
> --- /usr/src/gtk+head/glib/glib/gmem.c	(revision 5315)
> +++ /usr/src/gtk+head/glib/glib/gmem.c	(revision 5316)
> @@ -689,7 +689,7 @@
>   {
>     gchar buffer[1024];
>     const gchar *val;
> -  static const GDebugKey keys[] = {
> +  const GDebugKey keys[] = {
>       { "gc-friendly", 1 },
>     };
>     gint flags;
> Index: /usr/src/gtk+head/glib/ChangeLog
> ===================================================================
> --- /usr/src/gtk+head/glib/ChangeLog	(revision 5315)
> +++ /usr/src/gtk+head/glib/ChangeLog	(revision 5316)
> @@ -1,3 +1,11 @@
> +2007-01-26  Matthias Clasen <mclasen redhat com>
> +
> +	* gmem.c:
> +	* gslice.c:
> +	* gmessages.c:
> +	* gutils.c: Make some structs which are used only once 
> +	non-static. 
> +
>   2007-01-24  Benjamin Otte <otte gnome org>
> 
>   	* glib/gprintf.c (g_sprintf): Clarify the documentation
> _______________________________________________
> gtk-devel-list mailing list
> gtk-devel-list gnome org
> http://mail.gnome.org/mailman/listinfo/gtk-devel-list
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759