Bug 14183 - should use arg0 matching for NameOwnerChanged
Summary: should use arg0 matching for NameOwnerChanged
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Rob Taylor
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 14184
  Show dependency treegraph
 
Reported: 2008-01-21 11:10 UTC by Simon McVittie
Modified: 2009-02-10 07:02 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Bug-14183-Listen-to-NameOwnerChanged-using-arg0-ma (4.27 KB, patch)
2009-02-05 08:19 UTC, Colin Walters
Details | Splinter Review

Description Simon McVittie 2008-01-21 11:10:54 UTC
dbus-glib subscribes to all NameOwnerChanged signals for each DBusGProxy instance (or possibly just the ones bound to a unique name), so all dbus-glib applications are woken up whenever any application joins or leaves the bus. It should subscribe using "arg0" matching, like dbus-python does, so only signals it cares about cause a wakeup. It should also provide API for clients to do this.

Something like:

watch_name_owner(bus_name, callback, the usual closure stuff): like tp_dbus_daemon_watch_name_owner in telepathy-glib or dbus.Bus.watch_name_owner in dbus-python. Call a user-supplied callback when the name owner has been discovered, then call it again every time the name owner changes. Some way to disconnect from the notification should be provided.
Comment 1 Simon McVittie 2008-01-21 11:12:47 UTC
As a small optimization, if the name given was a unique name, we could automatically disconnect the notification when it fell off the bus (since the whole point is that unique names aren't re-used).
Comment 2 Mikhail Zabaluev 2009-01-27 02:28:41 UTC
As a nasty effect of this issue, application developers can afford to be sloppy with indiscriminately connecting to NameOwnerChanged and point to dbus-glib in justification. So the later this bug gets fixed, the more applications will commit the same mistake and won't be singled out as performance drains. I know at least one example apart from bug #14184.
Comment 3 Colin Walters 2009-02-05 08:19:53 UTC
Created attachment 22614 [details] [review]
Bug-14183-Listen-to-NameOwnerChanged-using-arg0-ma

Provisional patch - passes the current test suite, but I haven't double checked the wakeups yet.
Comment 4 Colin Walters 2009-02-05 08:21:50 UTC
On the subject of a watch_name_owner API - this would be very nice, I agree; but right now in a lot of cases it is sufficient to use dbus_g_proxy_new_for_name_owner and connect to the "destroy" signal.
Comment 5 Colin Walters 2009-02-05 10:27:16 UTC
I've verified this patch prevents wakeups.  Test case was attaching strace to bluetooth-applet and then starting up dbus-monitor --session.  With patch, every invocation of dbus-monitor woke the applet up out of poll() to read the NameOwnerChanged signal.  With the patch, it does not.
Comment 6 Dafydd Harries 2009-02-10 04:34:09 UTC
Oops, patch writing race condition. My patch is substantially similar, but more complicated in that it avoids adding duplicate match rules, but maybe this doesn't win us anything. Your patch looks fine to me.
Comment 7 Colin Walters 2009-02-10 07:02:03 UTC
Ahh, apologies for the duplication; both of us are probably at fault here for not claiming the bugzilla mutex =)

But thanks for the review!  I hadn't considered the duplicate issue; my intuition is that if it was a real-world problem, it'd be simpler to have the bus daemon keep a reference count for matches instead of a plain linked list.

I've pushed this patch to master.  I've been running it on my system since posting it, but if anyone else could give it some testing on their own system before the next release I'd appreciate it!

commit dfef72c61c050e7f57e1d2eb601701e0a27827cc
Author: Colin Walters <walters@verbum.org>
Date:   Thu Feb 5 11:17:15 2009 -0500

    Bug 14183 - Listen to NameOwnerChanged using arg0 matching
    
    This is more efficient - we avoid waking up every dbus-glib using process
    when a process joins or leaves the bus.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.