-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/crypto/chacha20: It should be possible to reinitialise a chacha20 primitive to avoid unnecessary allocations #44449
Comments
Rather than requiring a new memory allocation for every use of the chacha20 primitive allow an existing chacha20.Cipher to be reinitialised with a new key + nonce. This can help avoid allocations in hot paths. Fixes golang/go#44449
Rather than requiring a new memory allocation for every use of the chacha20 primitive allow an existing chacha20.Cipher to be reinitialised with a new key + nonce. This can help avoid allocations in hot paths. Fixes golang/go#44449
Rather than requiring a new memory allocation for every use of the chacha20 primitive allow an existing chacha20.Cipher to be reinitialised with a new key + nonce. This can help avoid allocations in hot paths. Fixes golang/go#44449
Can you provide a benchmark to show whether this is a performance gain over the stack allocation? Stack allocations are essentially free, so I would be a bit surprised. |
Change https://golang.org/cl/294649 mentions this issue: |
My issue isn't when the stack allocation succeeds; you are correct that I see no impact in that instance. However I have a situation where I'm using the chacha20 module under the hood to enable AEAD on a set of distinct scatter-gather buffers without the need to do unnecessary copying and the chacha20.Cipher allocation is escaping to the heap as a result of that abstraction. Being able to embed the Cipher in the abstraction structure as a private element allows both to live on the stack and avoid the allocation overhead. |
The chacha20 package tries to allow the Cipher allocation to be inlined and put upon the stack in NewUnauthenticatedCipher(), but it does not provide any way to use an existing Cipher and reinitialise it. This can result in workers that are on hot paths doing unnecessary allocations instead of being able to allocate the memory for the Cipher up front and simply initialise it with the correct key + nonce during operation.
I have a simple patch which adds an Init() method to allow this which I intend to submit shortly.
The text was updated successfully, but these errors were encountered: