Created attachment 120913 [details]
show that notify is indee called during construction

+ Trace 208414

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

Created attachment 120917 [details] [review]
remove the notify emission

(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.

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?

(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.

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 :)

(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.

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.

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 ...

(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).

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).

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.

(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.

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?

-- 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.