avoid global buffers #219
Merged
avoid global buffers #219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reynir
approved these changes
Mar 19, 2024
There was a problem hiding this comment.
On my system the performance seems to be about the same:
aes-128-gcm
| bytes | main | d3e0a57 | 26e4481 |
|---|---|---|---|
| 16 | 107.919318 | 108.127357 | 102.645635 |
| 64 | 352.568878 | 348.686773 | 336.847306 |
| 256 | 1127.308329 | 1126.252432 | 1074.113642 |
| 1024 | 1991.705995 | 1976.265390 | 1913.386337 |
| 8192 | 1385.496016 | 1380.143187 | 1404.725130 |
fortuna
| bytes | main | d3e0a57 | 26e4481 |
|---|---|---|---|
| 16 | 102.238773 | 102.721865 | 99.792642 |
| 64 | 369.019572 | 366.661943 | 354.053437 |
| 256 | 1205.616092 | 1200.784292 | 1163.625689 |
| 1024 | 2440.599733 | 2447.444005 | 2412.249927 |
| 8192 | 2258.243312 | 2233.075545 | 2114.863257 |
|
my benchmarks: aes-128-gcm
fortuna
|
|
I updated the table. It seems slightly slower on my system now. I'm not sure it's significant. |
|
thanks @reynir. if you run it 5 times, do you reproduce these slower numbers, or do they vary a lot? |
|
I ran it a few times. It seems fairly consistent that for short inputs it's slightly slower. For longer inputs it's about the same or in the case of aes-128-gcm maybe slightly faster. |
|
..slightly slower -- compared to the ~33% performance decrease measured in #186, the slight decrease observed here is IMHO fine. of course other opinions are welcome (and also more measurements). in the meantime, I'll merge - we can iterate on this code before release. |
hannesm
added a commit
that referenced
this pull request
Mar 19, 2024
* avoid global buffers * rng: safety - ensure generate_into takes a long enough buffer (raise otherwise) * rng: interrupt_hook only one unit argument (@reynir) * remove offset from counters Co-authored-by: Reynir Björnsson <[email protected]> Co-authored-by: Calascibetta Romain <[email protected]>
hannesm
added a commit
to hannesm/opam-repository
that referenced
this pull request
Jun 29, 2024
CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)
avsm
pushed a commit
to avsm/opam-repository
that referenced
this pull request
Sep 5, 2024
CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this should fix most of #186 -- performance-wise (tested on fortuna), this gives some speedup (but obviously should be tested a bit more)
//cc @reynir @dinosaure