Skip to content

Commit

Permalink
x/ibc/core/23-commitment/types: fix MerkleProof.Empty comparisons (#8092
Browse files Browse the repository at this point in the history
)

* x/ibc/core/23-commitment/types: fix MerkleProof.Empty comparisons

Fixes invalid pointer creation, reduces on unnecessary allocations
inside MerkleProof.Empty, and changes the method to a pointer; the
invalid pointer creation was symptomatic of broad use of values.

With this change we have improvements whose benchmarks produce:

```shell
name     old time/op    new time/op    delta
Empty-8     311ns ± 5%     232ns ± 5%  -25.49%  (p=0.000 n=20+19)

name     old alloc/op   new alloc/op   delta
Empty-8     56.0B ± 0%      8.0B ± 0%  -85.71%  (p=0.000 n=20+20)

name     old allocs/op  new allocs/op  delta
Empty-8      3.00 ± 0%      1.00 ± 0%  -66.67%  (p=0.000 n=20+20)
```

Fixes #8091

* Move Empty godoc to right place + add comments for blank*
  • Loading branch information
odeke-em authored Dec 7, 2020
1 parent d4a919b commit 291d966
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
15 changes: 15 additions & 0 deletions x/ibc/core/23-commitment/types/bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package types

import (
"testing"
)

func BenchmarkMerkleProofEmpty(b *testing.B) {
b.ReportAllocs()
var mk MerkleProof
for i := 0; i < b.N; i++ {
if !mk.Empty() {
b.Fatal("supposed to be empty")
}
}
}
9 changes: 7 additions & 2 deletions x/ibc/core/23-commitment/types/merkle.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,14 @@ func verifyChainedMembershipProof(root []byte, specs []*ics23.ProofSpec, proofs
return nil
}

// blankMerkleProof and blankProofOps will be used to compare against their zero values,
// and are declared as globals to avoid having to unnecessarily re-allocate on every comparison.
var blankMerkleProof = &MerkleProof{}
var blankProofOps = &tmcrypto.ProofOps{}

// Empty returns true if the root is empty
func (proof MerkleProof) Empty() bool {
return proto.Equal(&proof, nil) || proto.Equal(&proof, &MerkleProof{}) || proto.Equal(&proof, &tmcrypto.ProofOps{})
func (proof *MerkleProof) Empty() bool {
return proof == nil || proto.Equal(proof, blankMerkleProof) || proto.Equal(proof, blankProofOps)
}

// ValidateBasic checks if the proof is empty.
Expand Down

0 comments on commit 291d966

Please sign in to comment.