From 8147174eaa6b5d7b6cbd1966344fc3a984a41620 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 14 Apr 2025 02:19:56 +0300 Subject: [PATCH 1/2] perf(crypto): simplify and speed up Address.Compare MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takes advantage of knowledge that Address is simply an array derived from []byte and Go's rules allow us to address each with [:] to find []byte and pass those into bytes.Compare instead of the unnecessarily inefficient make, copy. crypto.Address is very fundamental and a building block to Gno and all of cryptocurrencies, thus making its operations much more efficient is necessary. While here also made Address.Compare take a pointer receiver which reduces the need to make a stack copy of the receiver and with that we have this speed up while making the code more succinct and direct and more idiomatic ```shell $ benchstat before.txt after.txt name old time/op new time/op delta AddressCompare-8 79.5ns ± 1% 52.9ns ± 3% -33.54% (p=0.000 n=10+9) name old alloc/op new alloc/op delta AddressCompare-8 8.00B ± 0% 8.00B ± 0% ~ (all equal) name old allocs/op new allocs/op delta AddressCompare-8 1.00 ± 0% 1.00 ± 0% ~ (all equal) ``` Fixes #4129 --- gno.land/cmd/gnoland/secrets_common.go | 2 +- tm2/pkg/bft/consensus/common_test.go | 3 ++- tm2/pkg/bft/types/priv_validator.go | 4 +-- tm2/pkg/crypto/bench_test.go | 29 ++++++++++++++++++++++ tm2/pkg/crypto/crypto.go | 8 ++---- tm2/pkg/crypto/keys/client/add_multisig.go | 3 ++- 6 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 tm2/pkg/crypto/bench_test.go diff --git a/gno.land/cmd/gnoland/secrets_common.go b/gno.land/cmd/gnoland/secrets_common.go index 500336e3489..4ae6c1c709a 100644 --- a/gno.land/cmd/gnoland/secrets_common.go +++ b/gno.land/cmd/gnoland/secrets_common.go @@ -89,7 +89,7 @@ func validateValidatorKey(key *privval.FilePVKey) error { // Make sure the address is derived // from the public key - if key.PubKey.Address().Compare(key.Address) != 0 { + if pk := key.PubKey.Address(); pk.Compare(key.Address) != 0 { return errAddressMismatch } diff --git a/tm2/pkg/bft/consensus/common_test.go b/tm2/pkg/bft/consensus/common_test.go index 41c1b21a1f5..9e924145828 100644 --- a/tm2/pkg/bft/consensus/common_test.go +++ b/tm2/pkg/bft/consensus/common_test.go @@ -128,7 +128,8 @@ func (vss ValidatorStubsByAddress) Len() int { } func (vss ValidatorStubsByAddress) Less(i, j int) bool { - return vss[i].GetPubKey().Address().Compare(vss[j].GetPubKey().Address()) == -1 + vai := vss[i].GetPubKey().Address() + return vai.Compare(vss[j].GetPubKey().Address()) == -1 } func (vss ValidatorStubsByAddress) Swap(i, j int) { diff --git a/tm2/pkg/bft/types/priv_validator.go b/tm2/pkg/bft/types/priv_validator.go index 46da2aa83df..c3ce231c254 100644 --- a/tm2/pkg/bft/types/priv_validator.go +++ b/tm2/pkg/bft/types/priv_validator.go @@ -28,8 +28,8 @@ func (pvs PrivValidatorsByAddress) Len() int { } func (pvs PrivValidatorsByAddress) Less(i, j int) bool { - return pvs[i].GetPubKey().Address().Compare( - pvs[j].GetPubKey().Address()) == -1 + pai := pvs[i].GetPubKey().Address() + return pai.Compare(pvs[j].GetPubKey().Address()) == -1 } func (pvs PrivValidatorsByAddress) Swap(i, j int) { diff --git a/tm2/pkg/crypto/bench_test.go b/tm2/pkg/crypto/bench_test.go new file mode 100644 index 00000000000..209bfe77ba9 --- /dev/null +++ b/tm2/pkg/crypto/bench_test.go @@ -0,0 +1,29 @@ +package crypto + +import ( + "crypto/rand" + "testing" +) + +var sink any = nil + +func BenchmarkAddressCompare(b *testing.B) { + var addr1, addr2 Address + rand.Read(addr1[:]) + rand.Read(addr2[:]) + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + sink = addr1.Compare(addr1) + sink = addr1.Compare(addr2) + sink = addr2.Compare(addr2) + sink = addr2.Compare(addr1) + } + + if sink == nil { + b.Fatal("Benchmark did not run!") + } + + sink = nil +} diff --git a/tm2/pkg/crypto/crypto.go b/tm2/pkg/crypto/crypto.go index 7908a082d3b..ea476c36650 100644 --- a/tm2/pkg/crypto/crypto.go +++ b/tm2/pkg/crypto/crypto.go @@ -85,12 +85,8 @@ func (addr *Address) UnmarshalAmino(b32str string) (err error) { return nil } -func (addr Address) Compare(other Address) int { - bz1 := make([]byte, len(addr)) - bz2 := make([]byte, len(other)) - copy(bz1, addr[:]) - copy(bz2, other[:]) - return bytes.Compare(bz1, bz2) +func (addr *Address) Compare(other Address) int { + return bytes.Compare(addr[:], other[:]) } func (addr Address) IsZero() bool { diff --git a/tm2/pkg/crypto/keys/client/add_multisig.go b/tm2/pkg/crypto/keys/client/add_multisig.go index 39b90571143..db720196ac0 100644 --- a/tm2/pkg/crypto/keys/client/add_multisig.go +++ b/tm2/pkg/crypto/keys/client/add_multisig.go @@ -120,7 +120,8 @@ func execAddMultisig(cfg *AddMultisigCfg, args []string, io commands.IO) error { // Check if the keys should be sorted if !cfg.NoSort { sort.Slice(publicKeys, func(i, j int) bool { - return publicKeys[i].Address().Compare(publicKeys[j].Address()) < 0 + pki := publicKeys[i].Address() + return pki.Compare(publicKeys[j].Address()) < 0 }) } From 47e1195fed2883360f6d44d511ad2e14a0b6c6a8 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 14 Apr 2025 17:04:56 +0300 Subject: [PATCH 2/2] Turn Compare argument into pointer too for more efficiency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The updated benchmark then becomes ```shell name old time/op new time/op delta AddressCompare-8 79.5ns ± 1% 33.3ns ± 8% -58.17% (p=0.000 n=10+9) name old alloc/op new alloc/op delta AddressCompare-8 8.00B ± 0% 8.00B ± 0% ~ (all equal) name old allocs/op new allocs/op delta AddressCompare-8 1.00 ± 0% 1.00 ± 0% ~ (all equal) ``` --- contribs/gnodev/pkg/address/book.go | 4 ++-- contribs/gnodev/pkg/address/book_test.go | 8 ++++---- gno.land/cmd/gnoland/secrets_common.go | 2 +- tm2/pkg/bft/consensus/common_test.go | 3 ++- tm2/pkg/bft/types/priv_validator.go | 3 ++- tm2/pkg/bft/types/validator.go | 2 +- tm2/pkg/bft/types/validator_set.go | 8 ++++---- tm2/pkg/bft/types/validator_set_test.go | 2 +- tm2/pkg/crypto/bench_test.go | 8 ++++---- tm2/pkg/crypto/crypto.go | 2 +- tm2/pkg/crypto/keys/client/add_multisig.go | 3 ++- 11 files changed, 24 insertions(+), 21 deletions(-) diff --git a/contribs/gnodev/pkg/address/book.go b/contribs/gnodev/pkg/address/book.go index 983fced5882..0be6137cf48 100644 --- a/contribs/gnodev/pkg/address/book.go +++ b/contribs/gnodev/pkg/address/book.go @@ -51,7 +51,7 @@ func (bk *Book) Add(addr crypto.Address, name string) { } // Check if the association already exist - if oldAddr.Compare(addr) == 0 { + if oldAddr.Compare(&addr) == 0 { return // nothing to do } @@ -87,7 +87,7 @@ func (bk Book) List() []Entry { // Find the correct place to insert newEntry using binary search. i := sort.Search(len(entries), func(i int) bool { - return entries[i].Address.Compare(newEntry.Address) >= 0 + return entries[i].Address.Compare(&newEntry.Address) >= 0 }) entries = append(entries[:i], append([]Entry{newEntry}, entries[i:]...)...) diff --git a/contribs/gnodev/pkg/address/book_test.go b/contribs/gnodev/pkg/address/book_test.go index 4ae0552a806..6beff97c423 100644 --- a/contribs/gnodev/pkg/address/book_test.go +++ b/contribs/gnodev/pkg/address/book_test.go @@ -47,7 +47,7 @@ func TestAdd(t *testing.T) { t.Run("get by name", func(t *testing.T) { addrFromName, ok := bk.GetByName("testname") assert.True(t, ok) - assert.True(t, addrFromName.Compare(testAddr) == 0) + assert.True(t, addrFromName.Compare(&testAddr) == 0) }) // Add same address with a new name @@ -59,7 +59,7 @@ func TestAdd(t *testing.T) { require.True(t, ok) addr2, ok := bk.GetByName("testname2") require.True(t, ok) - assert.True(t, addr1.Compare(addr2) == 0) + assert.True(t, addr1.Compare(&addr2) == 0) }) } @@ -75,7 +75,7 @@ func TestList(t *testing.T) { assert.Equal(t, 1, len(entries)) entry := entries[0] - assert.True(t, testAddr.Compare(entry.Address) == 0) + assert.True(t, testAddr.Compare(&entry.Address) == 0) assert.Equal(t, 1, len(entries[0].Names)) assert.Equal(t, "testname", entries[0].Names[0]) } @@ -103,7 +103,7 @@ func TestGetFromNameOrAddress(t *testing.T) { require.True(t, ok) require.Len(t, names, 1) assert.Equal(t, "testname", names[0]) - assert.True(t, resultAddr.Compare(testAddr) == 0) + assert.True(t, resultAddr.Compare(&testAddr) == 0) }) } } diff --git a/gno.land/cmd/gnoland/secrets_common.go b/gno.land/cmd/gnoland/secrets_common.go index 4ae6c1c709a..ba040dec6b7 100644 --- a/gno.land/cmd/gnoland/secrets_common.go +++ b/gno.land/cmd/gnoland/secrets_common.go @@ -89,7 +89,7 @@ func validateValidatorKey(key *privval.FilePVKey) error { // Make sure the address is derived // from the public key - if pk := key.PubKey.Address(); pk.Compare(key.Address) != 0 { + if pk := key.PubKey.Address(); pk.Compare(&key.Address) != 0 { return errAddressMismatch } diff --git a/tm2/pkg/bft/consensus/common_test.go b/tm2/pkg/bft/consensus/common_test.go index 9e924145828..bfce9c6d164 100644 --- a/tm2/pkg/bft/consensus/common_test.go +++ b/tm2/pkg/bft/consensus/common_test.go @@ -129,7 +129,8 @@ func (vss ValidatorStubsByAddress) Len() int { func (vss ValidatorStubsByAddress) Less(i, j int) bool { vai := vss[i].GetPubKey().Address() - return vai.Compare(vss[j].GetPubKey().Address()) == -1 + vaj := vss[j].GetPubKey().Address() + return vai.Compare(&vaj) == -1 } func (vss ValidatorStubsByAddress) Swap(i, j int) { diff --git a/tm2/pkg/bft/types/priv_validator.go b/tm2/pkg/bft/types/priv_validator.go index c3ce231c254..91ac1be9187 100644 --- a/tm2/pkg/bft/types/priv_validator.go +++ b/tm2/pkg/bft/types/priv_validator.go @@ -29,7 +29,8 @@ func (pvs PrivValidatorsByAddress) Len() int { func (pvs PrivValidatorsByAddress) Less(i, j int) bool { pai := pvs[i].GetPubKey().Address() - return pai.Compare(pvs[j].GetPubKey().Address()) == -1 + paj := pvs[j].GetPubKey().Address() + return pai.Compare(&paj) == -1 } func (pvs PrivValidatorsByAddress) Swap(i, j int) { diff --git a/tm2/pkg/bft/types/validator.go b/tm2/pkg/bft/types/validator.go index 34d11223b4a..7594c1e6d5d 100644 --- a/tm2/pkg/bft/types/validator.go +++ b/tm2/pkg/bft/types/validator.go @@ -58,7 +58,7 @@ func (v *Validator) CompareProposerPriority(other *Validator) *Validator { case v.ProposerPriority < other.ProposerPriority: return other default: - result := v.Address.Compare(other.Address) + result := v.Address.Compare(&other.Address) switch { case result < 0: return v diff --git a/tm2/pkg/bft/types/validator_set.go b/tm2/pkg/bft/types/validator_set.go index 368da86d66a..48b487d0abc 100644 --- a/tm2/pkg/bft/types/validator_set.go +++ b/tm2/pkg/bft/types/validator_set.go @@ -223,7 +223,7 @@ func (vals *ValidatorSet) Copy() *ValidatorSet { // otherwise. func (vals *ValidatorSet) HasAddress(address crypto.Address) bool { idx := sort.Search(len(vals.Validators), func(i int) bool { - return address.Compare(vals.Validators[i].Address) <= 0 + return address.Compare(&vals.Validators[i].Address) <= 0 }) return idx < len(vals.Validators) && vals.Validators[idx].Address == address } @@ -232,7 +232,7 @@ func (vals *ValidatorSet) HasAddress(address crypto.Address) bool { // itself if found. Otherwise, -1 and nil are returned. func (vals *ValidatorSet) GetByAddress(address crypto.Address) (index int, val *Validator) { idx := sort.Search(len(vals.Validators), func(i int) bool { - return address.Compare(vals.Validators[i].Address) <= 0 + return address.Compare(&vals.Validators[i].Address) <= 0 }) if idx < len(vals.Validators) && vals.Validators[idx].Address == address { return idx, vals.Validators[idx].Copy() @@ -440,7 +440,7 @@ func (vals *ValidatorSet) applyUpdates(updates []*Validator) { i := 0 for len(existing) > 0 && len(updates) > 0 { - if existing[0].Address.Compare(updates[0].Address) < 0 { // unchanged validator + if existing[0].Address.Compare(&updates[0].Address) < 0 { // unchanged validator merged[i] = existing[0] existing = existing[1:] } else { @@ -813,7 +813,7 @@ func (valz ValidatorsByAddress) Len() int { } func (valz ValidatorsByAddress) Less(i, j int) bool { - return valz[i].Address.Compare(valz[j].Address) == -1 + return valz[i].Address.Compare(&valz[j].Address) == -1 } func (valz ValidatorsByAddress) Swap(i, j int) { diff --git a/tm2/pkg/bft/types/validator_set_test.go b/tm2/pkg/bft/types/validator_set_test.go index f343655e575..8611eac8063 100644 --- a/tm2/pkg/bft/types/validator_set_test.go +++ b/tm2/pkg/bft/types/validator_set_test.go @@ -1319,7 +1319,7 @@ func (valz validatorsByPriority) Less(i, j int) bool { if valz[i].ProposerPriority > valz[j].ProposerPriority { return false } - return valz[i].Address.Compare(valz[j].Address) < 0 + return valz[i].Address.Compare(&valz[j].Address) < 0 } func (valz validatorsByPriority) Swap(i, j int) { diff --git a/tm2/pkg/crypto/bench_test.go b/tm2/pkg/crypto/bench_test.go index 209bfe77ba9..cff831b3ad3 100644 --- a/tm2/pkg/crypto/bench_test.go +++ b/tm2/pkg/crypto/bench_test.go @@ -15,10 +15,10 @@ func BenchmarkAddressCompare(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - sink = addr1.Compare(addr1) - sink = addr1.Compare(addr2) - sink = addr2.Compare(addr2) - sink = addr2.Compare(addr1) + sink = addr1.Compare(&addr1) + sink = addr1.Compare(&addr2) + sink = addr2.Compare(&addr2) + sink = addr2.Compare(&addr1) } if sink == nil { diff --git a/tm2/pkg/crypto/crypto.go b/tm2/pkg/crypto/crypto.go index ea476c36650..7f4cee742d5 100644 --- a/tm2/pkg/crypto/crypto.go +++ b/tm2/pkg/crypto/crypto.go @@ -85,7 +85,7 @@ func (addr *Address) UnmarshalAmino(b32str string) (err error) { return nil } -func (addr *Address) Compare(other Address) int { +func (addr *Address) Compare(other *Address) int { return bytes.Compare(addr[:], other[:]) } diff --git a/tm2/pkg/crypto/keys/client/add_multisig.go b/tm2/pkg/crypto/keys/client/add_multisig.go index db720196ac0..5e6ca13a1a0 100644 --- a/tm2/pkg/crypto/keys/client/add_multisig.go +++ b/tm2/pkg/crypto/keys/client/add_multisig.go @@ -121,7 +121,8 @@ func execAddMultisig(cfg *AddMultisigCfg, args []string, io commands.IO) error { if !cfg.NoSort { sort.Slice(publicKeys, func(i, j int) bool { pki := publicKeys[i].Address() - return pki.Compare(publicKeys[j].Address()) < 0 + pkj := publicKeys[j].Address() + return pki.Compare(&pkj) < 0 }) }