Skip to content

Commit

Permalink
perf: do less allocs in hasher (#287)
Browse files Browse the repository at this point in the history
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 -->
  • Loading branch information
cristaloleg authored Jan 20, 2025
1 parent 5b1b3d8 commit a336f12
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 30 deletions.
31 changes: 11 additions & 20 deletions hasher.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,12 @@ func (n *NmtHasher) BlockSize() int {

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

return digest
digest := make([]byte, zeroSize, fullSize)
return n.baseHasher.Sum(digest)
}

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

// add LeafPrefix to the ndata
leafPrefixedNData := make([]byte, 0, len(ndata)+1)
leafPrefixedNData = append(leafPrefixedNData, LeafPrefix)
leafPrefixedNData = append(leafPrefixedNData, ndata...)
h.Write(leafPrefixedNData)
h.Write([]byte{LeafPrefix})
h.Write(ndata)

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

res := make([]byte, 0, len(minNs)*2)
res := make([]byte, 0, len(minNs)+len(maxNs)+h.Size())
res = append(res, minNs...)
res = append(res, maxNs...)

// Note this seems a little faster than calling several Write()s on the
// underlying Hash function (see:
// https://github.com/google/trillian/pull/1503):
data := make([]byte, 0, 1+len(left)+len(right))
data = append(data, NodePrefix)
data = append(data, left...)
data = append(data, right...)
//nolint:errcheck
h.Write(data)
h.Write([]byte{NodePrefix})
h.Write(left)
h.Write(right)
return h.Sum(res), nil
}

Expand Down
32 changes: 22 additions & 10 deletions hasher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -900,17 +900,29 @@ func TestComputeNsRange(t *testing.T) {

// TestEmptyRoot ensures that the empty root is always the same, under the same configuration, regardless of the state of the Hasher.
func TestEmptyRoot(t *testing.T) {
nIDSzie := 1
ignoreMaxNS := true
t.Run("the empty root should match a hard-coded empty root", func(t *testing.T) {
nIDSize := 1
ignoreMaxNs := true

hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs)
got := hasher.EmptyRoot()
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}
assert.Equal(t, want, got)
})

t.Run("empty root should return the same root even if the hasher is modified", func(t *testing.T) {
nIDSize := 1
ignoreMaxNs := true

hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS)
expectedEmptyRoot := hasher.EmptyRoot()
hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs)
want := hasher.EmptyRoot()

// perform some operation with the hasher
_, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1))
assert.NoError(t, err)
gotEmptyRoot := hasher.EmptyRoot()
// perform some operation with the hasher
_, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1))
assert.NoError(t, err)
got := hasher.EmptyRoot()

// the empty root should be the same before and after the operation
assert.True(t, bytes.Equal(gotEmptyRoot, expectedEmptyRoot))
// the empty root should be the same before and after the operation
assert.Equal(t, want, got)
})
}

0 comments on commit a336f12

Please sign in to comment.