GNOME Bugzilla – Bug 567256
type check+cast optimization
Last modified: 2011-05-01 21:25:08 UTC
Lots of times you have to do something like: if (GTK_IS_FROBNICANT (bloop)) { GtkFrobnicant *frob = GTK_FROBNICANT (bloop); but as currently written, this will (generally) result in the evil slow typechecking machinery running twice. Since (a) g_type_instance_check_cast(ip, gt) is a no-op if g_type_instance_check_is_a(ip, gt) returns TRUE (b) g_type_check_instance_is_a() is G_GNUC_PURE (c) most *_get_type() functions (and thus *_TYPE_* macros) are G_GNUC_CONST we can tweak the cast macros to call _is_a() first, and only call _check_cast() if it fails (just to spew the warning). In the case above, when compiled with optimization, this would mean the GTK_FROBNICANT() cast would optimize to a no-op. In the case of a cast without a preceding check, the change would result in a call to g_type_instance_check_cast() being turned into a call to g_type_instance_check_is_a(), which is basically the same thing. (Actually, #ifdef __GNUC__, some calls to g_type_instance_check_cast() would be optimized out entirely.) The only place the new patch would be slower than the existing code would be when making an incorrect cast, in which case the object would be typechecked twice now (once via _is_a and again via _check_cast). But since this indicates a bug in the program anyway, who cares. The optimization does depend on the _get_type() function being marked G_GNUC_CONST, which IIRC is slightly controversial?
Created attachment 126172 [details] [review] patch to optimize typecheck followed by cast
> The optimization does depend on the _get_type() function being marked > G_GNUC_CONST, which IIRC is slightly controversial? What we would really like is to mark them as idempotent, but gcc doesn't let us atm.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32911
(In reply to comment #0) > The optimization does depend on the _get_type() function being marked > G_GNUC_CONST, which IIRC is slightly controversial? I should clarify that it doesn't do anything *wrong* (or slower-than-before) if the get_type() method isn't G_GNUC_CONST, it just doesn't manage to optimize it in that case. (Also, some related discussion in http://mail.gnome.org/archives/gtk-devel-list/2010-March/msg00127.html.)
I forgot to mention that I applied your patch, and found that it breaks various gobject and gio tests. It still did that after I converted it to an ({}) to avoid double-evaluation.
Comment on attachment 126172 [details] [review] patch to optimize typecheck followed by cast oh. huh.
Created attachment 186996 [details] [review] gtype: optimize typecheck macros a bit for a common case Take advantage of the fact that g_type_check_instance_is_a() is marked G_GNUC_PURE and rewrite G_TYPE_CHECK_INSTANCE_CAST() to use that rather than actually calling g_type_check_instance_cast() when possible. This way, code like this: if (GTK_IS_CONTAINER (widget)) { GtkContainer *container = GTK_CONTAINER (widget); will only end up doing a single type check when compiled with optimization (assuming that gtk_container_get_type() is marked G_GNUC_CONST, which it is).
attached a new version of the patch that compiles, passes make check, ... and doesn't actually optimize anything. I'm pretty sure it had worked when I tried it before, so I'm not sure what happened; maybe gcc changed its rules a little bit about how pure/const/etc functions can be optimized.