Discovered devices improvements #1047

dlech · 2022-09-30T19:46:47Z

Performance improvements for keeping track of discovered devices.
New property to get advertisement data along with discovered devices.
Adds rssi attribute to AdvertisemntData

Additional details in the commit messages.

This changes all scanner backends so that they only create a BLEDevice object once per scan. For example, in several backends, a new BLEDevice was created for each discovered device each time the discovered_devices property was called, which could be significant if many (~100) devices were discovered. This also caches the last advertisement data along with the BLEDevice which is expected to be used as an alternative to BLEDevice.metadata in the future.

This adds an `rssi` attribute to the AdvertisementData type. This is done so we can eventually deprecate the `rssi` on the BLEDevice object (#1025). While we are touching this, also change AdvertisementData to a named tuple so we don't have to initialize all of the values ourselves.

This reduces code duplication and helps ensure that all backends work the same.

This adds a new discovered_devices_and_advertisement_data property to BleakScanner that can be used to get the last advertisement data in addition to the BLEDevice of the devices discovered during the scan session. A new argument is added to the BleakScanner.discover() class method to allow this method to return the new property as well.

dlech · 2022-09-30T19:47:03Z

Hi @bdraco, could you please take a look?

bdraco · 2022-09-30T20:02:53Z

Should be able to take a look a bit later today.

👍

bdraco · 2022-09-30T20:03:49Z

bleak/backends/bluezdbus/scanner.py

            tx_power=tx_power,
+            rssi=props.get("RSSI", -127),


bdraco · 2022-09-30T20:04:35Z

bleak/backends/bluezdbus/utils.py

+    Returns:
+        A Bluetooth address as a string.
+    """
+    return ":".join(device_path[-17:].split("_"))


Might be faster to grab it and use .replace

Currently, this isn't used in any critical paths.

bdraco · 2022-09-30T20:05:59Z

bleak/backends/scanner.py


 from ..exc import BleakError
 from .device import BLEDevice


-class AdvertisementData:
+class AdvertisementData(NamedTuple):


I wonder if it would be faster as a dataclass?

Nope. Named tuple is faster

Found this while digging around https://medium.com/@jacktator/dataclass-vs-namedtuple-vs-object-for-performance-optimization-in-python-691e234253b9

But will get better data when I run in through the profiler since we will get actual use case data vs theory

Part of the reason for using NamedTuple was for the immutability, but if there is a way to speed it up, we can consider alternatives.

Makes sense. I don't think it's going to be an issue. Will know for sure once I do the profiles on the production systems. Identifying this as an area to target for additional profiling and testing.

Don't forget to add frozen=True to dataclasses when profiling 👍

bdraco · 2022-09-30T20:11:49Z

This looks awesome. Cleans things up quite a bit 👍

I added some first impression thoughts above. Some need to be validated with testing

I'll do testing and profiling when other stuff calms down later tonight and I don't have as many distractions

bdraco · 2022-09-30T20:15:52Z

bleak/backends/scanner.py

@@ -127,6 +160,45 @@ def detection_callback(s, d):

        self._callback = detection_callback

+    def create_or_update_device(


For profiling this later, assume this is hot path

bdraco · 2022-09-30T23:54:27Z

bleak/backends/scanner.py


 from ..exc import BleakError
 from .device import BLEDevice


-class AdvertisementData:
+class AdvertisementData(NamedTuple):


Not sure if it can be avoided but
Sep 30 23:52:42 homeassistant homeassistant[487]: TypeError: AdvertisementData.__new__() missing 1 required positional argument: 'rssi'

bdraco · 2022-10-01T00:06:06Z

It looks like the parse_msg path is slightly more expensive.

New

Original

bdraco · 2022-10-01T00:15:02Z

I think this could be made more performant by storing the _advertisement_callbacks by adapter path and doing a dict lookup to find it by adapter path so it can avoid enumerating them and calling startswith for every callback.

Maybe slice device_path to get the adapter path and lookup self._advertisement_callbacks_by_adapter.get(device_path[SLICED])?

        for (callback, adapter_path) in self._advertisement_callbacks:
            # filter messages from other adapters
            if not device_path.startswith(adapter_path):
                continue

            # TODO: this should be deep copy, not shallow
            callback(device_path, device.copy())

dlech · 2022-10-01T00:43:11Z

It looks like the parse_msg path is slightly more expensive.

How much of this difference could just be noise/variance between runs? For example _async_detection_callback increased 0.2% but that isn't even part of Bleak. And unpack_variants increased as well (0.04%) and it wasn't changed.

The part that was changed - create_or_update_device (0.24%) vs the previous (BleakDevice) __init__ (0.40%) seems to be an improvement though.

bdraco · 2022-10-01T01:02:06Z

I'll do a few more. Its not an exact science since we can't control how many advertisements come in.

bdraco · 2022-10-01T01:04:32Z

2nd run

bdraco · 2022-10-01T01:06:13Z

3rd run

bdraco · 2022-10-01T01:07:59Z

4th run

bdraco · 2022-10-01T01:11:05Z

Only thing that still stands out as easy to optimize is startswith since its call 80k/min

dlech · 2022-10-01T01:12:09Z

Comparing relative values instead of absolute values, it looks like "before", _run_advertisement_callbacks was > 10x unpack_variants while "after", it is consistently < 10x unpack_variants, so overall improvement, no? (assuming the one "before" measurement is repeatable with similar results)

bdraco · 2022-10-01T01:12:52Z

All of these are 1 min snapshots with % is run time on a single core of _parse_msg and descendants. _parse_msg is the 2nd most expensive branch running (unmarshalling is the first most expensive but only by ~25% more now -- it was taking almost all the cpu run time before the switch to dbus-fast and maxxing out the core -- that one is hard to find any more wins).

bdraco · 2022-10-01T01:15:05Z

I'll revert and do another one to compare. The system is mostly idle otherwise but its a real world performance test so everything is affected by everything else

bdraco · 2022-10-01T01:19:36Z

Original run 2

bdraco · 2022-10-02T17:22:51Z

Some good news about the hot unmarshalling path, it’s now much faster as it has an optional (auto fallback to pure python if not available) which makes this path now the hottest one in production.

I’ll see if I can get unpack variants exposed in dbus-fast since it was already added in a previous PR and add an optional (auto fallback to pure python) cython backend to shave that off a bit. #1055

This fixes a regression from #1047.

In #1047, the call to getRssi() was broken by using the wrong object. This fixes the mistake. Fixes #1085.

In hbldh#1047, the call to getRssi() was broken by using the wrong object. This fixes the mistake. Fixes hbldh#1085.

bojanpotocnik · 2023-07-17T07:31:51Z

bleak/backends/scanner.py

+    seen_devices: Dict[str, Tuple[BLEDevice, AdvertisementData]]
+    """
+    Map of device identifier to BLEDevice and most recent advertisement data.
+
+    This map must be cleared when scanning starts.
+    """
+


What is the point of this class variable in addition to the instance variable defined below?

Since nothing is assigned to the name here, this is just a type hint rather than a class variable.

Of course, my apologies 🤦 I suppose moved here to not clutter the __init__() method with docstring?

dlech and others added 4 commits September 30, 2022 14:45

backends/scanner: refactor common adv data code

54f68c5

This reduces code duplication and helps ensure that all backends work the same.

bdraco reviewed Sep 30, 2022

View reviewed changes

bleak/backends/bluezdbus/scanner.py

tx_power=tx_power,

rssi=props.get("RSSI", -127),

Copy link

Contributor

bdraco Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

bdraco reviewed Sep 30, 2022

View reviewed changes

dlech merged commit 0e439ee into develop Oct 3, 2022

dlech deleted the discovered-devices branch October 3, 2022 15:36

dlech mentioned this pull request Oct 3, 2022

backends/bluezdbus/manager: replace startswith in hot path #1058

Merged

dlech added a commit that referenced this pull request Oct 4, 2022

backends/winrt/scanner: fix incorrect arg to create_or_update_device

6561bb4

This fixes a regression from #1047.

dlech mentioned this pull request Oct 13, 2022

Release/0.19.0 #1081

Merged

dlech added a commit that referenced this pull request Oct 19, 2022

backends/p4android/scanner: fix crash when getting rssi

76e1c5b

In #1047, the call to getRssi() was broken by using the wrong object. This fixes the mistake. Fixes #1085.

dlech mentioned this pull request Oct 19, 2022

backends/p4android/scanner: fix crash when getting rssi #1086

Merged

dlech added a commit that referenced this pull request Oct 25, 2022

backends/p4android/scanner: fix crash when getting rssi

b48e4cf

In #1047, the call to getRssi() was broken by using the wrong object. This fixes the mistake. Fixes #1085.

pmessan pushed a commit to workaroundgmbh/bleak that referenced this pull request Nov 28, 2022

backends/p4android/scanner: fix crash when getting rssi

b80d71c

In hbldh#1047, the call to getRssi() was broken by using the wrong object. This fixes the mistake. Fixes hbldh#1085.

bojanpotocnik reviewed Jul 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discovered devices improvements #1047

Discovered devices improvements #1047

dlech commented Sep 30, 2022

dlech commented Sep 30, 2022

bdraco commented Sep 30, 2022

bdraco Sep 30, 2022

bdraco Sep 30, 2022

dlech Sep 30, 2022

bdraco Sep 30, 2022

bdraco Sep 30, 2022

bdraco Sep 30, 2022 •

edited

Loading

dlech Sep 30, 2022

bdraco Sep 30, 2022 •

edited

Loading

balloob Oct 2, 2022

bdraco commented Sep 30, 2022

bdraco Sep 30, 2022

bdraco Sep 30, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 •

edited

Loading

dlech commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022

dlech commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 2, 2022 •

edited

Loading

bojanpotocnik Jul 17, 2023

dlech Jul 17, 2023

bojanpotocnik Jul 17, 2023

dlech Jul 17, 2023

		@@ -127,6 +160,45 @@ def detection_callback(s, d):

		self._callback = detection_callback

		def create_or_update_device(

Discovered devices improvements #1047

Discovered devices improvements #1047

Conversation

dlech commented Sep 30, 2022

dlech commented Sep 30, 2022

bdraco commented Sep 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco commented Sep 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 • edited Loading

dlech commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 • edited Loading

bdraco commented Oct 1, 2022

dlech commented Oct 1, 2022 • edited Loading

bdraco commented Oct 1, 2022 • edited Loading

bdraco commented Oct 1, 2022

bdraco commented Oct 1, 2022 • edited Loading

bdraco commented Oct 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdraco Sep 30, 2022 •

edited

Loading

bdraco Sep 30, 2022 •

edited

Loading

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022 •

edited

Loading

dlech commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 1, 2022 •

edited

Loading

bdraco commented Oct 2, 2022 •

edited

Loading