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

pem.unarmor very poor performance #272

Open
James-E-A opened this issue Dec 8, 2023 · 0 comments
Open

pem.unarmor very poor performance #272

James-E-A opened this issue Dec 8, 2023 · 0 comments

Comments

@James-E-A
Copy link

James-E-A commented Dec 8, 2023

The attached test file (a Classic McEliece keypair, which includes a 1MiB public key, encoded per RFC5958+RFC7468) takes around 7 seconds to unarmor with asn1crypto. (Tested on asn1crypto version 1.5.1 .)

Path.read_bytes() takes only an instant, as (crucially) does OneAsymmetricKey.load()*; it's only the asn1crypto.pem.unarmor() step that takes an eternity.

I haven't done any Fancy Intra-Function Profiling yet, but the looped += byte construction here looks extremely painful.

Three solutions come immediately to mind:

  • Leave the code 99% as-is, just use base64_data = bytearray as a band-aid since it's got an optimized mutating __iadd__
  • Factor the body line loop out, and use b''.join() on a generator
  • Refactor the code to actually stream the body into base64.decode(), probably by duck typing

Of these, the second seems like an immediately reasonable choice, at least while "objects that don't even fit into RAM" still isn't on the table.

Would you be interested in a pull request, or is this something you are going to fix yourself?


*My own derivative of asn1crypto.keys.PrivateKeyInfo that's been updated to 'v2'; it's not directly relevant to this bug report, except I wanted to emphasize that according to time.perf_counter() the DER decoding is not the weak link.

3e-07 Moving key into RAM with Path.read_bytes()...
0.057328 Done.
0.0896701 Unarmoring key with pem.unarmor()...
7.0062617 Done.
7.0573913 Loading key with OneAsymmetricKey.load()...
7.0779615 Done.

James-E-A added a commit to James-E-A/asn1crypto that referenced this issue Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant