GNOME Bugzilla – Bug 536939
g_object_{set,get} has quite some overhead
Last modified: 2017-10-11 09:16:29 UTC
libraries that offer g_object_{set,get} as a main interface to acces gobject properties to keep the library size small, get quite a performance penalty for doing so. g_object_set is twice as slow as setting something directly and calling g_object_notify g_object_get is around 15 times slower as returning the value directly. pros-and-cons for g_object_{set/get}: + no extra functions > nm --defined-only ~/debug/lib/libgtk-x11-2.0.so | grep "_set_" | wc -l 2007 > nm --defined-only ~/debug/lib/libgtk-x11-2.0.so | grep "_get_" | wc -l 2949 > nm --defined-only ~/debug/lib/libgtk-x11-2.0.so | wc -l 20036 1/4 of libgtk api size could be saved! (yes, this has to be checked more detailed) + process multiple entries in one go - slow - not typesafe
Created attachment 112257 [details] test applications
Someone performed a simple study of that a few weeks ago: https://guij.emont.org/blog/2008/04/14/how-much-does-a-g_object_set-cost/
Created attachment 112961 [details] test application If someone wants to repeatedly set or get a property, e.g. while doing animations so there is some overhead that traditional optimization would address - moving constant calculations out of the loop. Constant work in the case at hand is to lookup the paramspec from the name. The attached 2nd version of the test uses a hack, namely accessing the private field pspec->param_id in order to call the klass->(s,g)et_property function directly. A can be seen fro the examples, that gets us almost to the same level like accessing the fields directly. Question: what about adding official api to retrieve the param_id like e.g. : guint g_param_spec_id(pspec); Remaining slowdown is caused by g_object_notify(). Dunno if this function could be made smart to quickly return if there are no handlers connected. Or if one could pass the pspec to avoid the lookup.
Before someone complains. The code in the "direct" part asumes right now that the property is owned by the example object there. It would not work for inherited properties. It can be easily expanded to support that too (iterating with g_type_class_peek_parent() and trying g_object_class_find_property() there).
Created attachment 112972 [details] test application Implements comment #4. Also directly emits the notify signal in the "direct" case. This is surprisingly much faster than calling g_object_notity(). There is a -DSKIP_MON for compiling now too, if not given the example adds one signal handler to monitor property changes (unrelated, but I wonder why for all 4 cases, the notify is called twice as often as the loop runs?)
Created attachment 112973 [details] test application forget the last question. The notify is called the correct number of times. The output was wrong.
I see one very fundamental problm with this approach. The GObject property system provides several guarantees for the class set_property/get_property implementations (such as value types matching pspecs for instance). Exposing the param ids *and* advocating that people call those klass functions directly breaks the current interface contract completely. That's why I'm opposing param_id exposition and advising people of API misuse. If there's a use case for speedier set/get property implementations, the following things can be investigated: - are there easy speed ups in the current code paths that preserve API/ABI semantics? - are there less convenient interfaces that gobject.h could offer which provide faster property access, however preserve API semantics?
Tim, thanks for the review. Your right about the interface contract regaring the gvalues. - As the tests point out the major overhead comes from g_object_notify() internals. So that could be optimized. - A second ide would be a more specific API for this use case. To recap the use-case is to 'animate' a gobject property. The key points are, its always the same property. So we can reuse the gvalue and like to avoid name->property-id and name->notify-signal lookups.
One problem I see is that there is a global GParamSpecPool in gobject.c. This makes lookup slower and causes lock contention as the pool is protected with one mutex (see gparam.c). Shouldn't there be a pool per class?
GobjectClass stil has 7 pointers for extension, we'd need one here. All the code that uses the global GParamSpecPool *pspec_pool is in gobject.c. The code either - has the class and thus can do class->pspec_pool - it receives the type and would need to do class = g_type_class_peek(type); - it receives the instance and would need to do class = G_OBJECT_GET_CLASS(self); Anyone one obvious oversight in my previous comment is accesing the GParamSpecPool of an instance needs to chain-up. So we would need a tree where we can look branches. GObject |- GtkWidget | |- GtkButton | `- GtkXYZ |- GstObject | |- GstElement | `- GstBus When accessing GstElement::name, we'd like to only search GstElement -> GstObject -> GObject and also only lock as we go. Should we do that with a specific data structure or in programatically? In the above example g_param_spec_pool_* would try the pool of GstElementClass, if it failed chainup via g_type_class_peek_parent() and try again. Until operation succeeds or there is no parent classed type.
The implementation should probably have reffred GParamSpecs of the parent in its pool. That could also fix the problem with GParamSpecOverride.
now that GType has private class data support a per-class GParamSpecPool would be a good candidate to start using it.
(In reply to comment #3) > Remaining slowdown is caused by g_object_notify(). Dunno if this function could > be made smart to quickly return if there are no handlers connected. Or if one > could pass the pspec to avoid the lookup. see bug 615425 for g_object_notify_by_pspec() which avoids the lookup.
Ryan moved back the notification queue inside gobject, speeding up the queue operations: commit 877c0ad5b885f598006f576a5756dd7cda1ef4ee Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:46:19 2011 +0000 [notify] remove some rather bogus 'inline' use commit ac0ddcf23f4f57efc2f4e6f370022586d7897f6a Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 13:02:23 2011 +0000 [notify] dispatch 'notify' directly if not frozen commit 39458748dd32c8745f60e8ba83bd35ad9ceebb12 Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:42:36 2011 +0000 [notify] add 'conditional' to _notify_queue_freeze commit 393d4c28b4af6aea45100edc636c116d38dca460 Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:41:06 2011 +0000 [notify] Remove GObjectNotifyContext indirection commit 8215fc5f255ae499904534042ac08e221c4f5f13 Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:38:25 2011 +0000 [notify] lift some logic out of _notify_queue_add commit 760037ec46bb3a8af8ea98e06a624a3ebac1cb1a Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:37:17 2011 +0000 [notify] remove an obviously false comment commit 45d80cf9bdd4ee3edaa64069dd16ad220d615c4e Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:36:57 2011 +0000 [notify] lift some code outside of critical region commit 1d98f931946c45301531a576107f5f9d12d63c28 Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:35:58 2011 +0000 [notify] drop some unused code commit 128862eafe49b5454c4b32b0866800d4aa7c53f3 Author: Ryan Lortie <desrt@desrt.ca> Date: Wed Nov 16 15:31:58 2011 +0000 [notify] merge gobjectnotifyqueue.c into gobject.c there's also bug 661140 for signal emission optimization.
finally, there's the gproperty work in bug 648526 which should reduce the amount of time we spend in boxing and unboxing GValues.
Just to give some update date results: with glib 2.40.2 (ubuntu 14.04 lts) ./gobjectprops doing 4194302 loops set/get : time spent 11.412968 (4194302 change notification) set/get_property : time spent 10.762123 (4194302 change notification) api : time spent 6.841566 (4194302 change notification) direct : time spent 5.297245 (4194302 change notification) with glib 2.43.2 (jhbuild) ./gobjectprops doing 4194302 loops set/get : time spent 5.032278 (4194302 change notification) set/get_property : time spent 4.840398 (4194302 change notification) api : time spent 3.004432 (4194302 change notification) direct : time spent 2.329232 (4194302 change notification) Not sure what has changed, but that's sweet!
Since there’s been no activity here for a while, and significant speedups have been measured by this bug, let’s close it. If someone comes along with another idea about how to speed up {get,set}_property(), they can open a new bug.