GNOME Bugzilla – Bug 697209
Individual: connect to Persona::notify less enthusiastically
Last modified: 2013-10-22 07:17:32 UTC
Created attachment 240534 [details] [review] Individual: connect to notify, not to all notify's versions In practice, we care about basically every property anyway, and tests/eds/perf was spending more than 6% of its time connecting to these signals. Just connect to notify and demultiplex through a lookup table for about a 5% speed-up. In principle we could make the lookup table into a hash table if it proves to be worth it, but life is probably too short.
Review of attachment 240534 [details] [review]: Looking good. ::: folks/individual.vala @@ +1155,3 @@ + [CCode (has_target = false)] + private delegate void _UnboundNotifier (Individual self, + Persona persona, ParamSpec ps); Won’t Vala try and copy the ParamSpec when a notifier is called, unless you declare it as ‘unowned’ here, or as an ‘owned’ transfer at call time? Might want to take a look at the C generated for the notifier calls and see if there’s any unnecessary copying/reffing going on. (Although you probably have done already.) @@ +1159,3 @@ + private struct _Notifier + { + weak string property; I think this should be ‘unowned’ rather than ‘weak’. Vala currently treats them identically, but I believe there are plans to support automatic use of weak notifiers for ‘weak’ references in the future. We don’t want that here. @@ +1196,3 @@ + private void _persona_notify_cb (Object obj, ParamSpec ps) + { + assert (obj is Persona); I think you could also assert(obj.individual == this); @@ +1202,3 @@ + if (ps.name == notifier.property) + { + notifier.notify (this, (!) (obj as Persona), ps); You could break out of the loop after this, assuming all entries in _notifiers are unique.
Tweaked and committed. commit dbd75fa4927537eb9723d523d08edb0e11b6ea82 Author: Simon McVittie <simon.mcvittie@collabora.co.uk> Date: Thu Mar 28 14:02:05 2013 +0000 core: Connect to Persona::notify, not all its details, in Individual In practice, we care about basically every property anyway, and tests/eds/perf was spending more than 6% of its time connecting to these signals. Just connect to notify and demultiplex through a lookup table for about a 5% speed-up. In principle we could make the lookup table into a hash table if it proves to be worth it, but life is probably too short. https://bugzilla.gnome.org/show_bug.cgi?id=697209