GNOME Bugzilla – Bug 682809
Lazy instantiation of multi-valued properties
Last modified: 2012-08-28 21:04:56 UTC
Branch coming up with some optimisations to lazily instantiate multi-valued properties (i.e. Sets and Maps of things) to reduce memory usage and speed up time-to-quiescence. See the commit messages in the branch for more details.
Created attachment 222561 [details] [review] Optimise multi-valued property initialisation https://www.gitorious.org/folks/folks/commits/properties-optimisation
From some quick tests, it halves the time-to-quiescence for folks-inspect, reduces RSS usage by ~1MB and halves the number of GObjects being created (all for a setup using EDS and Telepathy with ~115 personas in total). Note that these figures depend on the patch in bug #682803 also being applied to libgee.
I'm getting some aborts when I run folks-inspect individuals here. The error message I get is telepathy:ERROR:/home/jpwhiting/devel/collabora/src/gnome/folks/backends/telepathy/lib/tpf-persona.vala:864:_tpf_persona_contact_notify_contact_info: assertion failed: ((this._email_addresses == null) == (this._phone_numbers == null) == (this._urls == null)) Aborted Maybe we need to remove/change some assertions to be g_return_if_fail?
Added some warning lines there and it is aborting when all three of those are non-null which doesn't match the logic I read there in the vala assert statement and in the generated c statement. It should equate to assert(false == false == false) right?
Comment on attachment 222561 [details] [review] Optimise multi-valued property initialisation Committed with a NEWS entry and a fix for the problem mentioned in comment #4. commit af4bb7b8660cddc8c0b377a3bab72f93d9e3e5b3 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 13:27:22 2012 +0100 core: Tidy up multi-valued property update functions Create a new _update_multi_valued_property() helper function and corresponding delegates, to mirror the existing _update_single_valued_property() helper. This new function is now used for most multi-valued property updaters in Individual. It’s not used for groups, because they have weird signalling. folks/individual.vala | 659 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------ 1 file changed, 339 insertions(+), 320 deletions(-) commit eb75447e8eb6a95aad8f95ee19a2cd4a945fefa1 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 10:13:53 2012 +0100 telepathy: Support lazy initialisation of properties See commit 303547fec56e416f57f73643e2afb7bb4e4a8a7f. This adds support for lazy initialisation of multi-valued properties to the Telepathy backend. backends/telepathy/lib/tpf-persona.vala | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 25 deletions(-) commit 57f84d50f5fcaf3bf821d368a7ff309bc2edb3d2 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 10:13:19 2012 +0100 eds: Support lazy initialisation of properties See commit 303547fec56e416f57f73643e2afb7bb4e4a8a7f. This adds support for lazy initialisation of multi-valued properties to the EDS backend. backends/eds/lib/edsf-persona.vala | 275 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------- 1 file changed, 198 insertions(+), 77 deletions(-) commit 4a59af5c74f84932c8da90f6ac996a4a2f69e17c Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 10:05:35 2012 +0100 core: Support lazy initialisation of properties Creating several HashSets and HashMultiMaps for every Individual turns out to be quite wasteful, especially when most of them will typically be empty (as address books are generally quite sparse on properties) and never accessed (since most clients don’t need local IDs or web service addresses). This commit delays initialisation of various multi-valued Individual properties to the first time they’re accessed, giving them a null value before that time. It preserves the existing API for Individual, i.e. the properties themselves remain non-nullable, and only the object members backing them become nullable. This does introduce a slight behaviour change, in that an Individual will now emit change notifications for a non-initialised multi-valued property if *any* of the Personas in the Individual emit a notification for that property. This is because the Individual can’t compare the current value of its property to the new one resulting from the change in the Persona’s property value to determine if a change has really occurred. Therefore the Individual makes a safe over-estimate and emits notifications which might be false positives. This shouldn’t be a problem: if a client is interested in the property, they will have already queried it and caused it to be initialised. Initialised properties have the same notification behaviour as before. If a client isn’t interested in the property, it won’t be connected to the property notifications anyway. This change roughly quarters the number of GObjects being created when opening folks-inspect with the Telepathy, key-file and EDS backends enabled and ~115 personas in the system. NEWS | 1 + folks/individual.vala | 385 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------- 2 files changed, 290 insertions(+), 96 deletions(-) commit 8632764f450971f1c83e5b209735c4cf50edd7f8 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 10:05:14 2012 +0100 core: Expanding a profiling message slightly folks/persona-store.vala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) commit 3e48d450d35b8c21b407dc481b74b0983507bf1c Author: Philip Withnall <philip@tecnocode.co.uk> Date: Mon Aug 27 09:59:55 2012 +0100 eds: Batch up emissions of personas-changed while starting up There’s no point in emitting personas-changed multiple times before the EDS persona store has reached quiescence — it just means the individual aggregator has to re-link more personas with each emission. This patch batches up emissions of personas-changed from the EDS persona store until is-quiescent is reached, then notifies about all personas in a single emission. backends/eds/lib/edsf-persona-store.vala | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) commit 5df5c3f4cacf16c67c4b1493a3008b418ae98b2c Author: Philip Withnall <philip@tecnocode.co.uk> Date: Sun Aug 26 23:43:26 2012 +0100 eds: Remove unnecessary locking It didn’t achieve anything. backends/eds/lib/edsf-persona-store.vala | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)