GNOME Bugzilla – Bug 322174
Don't use G_TYPE_INSTANCE_GET_PRIVATE in PangoFcFont
Last modified: 2005-11-22 22:25:19 UTC
The attached screenshots from sysprof show two things: 1. PangoFcFont uses G_TYPE_INSTANCE_GET_PRIVATE(). In particular, it gets used in pango_fc_font_get_glyph(), which is a time-critical function. We spend 8.37% of the pango-language-benchmark time in that function, and 3.35% is getting the private data. 2. pango_fc_font_get_glyph() has a type check in a g_return_val_if_fail(); that one takes 2.13% of the total time. To fix (1), I looked for a field to replace with a "priv" in the PangoFcFont structure. That structure is private, so I can't change any field. There's a "gpointer context_key" field, used only internally, so I replaced it with "gpointer priv". I added internal accessors for the context_key, and used them elsewhere in Pango. The fix for (2) is trivial; just take out that g_return_val_if_fail().
Created attachment 55106 [details] Profile for pango_fc_font_get_glyph()
Created attachment 55107 [details] Profile after optimization. Notice how the time went from 8.37% to 1.31%
Created attachment 55108 [details] [review] pango-fcfont-no-gobject-private-data.diff
Ahem, PangoFcFont is *public*, so I can't change its size. Sorry for the thinko above.
In HEAD now. 2005-11-22 Federico Mena Quintero <federico@ximian.com> Fixes #322174: * pango/pangofc-font.h (struct _PangoFcFont): Replace the "gpointer context_key" field with "gpointer priv". This way we can access the private data quickly, instead of using g_type_instance_get_private(). * pango/pangofc-private.h: Added prototypes for _pango_fc_font_{get,set}_context_key(). * pango/pangofc-font.c (struct _PangoFcFontPrivate): Moved the "context_key" field to here. (PANGO_FC_FONT_GET_PRIVATE): Use the "priv" field instead of GType private data. (pango_fc_font_class_init): Don't register GType private data. (pango_fc_font_init): Initialize the private data here. (pango_fc_font_finalize): Free the private data. (_pango_fc_font_get_context_key): Implement. (_pango_fc_font_set_context_key): Implement. (pango_fc_font_get_glyph): Remove the g_return_val_if_fail(); it was appearing quite high in the profile. * pango/pangofc-fontmap.c (pango_fc_font_map_add): Call _pango_fc_font_set_context_key() instead of setting the fcfont->context_key directly. (_pango_fc_font_map_remove): Likewise; also use _pango_fc_font_get_context_key() instead of accessing the field directly.
- g_type_class_add_private (object_class, sizeof (PangoFcFontPrivate)); [...] + priv = g_new0 (PangoFcFontPrivate, 1); + fcfont->priv = priv; That looks wrong to me; just because you don't want to use G_TYPE_INSTANCE_GET_PRIVATE everywhere to get the priv pointer doesn't mean you cannot use g_type_class_add_private... ; you just use G_TYPE_INSTANCE_GET_PRIVATE in pango_fc_font_init to set the instance->priv pointer and access instance->priv directly elsewhere.
g_type_class_add_private() tells the type system to allocate the private structure somewhere. Then, in ::init(), I would have to get a pointer to the structure and stuff it in my ->priv. By doing it with g_new(), I have less code, and less places that deal with creating the private data. This all makes the code clearer.
Good point Christian. That's how it's done in other places in Pango too. 2005-11-22 Behdad Esfahbod <behdad@gnome.org> * pango/pangofc-font.c: Finish previous patch. Use GType private data, cache it into fcfont->priv. Get rid of PANGO_FC_FONT_GET_PRIVATE (fcfont) and access fcfont->priv. (#322174, Christian Persch)