altdesktop / python-dbus-next

🚌 The next great DBus library for Python with asyncio support
https://python-dbus-next.readthedocs.io/en/latest/
MIT License
187 stars 58 forks source link

Improve unmarshall performance #126

Closed bdraco closed 1 year ago

bdraco commented 1 year ago

This change reduces the overhead of unmarshalling with the goal of having bleak scanners not overwhelming the system.

See related issue: https://github.com/hbldh/bleak/issues/236#issuecomment-789055069

(speed_up_unmarsh) % python3 bench/unmarshall.py
Unmarshalling 1000000 bluetooth rssi messages took 13.497067291998974 seconds
(master) % python3 bench/unmarshall.py
Unmarshalling 1000000 bluetooth rssi messages took 47.7756206660124 seconds
bdraco commented 1 year ago

I've been running this in production for a while.

I get more broken pipe errors with this change.

Either there is a bug or its too fast that the data hasn't come over the wire yet, and since the unmarshall code never waits for data to arrive over the wire if its split between multiple reads it increases the chance it will process it too fast and it won't be there yet.

bdraco commented 1 year ago

I think broke resume with the change

bdraco commented 1 year ago

Added a test for resume

bdraco commented 1 year ago

Spent another couple hours testing this and its all working great with bleak

bdraco commented 1 year ago

Related previous PR https://github.com/altdesktop/python-dbus-next/pull/62

cc @rjarry in case you have any interest based on your previous PR

bdraco commented 1 year ago

Above comments are for the future. I'm speculating on ways to speed it up a bit more, but anything else is going to be a much smaller improvement and can come in a future PR

acrisci commented 1 year ago

You have failing tests. Please rebase against master and run make docker-test to see test failures. I believe CICD build should work now.

bdraco commented 1 year ago

Still looks good

bdraco@Js-MacBook-Pro python-dbus-next % python3 bench/unmarshall.py 
Unmarshalling 1000000 bluetooth rssi messages took 13.812721209134907 seconds
acrisci commented 1 year ago

:+1:

bdraco commented 1 year ago

Thanks. I have a few more I can pull out as well that I'll cleanup and send as soon. I'm traveling this week so its going to depend on how ✈️ goes and jet lag