[rfc6962] optimise SHA-256 tree hashing for interior nodes - #264
Conversation
Avoid allocating a new sha256.digest object and dynamic slice buffers on every interior node hash when using crypto.SHA256. In HashChildren, the preimage size is strictly fixed by the RFC6962 specification (1 prefix byte + 2 * 32-byte hashes = 65 bytes). Using a fixed stack buffer and sha256.Sum256 directly avoids the sha256.digest allocation and slice resizing, dropping allocations to 1 (the returned slice), instead of the previous 3. Noted that there is a similar performance optimisation for HashLeaf, but this one requires an arbitrary threshold so has been left out of scope for this PR. Benchmarks (averaged over 5 runs): BenchmarkHashChildren: Before: 1282 ns/op, 240 B/op, 3 allocs/op After: 655 ns/op, 32 B/op, 1 allocs/op (-48.9% ns/op, -86.7% B/op, -66.7% allocs) compact.BenchmarkAppend (1024 leaves): Before: 2296773 ns/op, 414202 B/op, 7170 allocs/op After: 1331209 ns/op, 201417 B/op, 5124 allocs/op (-42.0% ns/op, -51.4% B/op, -28.5% allocs)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
==========================================
+ Coverage 47.14% 47.68% +0.53%
==========================================
Files 8 8
Lines 980 992 +12
==========================================
+ Hits 462 473 +11
- Misses 508 509 +1
Partials 10 10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| return h.Sum(nil) | ||
| } | ||
|
|
||
| // hashChildren256 avoids allocating a sha256.digest and preimage slice by using a fixed 65-byte stack buffer. |
There was a problem hiding this comment.
I think the allocation of a sha256 digest instance is a red herring, and the wins are coming from having b on the stack.
E.g. if you change the implementation to below (as as yours, but using sha256.New as before), I think you'll still only see 1 alloc:
// hashChildren256 avoids allocating a sha256.digest and preimage slice by using a fixed 65-byte stack buffer.
func hashChildren256(l, r []byte) []byte {
var b [1 + 2*sha256.Size]byte
b[0] = RFC6962NodeHashPrefix
copy(b[1:], l)
copy(b[1+sha256.Size:], r)
h := sha256.New()
h.Write(b[:])
var s [sha256.Size]byte
h.Sum(s[:0])
return s[:]
}There was a problem hiding this comment.
It comes from 2 things. Having b on the stack is a big part, but the subtle part is that it really matters whether you go through an interface or a concrete class. Take a look at https://go.dev/play/p/MUT8x7Jb-7j. If it goes via the interface, then it can't prove to itself that no impl of the function will keep hold of the parameter, and so it escapes to the heap. When it's a concrete type then it can keep it on the stack.
So yeah, your version here will also be a single alloc. But if it called the struct field in the interface, it would alloc everywhere.
There was a problem hiding this comment.
There was a problem hiding this comment.
FWIW, you can make (3) in there be 2 allocs by re-purposing b for the return.
...
h.Sum(b[:0)]
return b[:]It's interesting that the interface scuppers the escape analysis, clearly the compiler knows what's hiding behind the interface, particularly in your playground sketch.
There was a problem hiding this comment.
Anyway, could you add a comment somewhere sensible which just explains why it works, and when it wouldn't (basically C'N'P the para from a couple of comments above)? Otherwise one of us is going to have a headache in a year or two when we're next in here :)
There was a problem hiding this comment.
Yeah fair enough. Added a longer comment about performance when you call through the interface instead of going direct.
| // HashChildren returns the inner Merkle tree node hash of the two child nodes l and r. | ||
| // The hashed structure is NodeHashPrefix||l||r. | ||
| func (t *Hasher) HashChildren(l, r []byte) []byte { | ||
| if t.Hash == crypto.SHA256 && len(l) == sha256.Size && len(r) == sha256.Size { |
There was a problem hiding this comment.
Is it possible to optimize this for any 256 bit hash (t.Hash.Size() == 32), rather than only the specific SHA-256 one?
Optionally, any hash <= 512 bits too. The optimization seems to be about placing the buffer on stack, so it seems like it could be only based on the size and algorithm-agnostic?
There was a problem hiding this comment.
Hey Pavel, good to have the old gang back together for some low level Merkle Go optimization!
Take a look at the reply to Al above, and https://go.dev/play/p/MUT8x7Jb-7j which shows the different approaches. It isn't just the buffer on the stack. It's the interface and virtual function that gets you.
|
Another interesting datapoint - I wondered what would happen if the build was made using
func (t *Hasher) HashChildren2(l, r []byte) []byte {
var b []byte = make([]byte, 1+2*t.Size())
b[0] = RFC6962NodeHashPrefix
copy(b[1:], l)
copy(b[1+len(l):], r)
h := t.New()
h.Write(b)
h.Sum(b[:0])
return b[0:t.Size()]
}
|
3 participants
Avoid allocating a new sha256.digest object and dynamic slice buffers
on every interior node hash when using crypto.SHA256.
In HashChildren, the preimage size is strictly fixed by the RFC6962
specification (1 prefix byte + 2 * 32-byte hashes = 65 bytes).
Using a fixed stack buffer and sha256.Sum256 directly avoids the
sha256.digest allocation and slice resizing, dropping allocations to 1
(the returned slice), instead of the previous 3.
Noted that there is a similar performance optimisation for HashLeaf, but
this one requires an arbitrary threshold so has been left out of scope for
this PR.
Benchmarks (averaged over 5 runs):
BenchmarkHashChildren:
Before: 1282 ns/op, 240 B/op, 3 allocs/op
After: 655 ns/op, 32 B/op, 1 allocs/op (-48.9% ns/op, -86.7% B/op, -66.7% allocs)
compact.BenchmarkAppend (1024 leaves):
Before: 2296773 ns/op, 414202 B/op, 7170 allocs/op
After: 1331209 ns/op, 201417 B/op, 5124 allocs/op (-42.0% ns/op, -51.4% B/op, -28.5% allocs)