After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 731829 - gtype: G_VALUE_HOLDS optimisation for case where types are different
gtype: G_VALUE_HOLDS optimisation for case where types are different
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: gobject
2.41.x
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on: 731756 731892
Blocks:
 
 
Reported: 2014-06-18 09:46 UTC by Tim-Philipp Müller
Modified: 2014-06-19 05:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtype: G_VALUE_HOLDS optimisation for case where types are different (1.12 KB, patch)
2014-06-18 09:46 UTC, Tim-Philipp Müller
none Details | Review

Description Tim-Philipp Müller 2014-06-18 09:46:14 UTC
Created attachment 278663 [details] [review]
gtype: G_VALUE_HOLDS optimisation for case where types are different

+++ This bug was initially created as a clone of Bug #731756 +++

G_VALUE_HOLDS is called a fair bit during GStreamer caps negotiation, since GStreamer caps are implemented via field_name + GValue. Almost all of these fields are made up of fundamental types, either GLib's or GStreamer's (ints, strings, booleans, gstreamer lists/arrays/fractions/int ranges/fraction ranges)

The attached patch adds a shortcut to the gcc macro to bail our early with negative result if both types are different fundamental types. This avoids going into g_type_value_holds() for a lot of cases and makes quite a difference for GStreamer's use case (15-20% speed-up of non-artificial capsnego benchmark).
Comment 1 Allison Karlitskaya (desrt) 2014-06-18 14:56:52 UTC
Would it be possible to have a HOLDS_FUNDAMENTAL similar to the instance typechecks we introduced elsewhere, or is the type that you compare against not statically known?

I'm concerned that although we speed up this one case, we introduce another branch everywhere else...
Comment 2 Tim-Philipp Müller 2014-06-18 15:17:57 UTC
The type we compare against is not statically known and might not be a fundamental type. We can work around this locally of course, but I'm not really sure whether that additional branch is really going to make anything worse since it's just before we jump into the rather-expensive-anyway g_type_check_value_holds().
Comment 3 Tim-Philipp Müller 2014-06-18 15:30:28 UTC
Actually, a HOLDS_FUNDAMENTAL might work for us as well. But then the question is what to do with derived types I guess.

We could just rewrite our defines not to use G_VALUE_HOLDS and just make them value && value->type == type.

What might also be extremely beneficial for us would be to nick a part of the constant range between G_TYPE_RESERVED_BSE_FIRST and _LAST for our purposes :) I always wondered if that would be possible..
Comment 4 Allison Karlitskaya (desrt) 2014-06-18 16:52:29 UTC
(In reply to comment #3)
> We could just rewrite our defines not to use G_VALUE_HOLDS and just make them
> value && value->type == type.

Sounds like this is the best idea.  We could introduce a G_VALUE_HOLDS_EXACTLY() but that's really just G_VALUE_TYPE(x) == (y), so we can skip it.


Meanwhile, I find this to be pretty hilarious:

#define G_VALUE_HOLDS_INT(value)	 (G_TYPE_CHECK_VALUE_TYPE ((value), G_TYPE_INT))

..., etc, etc.

we should change all of those to comparing the type directly (since ints are derivable).  New bug for that.