Skip to content

Commit a336f12

Browse files
authored
perf: do less allocs in hasher (#287)
Have found few places after #285 that can be improved a bit more without going into enigma mode™: ``` $ go-perftuner bstat a.txt c.txt args: [a.txt c.txt]name old time/op new time/op delta ComputeRoot/64-leaves-10 37.5µs ± 1% 34.2µs ± 2% -8.76% (p=0.000 n=8+10) ComputeRoot/128-leaves-10 73.9µs ± 0% 68.6µs ± 2% -7.21% (p=0.000 n=9+10) ComputeRoot/256-leaves-10 151µs ± 2% 140µs ± 4% -6.91% (p=0.000 n=9+10) ComputeRoot/20k-leaves-10 14.9ms ± 2% 13.2ms ± 4% -11.53% (p=0.000 n=10+10) name old alloc/op new alloc/op delta ComputeRoot/64-leaves-10 57.6kB ± 0% 31.7kB ± 0% -45.02% (p=0.000 n=10+10) ComputeRoot/128-leaves-10 109kB ± 0% 57kB ± 0% -47.83% (p=0.000 n=10+10) ComputeRoot/256-leaves-10 224kB ± 0% 120kB ± 0% -46.45% (p=0.000 n=10+10) ComputeRoot/20k-leaves-10 26.2MB ± 0% 12.3MB ± 0% -53.11% (p=0.000 n=9+10) name old allocs/op new allocs/op delta ComputeRoot/64-leaves-10 590 ± 0% 527 ± 0% -10.68% (p=0.000 n=10+10) ComputeRoot/128-leaves-10 1.17k ± 0% 1.04k ± 0% -10.86% (p=0.000 n=10+10) ComputeRoot/256-leaves-10 2.33k ± 0% 2.07k ± 0% -10.95% (p=0.000 n=10+10) ComputeRoot/20k-leaves-10 181k ± 0% 161k ± 0% -11.08% (p=0.000 n=10+10) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Optimized hashing methods for improved efficiency. - Simplified data preparation for leaf and node hashing. - Reduced unnecessary memory allocations in hash calculations. - **Tests** - Enhanced the `TestEmptyRoot` function with sub-tests for better validation. - Improved variable naming for clarity in test cases. - Updated assertions for better readability in test results. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 5b1b3d8 commit a336f12

File tree

2 files changed

+33
-30
lines changed

2 files changed

+33
-30
lines changed

hasher.go

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,12 @@ func (n *NmtHasher) BlockSize() int {
154154

155155
func (n *NmtHasher) EmptyRoot() []byte {
156156
n.baseHasher.Reset()
157-
h := n.baseHasher.Sum(nil)
158-
digest := append(make([]byte, int(n.NamespaceLen)*2), h...)
157+
// make returns a zeroed slice, exactly what we need for the (nID || nID)
158+
zeroSize := int(n.NamespaceLen) * 2
159+
fullSize := zeroSize + n.baseHasher.Size()
159160

160-
return digest
161+
digest := make([]byte, zeroSize, fullSize)
162+
return n.baseHasher.Sum(digest)
161163
}
162164

163165
// ValidateLeaf verifies if data is namespaced and returns an error if not.
@@ -174,8 +176,6 @@ func (n *NmtHasher) ValidateLeaf(data []byte) (err error) {
174176
// ns(ndata) || ns(ndata) || hash(leafPrefix || ndata), where ns(ndata) is the
175177
// namespaceID inside the data item namely leaf[:n.NamespaceLen]). Note that for
176178
// leaves minNs = maxNs = ns(leaf) = leaf[:NamespaceLen]. HashLeaf can return the ErrInvalidNodeLen error if the input is not namespaced.
177-
//
178-
//nolint:errcheck
179179
func (n *NmtHasher) HashLeaf(ndata []byte) ([]byte, error) {
180180
h := n.baseHasher
181181
h.Reset()
@@ -190,11 +190,8 @@ func (n *NmtHasher) HashLeaf(ndata []byte) ([]byte, error) {
190190
minMaxNIDs = append(minMaxNIDs, nID...) // nID
191191
minMaxNIDs = append(minMaxNIDs, nID...) // nID || nID
192192

193-
// add LeafPrefix to the ndata
194-
leafPrefixedNData := make([]byte, 0, len(ndata)+1)
195-
leafPrefixedNData = append(leafPrefixedNData, LeafPrefix)
196-
leafPrefixedNData = append(leafPrefixedNData, ndata...)
197-
h.Write(leafPrefixedNData)
193+
h.Write([]byte{LeafPrefix})
194+
h.Write(ndata)
198195

199196
// compute h(LeafPrefix || ndata) and append it to the minMaxNIDs
200197
nameSpacedHash := h.Sum(minMaxNIDs) // nID || nID || h(LeafPrefix || ndata)
@@ -306,19 +303,13 @@ func (n *NmtHasher) HashNode(left, right []byte) ([]byte, error) {
306303
// compute the namespace range of the parent node
307304
minNs, maxNs := computeNsRange(lRange.Min, lRange.Max, rRange.Min, rRange.Max, n.ignoreMaxNs, n.precomputedMaxNs)
308305

309-
res := make([]byte, 0, len(minNs)*2)
306+
res := make([]byte, 0, len(minNs)+len(maxNs)+h.Size())
310307
res = append(res, minNs...)
311308
res = append(res, maxNs...)
312309

313-
// Note this seems a little faster than calling several Write()s on the
314-
// underlying Hash function (see:
315-
// https://github.com/google/trillian/pull/1503):
316-
data := make([]byte, 0, 1+len(left)+len(right))
317-
data = append(data, NodePrefix)
318-
data = append(data, left...)
319-
data = append(data, right...)
320-
//nolint:errcheck
321-
h.Write(data)
310+
h.Write([]byte{NodePrefix})
311+
h.Write(left)
312+
h.Write(right)
322313
return h.Sum(res), nil
323314
}
324315

hasher_test.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -900,17 +900,29 @@ func TestComputeNsRange(t *testing.T) {
900900

901901
// TestEmptyRoot ensures that the empty root is always the same, under the same configuration, regardless of the state of the Hasher.
902902
func TestEmptyRoot(t *testing.T) {
903-
nIDSzie := 1
904-
ignoreMaxNS := true
903+
t.Run("the empty root should match a hard-coded empty root", func(t *testing.T) {
904+
nIDSize := 1
905+
ignoreMaxNs := true
906+
907+
hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs)
908+
got := hasher.EmptyRoot()
909+
want := []byte{0x0, 0x0, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}
910+
assert.Equal(t, want, got)
911+
})
912+
913+
t.Run("empty root should return the same root even if the hasher is modified", func(t *testing.T) {
914+
nIDSize := 1
915+
ignoreMaxNs := true
905916

906-
hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS)
907-
expectedEmptyRoot := hasher.EmptyRoot()
917+
hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs)
918+
want := hasher.EmptyRoot()
908919

909-
// perform some operation with the hasher
910-
_, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1))
911-
assert.NoError(t, err)
912-
gotEmptyRoot := hasher.EmptyRoot()
920+
// perform some operation with the hasher
921+
_, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1))
922+
assert.NoError(t, err)
923+
got := hasher.EmptyRoot()
913924

914-
// the empty root should be the same before and after the operation
915-
assert.True(t, bytes.Equal(gotEmptyRoot, expectedEmptyRoot))
925+
// the empty root should be the same before and after the operation
926+
assert.Equal(t, want, got)
927+
})
916928
}

0 commit comments

Comments
 (0)