perf: manager device list - #3306
Open
geraldo-netto wants to merge 9 commits into
Open
Conversation
Contributor
|
How much of a performance impact are you seeing? It may be negative as getting all the properties instead of a couple will result in more dbus calls in the end. The best performance increase will be using the cached properties on the proxy. However last time I tried I ran into issues with the singleton design and most likely stale proxies. I haven't looked too closely but you don't need to cast |
Contributor
Author
Contributor
Author
Benchmarks the row_setup_event/row_update_event hot path by modeling each device property access as one synchronous D-Bus round-trip. run_compare.sh diffs the current branch against a base ref (default main) and asserts a minimum gain. Why the gain: every device["Key"] resolves to one synchronous org.freedesktop.DBus.Properties.Get via Gio call_sync (blueman/bluez/Base.py get()). main reads each property individually and re-runs row_update_event four times inside row_setup_event, so a single row costs ~57 round-trips. The branch calls get_properties() once (one Properties.GetAll), caches uuids in the liststore, and batches the set() calls, cutting the path to ~5 round-trips. D-Bus round-trips dominate wall time, so collapsing them is the gain; pure-Python CPU is near-flat between versions. Measured (10000 iterations, DBUS_RTT=200us, /usr/bin/python3): metric main (fb41890) branch (af0d8fe) gain ---------------------- --------------- ----------------- --------- D-Bus round-trips/iter 57 5 91.2% fewer round-trips total 570000 50000 91.2% fewer cpu_seconds 4.54 4.15 8.5% faster modeled wall time 118.54s 14.15s 88.1% faster speedup factor 1.0x 8.4x 8.4x Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
geraldo-netto
force-pushed
the
fix/manager-device-list-perf
branch
from
June 19, 2026 09:01
e1e13e8 to
c100da1
Compare
Contributor
Author
|
@infirit , could you please review it now? |
Contributor
|
Apologies I should have looked closer. I forgot that GetProperties is a single dbus call 😓. I'll do some testing over the weekend. |
Per review: NewType values can be constructed directly (BtAddress(value)); type checkers infer the correct type without typing.cast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GenericList.set wrote columns one liststore.set call at a time, emitting one row-changed per column. Every emission re-runs the sort machinery and the visibility filter, so a multi-column row update paid that cost repeatedly. Passing all columns in a single call emits row-changed once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every device["Key"] is a synchronous Properties.Get round trip and get_properties() is one GetAll. Cache the remaining hot properties in row state and share one snapshot across the add path: - Cache Class, Address and Icon in liststore columns (kept fresh from property-changed signals, mirroring the GetAll fallbacks for invalidated properties). Icon rebuilds and Alias updates in row_update_event now run without any D-Bus calls, as do filter_func (evaluated on every row-changed), drag-and-drop send, Ctrl+C address copy, double-click connect and menu generation. - The per-second power-level timer read Connected live from D-Bus for every monitored device; use the cached column instead. - add_device did three round trips per discovered device (Adapter get, GetAll for the Name check, GetAll in row_setup_event); do one GetAll and pass the snapshot through row_setup_event. Same for DeviceSelectorList, which did four separate property reads per row. - Cache signal bar pixbufs per (bar, level, scale) instead of decoding the PNG on every level change, and stop reading the battery percentage over D-Bus just to build a debug log line when debug logging is disabled. Benchmark (test/benchmarks): 5 -> 1 modeled round trips per row lifecycle vs the previous commit, 57 -> 1 vs main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend the headless unit tests to cover the paths the perf work touches: GenericList.set batching (row-changed emission count against a real ListStore), single-GetAll add_device including the foreign-adapter and named-device branches, and DeviceSelectorList row setup. Add deterministic fuzz-style tests: a 2000-event randomized property state machine over row_update_event asserting the cached row state always matches a reference model and the device proxy is never read, plus randomized sweeps over _has_objpush and make_display_name. Changed-line coverage vs main: ManagerDeviceList 94%, GenericList, DeviceList and DeviceSelectorList 100% (ManagerDeviceMenu.generate still needs a real Gtk.Menu and GSettings schemas, so it stays untested, as before). Also list the test modules in EXTRA_DIST so release tarballs include them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
geraldo-netto
force-pushed
the
fix/manager-device-list-perf
branch
from
July 8, 2026 19:37
4a08679 to
78cfefc
Compare
|
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
get_properties()snapshots.GetAllsnapshot when adding device rows (DeviceList.add_device,DeviceSelectorList) instead of three to four round trips per discovered device.GenericList.setcolumn writes into a singlerow-changedemission, so the sort machinery and visibility filter run once per update instead of once per column.Measured (10000 iterations, DBUS_RTT=200us, /usr/bin/python3):
fb41890)24cdca0)Benchmarks the row_setup_event/row_update_event hot path by modeling each device property access as one synchronous D-Bus round-trip. run_compare.sh diffs the current branch against a base ref (default main) and asserts a minimum gain.
Why the gain: every
device["Key"]resolves to one synchronousorg.freedesktop.DBus.Properties.Getvia Gio call_sync (blueman/bluez/Base.pyget()). main reads each property individually and re-runs row_update_event four times inside row_setup_event, so a single row costs ~57 round-trips. The branch callsget_properties()once (oneProperties.GetAll) per row lifecycle, caches uuids/class/address/icon in the liststore and keeps them fresh from property-changed signals, so setup costs exactly one round trip and property updates cost zero. D-Bus round-trips dominate wall time, so collapsing them is the gain; pure-Python CPU is near-flat between versions.Tests
/usr/bin/python3 -m unittest test.gui.manager.test_manager_device_list test.gui.test_device_lists(36 tests)row_update_eventasserting the cached row state always matches a reference model and the device proxy is never read, plus randomized sweeps over_has_objpushandmake_display_name.trace): ManagerDeviceList 94%, GenericList / DeviceList / DeviceSelectorList 100%.ManagerDeviceMenu.generateremains untested (needs a realGtk.Menuand installed GSettings schemas), as before this PR.test/benchmarks/run_compare.sh 10000 mainpasses (numbers above).