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

x/crypto/chacha20: It should be possible to reinitialise a chacha20 primitive to avoid unnecessary allocations #44449

Open
u1f35c opened this issue Feb 20, 2021 · 3 comments · May be fixed by golang/crypto#178
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@u1f35c
Copy link

u1f35c commented Feb 20, 2021

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.

@gopherbot gopherbot added this to the Unreleased milestone Feb 20, 2021
u1f35c added a commit to u1f35c/crypto that referenced this issue Feb 20, 2021
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
u1f35c added a commit to u1f35c/crypto that referenced this issue Feb 20, 2021
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
u1f35c added a commit to u1f35c/crypto that referenced this issue Feb 20, 2021
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
@FiloSottile
Copy link
Contributor

FiloSottile commented Feb 20, 2021

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.

@gopherbot
Copy link

gopherbot commented Feb 20, 2021

Change https://golang.org/cl/294649 mentions this issue: chacha20: Allow reinitialisation of existing chacha20 structure

@u1f35c
Copy link
Author

u1f35c commented Feb 20, 2021

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.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants