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

Implement Reverse Input/Output Lookup Table when running table_based #40

Open
gaiuscosades opened this issue Sep 11, 2022 · 7 comments
Open

Comments

@gaiuscosades
Copy link

gaiuscosades commented Sep 11, 2022

I use the module with the following configuration.

crc.CrcCalculator(
   crc.Configuration(
      width=16,
      polynomial=0x8005, 
      init_value=0x0000,
      final_xor_value=0, 
      reverse_input=True,
      reverse_output=False
   ), 
   table_based=True
)

Performance when crunching big payloads was in the range that I expected.
But while profiling other code I noticed that a vast amount of time was spent simply reversing the input bytes.
I now use the module without reversed_input and reverse the input externally (bytes.translate) which I propose should be done inside the library when running in Table Based Mode, as the memory overhead is negligible, but leads to a 45% speedup in my setup.

Alternatively I think one could transpose the whole lookup table when working with reversed input to get even better performance.

Greetings to the maintainers of this very helpful package.

@Nicoretti
Copy link
Owner

Hi @gaiuscosades,

first of all thanks for taking the time for creating the issue and the kind words regarding the package, appreciate it.
I like the idea(s) for improving the performance of the reversed configurations. Sadly I am currently very short on time
and barley can make some time for the side projects I want to focus on right now. Therefore I don't see me doing this any time soon (just so you know what you are dealing with). Still I think it its a very good issue, therefore I'll keep it in the backlog in case
someone else will/want to address it e.g. during hacktoberfest 2022.

best
Nico

@gaiuscosades
Copy link
Author

Thank you @Nicoretti,

I of course totally understand that you are on a time budget especially since this issue is far from critical!
I would offer to propose an implementation, but sadly I will not have the time as I myself cannot use even a reworked version of the package.
Unfortunately I realised today that I am even more performance constrained than I thought, which demands a way lower level implementation of this functionality on my side.

Still this improvement could be interesting for someone looking for a significant speedup while staying python only!

Best Wishes
Gaius

@Nicoretti
Copy link
Owner

I of course totally understand that you are on a time budget especially since this issue is far from critical! I would offer to propose an implementation, but sadly I will not have the time as I myself cannot use even a reworked version of the package. Unfortunately I realised today that I am even more performance constrained than I thought, which demands a way lower level implementation of this functionality on my side.

fastcrc is using a rust based implementation, this may be a suitable alternative for you

Still this improvement could be interesting for someone looking for a significant speedup while staying python only!

agreed ✔️

@gaiuscosades
Copy link
Author

fastcrc is using a rust based implementation, this may be a suitable alternative for you

Absolutely, found it yesterday!
In my evaluations performance was as good as one would hope for! But as my experience using Rust let alone a Rust Library in Python is nonexistent, I could not figure out yet how one would be able to customize fastcrc to support arbitrary CRC Polynomials etc.

overcat/fastcrc#2

So the ease of use and versatility of this project will not be beaten any time soon... ;D

@Nicoretti
Copy link
Owner

fastcrc is using a rust based implementation, this may be a suitable alternative for you

Absolutely, found it yesterday! In my evaluations performance was as good as one would hope for! But as my experience using Rust let alone a Rust Library in Python is nonexistent, I could not figure out yet how one would be able to customize fastcrc to support arbitrary CRC Polynomials etc.

From what I have seen no such api currently is exposed from the rust library part. One would need to add API for it or add the specific algorithm/configuration. Adding a algorithm/configuration should be pretty straight forward for someone with rust knowledge.

@gaiuscosades
Copy link
Author

gaiuscosades commented Sep 13, 2022

Thanks you for your remarks.

From what I have seen no such api currently is exposed from the Rust library part.

What do you exactly mean by "exposed"? As I understand the Rust part is adaped from https://github.com/mrhooray/crc-rs which shows the desired functionality in it's first example of README.md.

Again as I have absolutely no experience with Rust: Is a compiled artifact of crc-rs part of fastcrc, or is https://github.com/overcat/fastcrc/blob/main/src/lib.rs the only Rust Code present and the lines 1 to 10 of src/lib.rs simply point to a standard Rust library that configures these, which could be extended?

Greetings

@Nicoretti
Copy link
Owner

Thanks you for your remarks.

From what I have seen no such api currently is exposed from the Rust library part.

What do you exactly mean by "exposed"? As I understand the Rust part is adaped from https://github.com/mrhooray/crc-rs which shows the desired functionality in it's first example of README.md.

Well, yes the library which fastcrc uses to provide the crc functionality would support the desired functionality, but from
what I have seen it does not expose it to python.

Basically fastcrc just is a thin wrapper around the crc library, using the PY03 library to expose specific functionality to python.
From what I could see from first glance it does not expose the "configuration" api to python, rather specific crc configurations/algorithms.

Again as I have absolutely no experience with Rust: Is a compiled artifact of crc-rs part of fastcrc, or is https://github.com/overcat/fastcrc/blob/main/src/lib.rs the only Rust Code present and the lines 1 to 10 if src/lib.rs simply point to a standard Rust library that configures these?

crc/crc-rs is "part" of fastcrc (in the form of a dependency: see Cargo.toml) so it will be part of the resulting binary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants