Bluetooth-Devices / dbus-fast

A faster version of dbus-next
MIT License
37 stars 8 forks source link

fix: avoid raising exception when we know there is no data in the buffer #211

Closed bdraco closed 1 year ago

bdraco commented 1 year ago

If we know _unmarshall will internally raise EOFError via peek, we now return and wait until asyncio calls us again to read

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 25.00% and project coverage change: +0.04% :tada:

Comparison is base (c40c7bc) 82.46% compared to head (72d6d67) 82.51%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #211 +/- ## ========================================== + Coverage 82.46% 82.51% +0.04% ========================================== Files 27 27 Lines 3171 3174 +3 Branches 655 656 +1 ========================================== + Hits 2615 2619 +4 Misses 342 342 + Partials 214 213 -1 ``` | [Files Changed](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices) | Coverage Δ | | |---|---|---| | [src/dbus\_fast/\_\_version\_\_.py](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices#diff-c3JjL2RidXNfZmFzdC9fX3ZlcnNpb25fXy5weQ==) | `0.00% <0.00%> (ø)` | | | [src/dbus\_fast/\_private/unmarshaller.py](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices#diff-c3JjL2RidXNfZmFzdC9fcHJpdmF0ZS91bm1hcnNoYWxsZXIucHk=) | `96.50% <ø> (ø)` | | | [src/dbus\_fast/proxy\_object.py](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices#diff-c3JjL2RidXNfZmFzdC9wcm94eV9vYmplY3QucHk=) | `76.51% <ø> (ø)` | | | [src/dbus\_fast/aio/message\_reader.py](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices#diff-c3JjL2RidXNfZmFzdC9haW8vbWVzc2FnZV9yZWFkZXIucHk=) | `84.61% <33.33%> (-6.69%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/Bluetooth-Devices/dbus-fast/pull/211/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Bluetooth-Devices)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bdraco commented 1 year ago

peek does more than peek

It can actually READ!

https://github.com/python/cpython/blob/f106254e0b3c83d9064763d6132b3fe997da901b/Lib/_pyio.py#L1151

bdraco commented 1 year ago

this is so much better for small messages but kills performance for large ones

bdraco commented 1 year ago

I'm not sure this is going to work since there is no public method to peek that does not cause a read to happen

bdraco commented 1 year ago

Its not worth the change risk unless we can actually avoid the syscall