Re: PangoXft and PangoFT2 patch from hell

[Tim Janik <timj gtk org>]

On 18 Nov 2001, Owen Taylor wrote:

> Tim Janik <timj gtk org> writes:
> 

> But yes, it is also about type node lookup overhead, and the
> fact that g_object_newv() does an awful lot of function calls.
> Would it be useful to special case g_object_newv() for objects
> without any parameters? It might be worth timing if we ever
> run out of more obvious things to optimize. 

i'll let you decide on this, lets see how we'd benefit from this:


> gpointer
> g_object_newv (GType       object_type,
> 	       guint       n_parameters,
> 	       GParameter *parameters)
> {
>   GObjectConstructParam *cparams, *oparams;
>   GObjectNotifyQueue *nqueue;
>   GObject *object;
>   GObjectClass *class;
>   GSList *slist;
>   guint n_total_cparams = 0, n_cparams = 0, n_oparams = 0, n_cvalues;
>   GValue *cvalues;
>   GList *clist = NULL;
>   guint i;
> 
>   g_return_val_if_fail (G_TYPE_IS_OBJECT (object_type), NULL);
> 
>   class = g_type_class_ref (object_type);
>   for (slist = class->construct_properties; slist; slist = slist->next)

this is not triggered if you don't have construct properties.

>     {
>       clist = g_list_prepend (clist, slist->data);
>       n_total_cparams += 1;
>     }
> 
>   /* collect parameters, sort into construction and normal ones */
>   oparams = g_new (GObjectConstructParam, n_parameters);

this is a g_malloc(0) (which immediately returns for this case) if
you don't pass in parameters.

>   cparams = g_new (GObjectConstructParam, n_total_cparams);

this is a g_malloc(0) if you don't have construct properties.

>   for (i = 0; i < n_parameters; i++)

this loop is not triggered if you don't pass in parameters.

>     {
>       GValue *value = &parameters[i].value;
>       GParamSpec *pspec = g_param_spec_pool_lookup (pspec_pool,
> 						    parameters[i].name,
> 						    object_type,
> 						    TRUE);
>       if (!pspec)
> 	{
> 	  g_warning ("%s: object class `%s' has no property named `%s'",
> 		     G_STRLOC,
> 		     g_type_name (object_type),
> 		     parameters[i].name);
> 	  continue;
> 	}
>       if (!(pspec->flags & G_PARAM_WRITABLE))
> 	{
> 	  g_warning ("%s: property `%s' of object class `%s' is not writable",
> 		     G_STRLOC,
> 		     pspec->name,
> 		     g_type_name (object_type));
> 	  continue;
> 	}
>       if (pspec->flags & (G_PARAM_CONSTRUCT | G_PARAM_CONSTRUCT_ONLY))
> 	{
> 	  GList *list = g_list_find (clist, pspec);
> 
> 	  if (!list)
> 	    {
> 	      g_warning (G_STRLOC ": construct property \"%s\" for object `%s' can't be set twice",
> 			 pspec->name, g_type_name (object_type));
> 	      continue;
> 	    }
> 	  cparams[n_cparams].pspec = pspec;
> 	  cparams[n_cparams].value = value;
> 	  n_cparams++;
> 	  if (!list->prev)
> 	    clist = list->next;
> 	  else
> 	    list->prev->next = list->next;
> 	  if (list->next)
> 	    list->next->prev = list->prev;
> 	  g_list_free_1 (list);
> 	}
>       else
> 	{
> 	  oparams[n_oparams].pspec = pspec;
> 	  oparams[n_oparams].value = value;
> 	  n_oparams++;
> 	}
>     }
> 
>   /* set remaining construction properties to default values */
>   n_cvalues = n_total_cparams - n_cparams;
>   cvalues = g_new (GValue, n_cvalues);

this is a g_malloc(0) if you don't have construct properties.

>   while (clist)
>     {
>       GList *tmp = clist->next;
>       GParamSpec *pspec = clist->data;
>       GValue *value = cvalues + n_total_cparams - n_cparams - 1;
> 
>       value->g_type = 0;
>       g_value_init (value, G_PARAM_SPEC_VALUE_TYPE (pspec));
>       g_param_value_set_default (pspec, value);
> 
>       cparams[n_cparams].pspec = pspec;
>       cparams[n_cparams].value = value;
>       n_cparams++;
> 
>       g_list_free_1 (clist);
>       clist = tmp;
>     }
> 
>   /* construct object from construction parameters */
>   object = class->constructor (object_type, n_total_cparams, cparams);

this is obviously always required.

>   /* free construction values */
>   g_free (cparams);
>   while (n_cvalues--)
>     g_value_unset (cvalues + n_cvalues);
>   g_free (cvalues);

this loop is not triggered if you don't have construct properties.
so we end up with g_free(0); g_free(0);

>   /* release g_object_init() notification queue freeze_count */
>   nqueue = g_object_notify_queue_freeze (object, &property_notify_context);
>   g_object_notify_queue_thaw (object, nqueue);
>   
>   /* set remaining properties */
>   for (i = 0; i < n_oparams; i++)
>     object_set_property (object, oparams[i].pspec, oparams[i].value, nqueue);
>   g_free (oparams);
> 
>   g_type_class_unref (class);
> 
>   /* release our own freeze count and handle notifications */
>   g_object_notify_queue_thaw (object, nqueue);

the loop again is not triggered without parameters, and we have one g_free(0),
freezing/thawing the notify queue does involve g_new() and qdata access, but it's
required in any case, since the queue needs to be unthawed from a previous freeze
issued in g_object_init.

>   
>   return object;
> }


so, what we can get rid of are 3 times g_malloc(0) and three times g_free(0).
just for reference they look like:

gpointer
g_malloc (gulong n_bytes)
{
  if (n_bytes)
    {
      [...]
    }

  return NULL;
}
void
g_free (gpointer mem)
{
  if (mem)
    glib_mem_vtable.free (mem);
}

so, what's your call?

> 
> Regards,
>                                         Owen

---
ciaoTJ