Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Change parsing logic to bytes manipulation directly #51

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

greglucas
Copy link
Collaborator

@greglucas greglucas commented Jul 30, 2024

Speed up parsing by removing bitstring reads

Avoid bitstring and do the bitwise reading and manipulations ourselves with bitwise logic. This greatly increases the performance of the parser by ~3-5x in the read routine. This is sort of the MVP for speed-ups by just implementing the routines we need and trying to replicate bitstring. We could do better by refactoring the _get_format_string() logic and avoiding any string matching/parsing/splitting too.

I did not fully remove bitstring at this point since there is a lot of logic surrounding it in parser.py. We could make it an optional dependency, but it would be nice to get rid of the legacy_parse_packet() method if we go that route. Happy to add that deprecation path here if it'd be useful, but probably a major version bump then.

Profiling

main
image

this branch
image

Checklist

  • Changes are fully implemented without dangling issues or TODO items
  • Deprecated/superseded code is removed or marked with deprecation warning
  • Current dependencies have been properly specified and old dependencies removed
  • [n/a] New code/functionality has accompanying tests and any old tests have been updated to match any new assumptions
  • The changelog.md has been updated

Avoid bitstring and do the manipulations ourselves with bitwise logic.
This greatly increases the performance of the parser.
…ests

The extract_bit function was incorrect when a value crossed the boundary
due to the modulo arithmetic not adding one to the modulo start bit not
being added to the end_byte calculation.
Copy link
Owner

@medley56 medley56 left a comment

Choose a reason for hiding this comment

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

This looks good to me. All the unit and integration tests pass.
I think no matter what, the next version is going to be a major bump, so let's go ahead and remove bitstring entirely, but let's do it in a separate PR. LGTM!

@medley56 medley56 merged commit 51dee61 into medley56:main Aug 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants