Conversation
ia0
left a comment
ia0
left a comment
There was a problem hiding this comment.
Wait, I've just seen the performance comment, let me write something down to try to fix it.
| with_nfc = ["libtock_drivers/with_nfc"] | ||
| vendor_hid = ["opensk/vendor_hid"] | ||
| ed25519 = ["ed25519-compact", "opensk/ed25519"] | ||
| rust_crypto = ["opensk/rust_crypto"] |
There was a problem hiding this comment.
At the end of this file, you can add the following for crates you want to optimize. This will trade with binary size, so only select those that matter. Might not work though, so needs testing.
[profile.release.package]
aes = { opt-level = 3 }
p256 = { opt-level = 3 }
sha2 = { opt-level = 3 }There was a problem hiding this comment.
Good idea! I'll keep the speed settings, as RustCrypto is only an option for boards with enough binary size anyway.
Speed in ms / iter (-Oz -> -O3 (our implementation)):
AES and SHA now beats our implementation, but ECDSA is still slower. I wonder why key generation is so slow though.
| Our impl | -Oz | -O3 | |
|---|---|---|---|
| Aes256::encrypt_block | 0.21 | 0.55 | 0.37 |
| Aes256::decrypt_block | 1.28 | 0.55 | 0.40 |
| Sha256::digest(32 bytes) | 0.18 | 0.24 | 0.15 |
| Ecdsa::SecretKey::random | 3.51 | 356.84 | 221.50 |
| Ecdsa::SecretKey::public_key | 114.62 | 0.03 | 0.03 |
| Ecdsa::SecretKey::sign | 194.89 | 402.95 | 240.11 |
Binary size
- aes: +2.1 kB
- sha2: +3.7 kB
- p256: +7.0 kB
There was a problem hiding this comment.
First time I try this, so I'm learning. Apparently this is not recursive, so you have to specify the relevant dependencies too. I think we need to add also those crates to the list of opt-level = 3:
ecdsaelliptic-curvecrypto-bigint- maybe
primeorder - maybe
rand_core - maybe
subtle
Or maybe a subset of those is enough, or maybe I'm still missing some that are important.
I don't think it's worth trying to optimize too much though, so I would not try more than once.
There was a problem hiding this comment.
1% faster for Ecdsa::SecretKey::sign, no effect on anything else, but we lose more binary size. So I suggest we keep it as is!
| with_nfc = ["libtock_drivers/with_nfc"] | ||
| vendor_hid = ["opensk/vendor_hid"] | ||
| ed25519 = ["ed25519-compact", "opensk/ed25519"] | ||
| rust_crypto = ["opensk/rust_crypto"] |
3 participants
Adds a feature flag for RustCrypto (see #1).
As mentioned in #620, binary size increases significantly with the new flag. Our
crypto_benchexample also sees that speed decreases. Non-embedded new environments might still find it interesting.The benchmarks and
cargo bloatallow a more detailed analysis: AES doesn't seem to hurt binary size or speed, so we could decide to move to the RustCrypto implementation there, and keep ours for ECDSA for now.