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 557047 - g_object_newv() should skip notify:: handling if no handler are connected
g_object_newv() should skip notify:: handling if no handler are connected
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gobject
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2008-10-20 10:37 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2018-05-24 11:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
show that notify is indee called during construction (3.06 KB, text/x-csrc)
2008-10-20 10:38 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
  Details
same example modified to meassure the overhead (3.78 KB, text/x-csrc)
2008-10-20 11:00 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
  Details
remove the notify emission (1.25 KB, patch)
2008-10-20 11:05 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
rejected Details | Review
use g_signal_has_handler_pending to eventualy skip notify queue work (2.62 KB, patch)
2008-10-21 11:58 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
rejected Details | Review
version of meassurement example that has a construct property as well (5.87 KB, text/plain)
2008-10-21 12:00 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
  Details

Description Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 10:37:29 UTC
g_object_newv() currently triggers emmission of the notify signal. This is kind of useless as noone can be subscribed to it yet, as the object is under creation still. As can be seen from bug #536939, the notify handling is not very light.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 10:38:34 UTC
Created attachment 120913 [details]
show that notify is indee called during construction
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 10:39:16 UTC


  • #0 g_object_dispatch_properties_changed
    at gobject.c line 765
  • #1 g_object_notify_dispatcher
    at gobject.c line 312
  • #2 g_object_newv
    at gobjectnotifyqueue.c line 125
  • #3 g_object_new_valist
    at gobject.c line 1315
  • #4 g_object_new
    at gobject.c line 1056
  • #5 main
    at gobjectnotify.c line 116

Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 11:00:07 UTC
Created attachment 120916 [details]
same example modified to meassure the overhead

# before
3.05user 0.02system 0:03.11elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
3.14user 0.01system 0:03.23elapsed 97%CPU (0avgtext+0avgdata 0maxresident)k
3.04user 0.00system 0:03.09elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
3.05user 0.00system 0:03.08elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k

# after
2.55user 0.00system 0:02.58elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
2.60user 0.00system 0:02.62elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
2.58user 0.00system 0:02.59elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
2.56user 0.01system 0:02.59elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 11:05:32 UTC
Created attachment 120917 [details] [review]
remove the notify emission
Comment 5 Christian Persch 2008-10-20 12:38:30 UTC
(In reply to comment #0)
> g_object_newv() currently triggers emmission of the notify signal. This is kind
> of useless as noone can be subscribed to it yet,

The GObject class notify handler (G_OBJECT_CLASS(myclass)->notify) _is_ subscribed.
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 12:44:22 UTC
What I meant is that g_signal_connect() requires an instance which is not yet available outside g_object_new(). Do you have a (theoretical) example in mind how the notify could be used during construction?
Comment 7 Tim Janik 2008-10-20 12:47:33 UTC
(In reply to comment #5)
> (In reply to comment #0)
> > g_object_newv() currently triggers emmission of the notify signal. This is kind
> > of useless as noone can be subscribed to it yet,
> 
> The GObject class notify handler (G_OBJECT_CLASS(myclass)->notify) _is_
> subscribed.

Right, also the following case is not unavoidable:
  parent_init () { container_add (parent, object_new (child_type)); }
  child_paret_set() { signal_connect (parent, "notify", prophandler); }
If we skip emissions for properties changing in parent_init() or from construct properties at the end of object_newv, prophandler might be missing out on calls that the child implementation could validly expect.
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 12:57:52 UTC
In the parent-child only non-construct properties would be notified, right? Should probably write a test for that too.

I think the one definite good outcome of this is that I can better document the contract :)
Comment 9 Tim Janik 2008-10-20 13:01:39 UTC
(In reply to comment #8)
> In the parent-child only non-construct properties would be notified, right?

No, also normal properties, example:
parent_init (self)
{
  container_add (parent, object_new (child_type));
  g_object_set (self, "can_focus", TRUE, NULL);
}

The child's prophandler should be seeing a parent::can-focus notification at the end of g_object_newv() here.

> Should probably write a test for that too.

That'd be good to have.
Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 13:06:54 UTC
Your right, also just found the gobject constructor going the freeze/thaw for the construct properties. I'll see if the mechanism itself can be optimized then.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-20 14:24:50 UTC
In my testcase I have actually nothing subscribed to the notify signal. If I can make it 20% faster by removing the notify code, I should actually be able to also bail out by having checks like signal_check_skip_emission() for notify. Now that is unfortunately local to gsignal.c ...
Comment 12 Tim Janik 2008-10-20 14:46:10 UTC
(In reply to comment #11)
> In my testcase I have actually nothing subscribed to the notify signal. If I
> can make it 20% faster by removing the notify code, I should actually be able
> to also bail out by having checks like signal_check_skip_emission() for notify.
> Now that is unfortunately local to gsignal.c ...

g_signal_has_handler_pending() is an external approximation of signal_check_skip_emission(). Exposing the latter as API would unfortunately also be moderately expensive (it'd e.g. need to acquire a mutex, which trashes caches and pipelines).
Comment 13 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-21 11:58:28 UTC
Created attachment 121004 [details] [review]
use g_signal_has_handler_pending to eventualy skip notify queue work

This patch has two defines to tests how much difference the changes bring. If thats the way to go it should also be tried for regular g_object_set and g_object_notify api.

Regarding g_signal_has_handler_pending() it passes G_SIGNAL_MATCH_DETAIL unconditionally, where it should probably be (may_be_detail ? 0 : G_SIGNAL_MATCH_DETAIL).
Comment 14 Stefan Sauer (gstreamer, gtkdoc dev) 2008-10-21 12:00:15 UTC
Created attachment 121005 [details]
version of meassurement example that has a construct property as well

Its interesting to notice that time almost double once one has a construct property.
Comment 15 Tim Janik 2008-12-03 11:15:10 UTC
(In reply to comment #13)
> Created an attachment (id=121004) [edit]
> use g_signal_has_handler_pending to eventualy skip notify queue work
> 
> This patch has two defines to tests how much difference the changes bring. If
> thats the way to go it should also be tried for regular g_object_set and
> g_object_notify api.
> 
> Regarding g_signal_has_handler_pending() it passes G_SIGNAL_MATCH_DETAIL
> unconditionally, where it should probably be (may_be_detail ? 0 :
> G_SIGNAL_MATCH_DETAIL).

One problem with your approach of supressing notifies already during the queueing phase is that this still breaks compatibility with classes that currently override GObejctClass.dispatch_properties_changed. Just supressing unneeded emissions in g_object_dispatch_properties_changed() could be done compatibly though. Not sure if that significantly improves performance though.
Comment 16 Stefan Sauer (gstreamer, gtkdoc dev) 2009-01-03 17:11:26 UTC
Or I could check if GObejctClass.dispatch_properties_changed() is not overridden. 

Idealy we would have another vmethod GObejctClass.notify_needed(). The default implementation would check if GObejctClass.dispatch_properties_changed() is not overridden and then check g_signal_has_handler_pending().

If someone overrides GObejctClass.dispatch_properties_changed(), they could also override GObejctClass.notify_needed(), if they can do a simillar check.

Does that makes sense?
Comment 17 GNOME Infrastructure Team 2018-05-24 11:34:34 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/165.