blake2b: export Digest and (*Digest).Init #190
+40
−26
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 allows clients to eliminate allocations when streaming data into the hash.
Thanks to golang/go#33160, allocations are already eliminated in many simple contexts. However, once you want to do anything more sophisticated -- specifically, introducing helper functions or types -- it becomes difficult or impossible to avoid allocations due to
hash.Hash
. As a motivating example:It is clear that substituting a concrete type for the
hash.Hash
in 2 and 3 will eliminate all allocations, but this is not currently possible. This PR exportsblake2b.Digest
, which allows clients to access the concrete type without breaking API compatibility.Originally, I had hoped that merely exporting the type would be sufficient. Clients could then use a type assertion on the returned
hash.Hash
to convert it to a*blake2b.Digest
. Unfortunately, this isn't good enough forNewUint64Hasher
, because the combination of constructors invariably exceeds the inlining budget, causing the Digest to escape:Now, it is possible to refactor
blake2b.New256
such that the combined budget is not exceeded, by manually inliningnewDigest
and removing a few unnecessary expressions:...but this is probably too ugly to be worth it, considering we'd have to do this for every constructor.
Another option would be to add a
NewDigest(size int, key []byte) (*Digest, error)
method. Unfortunately, this too exceeds the inlining budget, due to theerrHashSize
check.So the only practical recourse is to add an
Init
method. We can then write the constructor as:Since the
Digest
is declared inNewUint64Hasher
, it won't escape to the heap as long asNewUint64Hasher
itself can be inlined. Benchmarking confirms that this is zero-alloc:Assuming this is accepted, I'm happy to open equivalent PRs for other hash functions as well.