Xfconf signal bindings branch

Nick Schermer nickschermer at gmail.com
Sun Sep 13 22:03:09 CEST 2009


2009/9/13 Brian J. Tarricone <brian at tarricone.org>:
> On 09/13/2009 09:46 AM, Nick Schermer wrote:
>> It also uses 1 internal list for holding all the
>> bindings, which makes it easier to understand (and probably as fast as
>> the combination of the hash table and lists set on the objects).
>
> I don't like "probably."  Have you checked this?  The current method
> uses one list per object.  It's reasonable (sorta) to expect that each
> per-object list will be shorter than a single list for all bindings.
> Does this scale when adding large numbers of bindings?  I'm concerned
> about app startup when an app adds all its bindings.

Startup if definitely faster, since we only prepend to the __bindings
list (ie. faster then add to hash-table and update object list, since
we basically only do the latter without the g_object_get/set_property
stuff). Also no hash table creation on xfconf_init(), esp. for apps
not using the bindings.

Removing a binding by id is slower, because a hash table is faster in
this kinda stuff, but how often is this done? I don't even think we
use unbind by id somewhere right now, in combination with the amount
of connected bindings. Pretty sure this is not noticeable. Basically
the same for unbind by property.

The 'slowest' part is when you close your application, then all the
objects/channels are destroyed and the bindings are disconnected. Then
each binding removes it self from the list. Still very fast since the
list is getting smaller quickly and we directly use g_slist_find.
So _maybe_ slower, but it also drops the hash table destroy and the
glist get/set from the object.

> Anyway, that doesn't matter.  Since you're not doing anything fancy with
> the list, just use a GHashTable or GTree.  The most common operations
> are search and insert (and possibly delete, depending on the app); pick
> the best data structure for that.

I think we should stick with the list (for now at least), it is by far
the simplest way, it easy and fast enough (basically only pointer
comparison). If one application will feel the slowdown its going to be
the panel, so then I'll start a new nick/### branch ;-).

> I'd lean toward GTree since you
> iterate over all items in _shutdown(), and a hash table isn't really the
> greatest for iteration, but it doesn't matter all that much since it's a
> one-time action and you expect the list to be empty.

Like i said, _object_disconnect is the slowest part, shutdown will (if
you released all channels before calling xfconf_shutdown, and you
should do that therefore the warning, see xfdesktop-settings), never
iterate the list, just a tool to make sure nothing is left when xfconf
is closed.

> Coding style comments interspersed in the patch below.  I haven't had a
> chance to look too much into the actual functional changes in the patch;
> I'll try to review later today or tomorrow.

Will fix you comments, although I have to say !object should be
forbidden on structures in gcc ^_^. I think NULL or != NULL shows we
dealing with not-booleans, which makes it easier to understand.
Anyway, your party.

> Did you run a test app through valgrind that creates and destroys a
> bunch of bindings to check for memleaks etc.?

Nope, but I'm sure all the bindings are disconnected and thus the
structure is freed. So no worries.

> Oh, please don't merge master into your branch; I prefer rebasing so
> history stays linear and it's easier to merge the branch to master
> later.  Rewriting history is fine for a temporary branch.

Yeah well, what can I say... Probably has something to do with a
combination of lazy and not-sure-what-is-going-to-happen.

Nick



More information about the Xfce4-dev mailing list