From b69e19b2c8765df9b99a9259f786bf76ef4597b9 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 26 Feb 2021 21:03:27 +0100 Subject: [PATCH 01/10] address: adding cache address.String() cache benchmark --- types/address_bench_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/types/address_bench_test.go b/types/address_bench_test.go index 88a7e537a0d2..0784bcddb105 100644 --- a/types/address_bench_test.go +++ b/types/address_bench_test.go @@ -11,6 +11,18 @@ import ( "github.com/cosmos/cosmos-sdk/types" ) +func BenchmarkAccAddressString(b *testing.B) { + b.ReportAllocs() + pkBz := make([]byte, ed25519.PubKeySize) + pk := &ed25519.PubKey{Key: pkBz} + aa := pk.Address() + var str string + for i := 0; i < b.N; i++ { + str = aa.String() + } + require.NotEmpty(b, str) +} + func BenchmarkBech32ifyPubKey(b *testing.B) { b.ReportAllocs() pkBz := make([]byte, ed25519.PubKeySize) From 280168ce6d8e7dfd86bcdc79cdf1c1fa5bf0ff5e Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 26 Feb 2021 21:07:18 +0100 Subject: [PATCH 02/10] address: use LRU cache for .String() --- types/address.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/types/address.go b/types/address.go index 8e6a8103f3b0..5d83cccbd91f 100644 --- a/types/address.go +++ b/types/address.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/address" "github.com/cosmos/cosmos-sdk/types/bech32" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/hashicorp/golang-lru/simplelru" ) const ( @@ -239,37 +240,33 @@ func (aa AccAddress) Bytes() []byte { // AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles, // yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type. -// With the string interning below, we are able to achieve zero cost allocations for string->[]byte -// because the Go compiler recognizes the special case of map[string([]byte)] when used exactly -// in that pattern. See https://github.com/cosmos/cosmos-sdk/issues/8693. var addMu sync.Mutex -var addrStrMemoize = make(map[string]string) +var addrStrMemoize, _ = simplelru.NewLRU(1000, nil) // String implements the Stringer interface. -func (aa AccAddress) String() (str string) { - addMu.Lock() - defer addMu.Unlock() - - if str, ok := addrStrMemoize[string(aa)]; ok { - return str +func (aa AccAddress) String() string { + var key = string(aa) + if str, ok := addrStrMemoize.Get(key); ok { + return str.(string) } - // Otherwise cache it for later memoization. - defer func() { - addrStrMemoize[string(aa)] = str - }() if aa.Empty() { + addMu.Lock() + addrStrMemoize.Add(key, "") + addMu.Unlock() return "" } bech32PrefixAccAddr := GetConfig().GetBech32AccountAddrPrefix() - - bech32Addr, err := bech32.ConvertAndEncode(bech32PrefixAccAddr, aa.Bytes()) + bech32Addr, err := bech32.ConvertAndEncode(bech32PrefixAccAddr, aa) if err != nil { panic(err) } + addMu.Lock() + addrStrMemoize.Add(key, bech32Addr) + addMu.Unlock() return bech32Addr } From 60e6d7184e39a93c1468165e2e6a64149d3af40d Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 26 Feb 2021 21:13:22 +0100 Subject: [PATCH 03/10] optimize address.Empty --- types/address.go | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/types/address.go b/types/address.go index 5d83cccbd91f..0c4609cbb561 100644 --- a/types/address.go +++ b/types/address.go @@ -159,12 +159,7 @@ func (aa AccAddress) Equals(aa2 Address) bool { // Returns boolean for whether an AccAddress is empty func (aa AccAddress) Empty() bool { - if aa == nil { - return true - } - - aa2 := AccAddress{} - return bytes.Equal(aa.Bytes(), aa2.Bytes()) + return aa == nil || len(aa) == 0 } // Marshal returns the raw address bytes. It is needed for protobuf @@ -329,12 +324,7 @@ func (va ValAddress) Equals(va2 Address) bool { // Returns boolean for whether an AccAddress is empty func (va ValAddress) Empty() bool { - if va == nil { - return true - } - - va2 := ValAddress{} - return bytes.Equal(va.Bytes(), va2.Bytes()) + return va == nil || len(va) == 0 } // Marshal returns the raw address bytes. It is needed for protobuf @@ -489,12 +479,7 @@ func (ca ConsAddress) Equals(ca2 Address) bool { // Returns boolean for whether an ConsAddress is empty func (ca ConsAddress) Empty() bool { - if ca == nil { - return true - } - - ca2 := ConsAddress{} - return bytes.Equal(ca.Bytes(), ca2.Bytes()) + return ca == nil || len(ca) == 0 } // Marshal returns the raw address bytes. It is needed for protobuf From 991169afe6993acf891ac1eff24953c8c0c8d9b7 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Sun, 28 Feb 2021 11:03:16 +0100 Subject: [PATCH 04/10] move cache initialization to init function --- types/address.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/types/address.go b/types/address.go index 0c4609cbb561..1875e7f60640 100644 --- a/types/address.go +++ b/types/address.go @@ -73,6 +73,22 @@ const ( Bech32PrefixConsPub = Bech32MainPrefix + PrefixValidator + PrefixConsensus + PrefixPublic ) +// cache variables +var ( + // AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles, + // yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type. + addrMu sync.Mutex + addrCache *simplelru.LRU +) + +func init() { + var err error + addrCache, err = simplelru.NewLRU(1000, nil) + if err != nil { + panic(err) + } +} + // Address is a common interface for different types of addresses used by the SDK type Address interface { Equals(Address) bool @@ -233,23 +249,18 @@ func (aa AccAddress) Bytes() []byte { return aa } -// AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles, -// yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type. -var addMu sync.Mutex -var addrStrMemoize, _ = simplelru.NewLRU(1000, nil) - // String implements the Stringer interface. func (aa AccAddress) String() string { var key = string(aa) - if str, ok := addrStrMemoize.Get(key); ok { + if str, ok := addrCache.Get(key); ok { return str.(string) } // Otherwise cache it for later memoization. if aa.Empty() { - addMu.Lock() - addrStrMemoize.Add(key, "") - addMu.Unlock() + addrMu.Lock() + addrCache.Add(key, "") + addrMu.Unlock() return "" } @@ -259,9 +270,9 @@ func (aa AccAddress) String() string { panic(err) } - addMu.Lock() - addrStrMemoize.Add(key, bech32Addr) - addMu.Unlock() + addrMu.Lock() + addrCache.Add(key, bech32Addr) + addrMu.Unlock() return bech32Addr } From 2294920c765932a4e3587b3259950f7919547363 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 1 Mar 2021 20:58:25 +0100 Subject: [PATCH 05/10] Use UnsafeBytesToStr convertion with Addr cache --- types/address.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/types/address.go b/types/address.go index 1875e7f60640..6573dfe9ba03 100644 --- a/types/address.go +++ b/types/address.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec/legacy" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/internal/conv" "github.com/cosmos/cosmos-sdk/types/address" "github.com/cosmos/cosmos-sdk/types/bech32" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -83,7 +84,7 @@ var ( func init() { var err error - addrCache, err = simplelru.NewLRU(1000, nil) + addrCache, err = simplelru.NewLRU(30000, nil) if err != nil { panic(err) } @@ -251,9 +252,9 @@ func (aa AccAddress) Bytes() []byte { // String implements the Stringer interface. func (aa AccAddress) String() string { - var key = string(aa) - if str, ok := addrCache.Get(key); ok { - return str.(string) + var key = conv.UnsafeBytesToStr(aa) + if addr, ok := addrCache.Get(key); ok { + return addr.(string) } // Otherwise cache it for later memoization. From 7e577b9ed97334eac2bbdb4b6c9b8cc51bc2e4c0 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 1 Mar 2021 21:31:45 +0100 Subject: [PATCH 06/10] add cache for other address String() methods --- types/address.go | 75 +++++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/types/address.go b/types/address.go index 6573dfe9ba03..e1bd2e76fd11 100644 --- a/types/address.go +++ b/types/address.go @@ -78,14 +78,23 @@ const ( var ( // AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles, // yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type. - addrMu sync.Mutex - addrCache *simplelru.LRU + accAddrMu sync.Mutex + accAddrCache *simplelru.LRU + consAddrMu sync.Mutex + consAddrCache *simplelru.LRU + valAddrMu sync.Mutex + valAddrCache *simplelru.LRU ) func init() { var err error - addrCache, err = simplelru.NewLRU(30000, nil) - if err != nil { + if accAddrCache, err = simplelru.NewLRU(25000, nil); err != nil { + panic(err) + } + if consAddrCache, err = simplelru.NewLRU(500, nil); err != nil { + panic(err) + } + if valAddrCache, err = simplelru.NewLRU(500, nil); err != nil { panic(err) } } @@ -252,29 +261,16 @@ func (aa AccAddress) Bytes() []byte { // String implements the Stringer interface. func (aa AccAddress) String() string { - var key = conv.UnsafeBytesToStr(aa) - if addr, ok := addrCache.Get(key); ok { - return addr.(string) - } - // Otherwise cache it for later memoization. - if aa.Empty() { - addrMu.Lock() - addrCache.Add(key, "") - addrMu.Unlock() return "" } - bech32PrefixAccAddr := GetConfig().GetBech32AccountAddrPrefix() - bech32Addr, err := bech32.ConvertAndEncode(bech32PrefixAccAddr, aa) - if err != nil { - panic(err) + var key = conv.UnsafeBytesToStr(aa) + if addr, ok := accAddrCache.Get(key); ok { + return addr.(string) } - - addrMu.Lock() - addrCache.Add(key, bech32Addr) - addrMu.Unlock() - return bech32Addr + return cacheBech32Addr(GetConfig().GetBech32AccountAddrPrefix(), + aa, &accAddrMu, accAddrCache, key) } // Format implements the fmt.Formatter interface. @@ -417,14 +413,12 @@ func (va ValAddress) String() string { return "" } - bech32PrefixValAddr := GetConfig().GetBech32ValidatorAddrPrefix() - - bech32Addr, err := bech32.ConvertAndEncode(bech32PrefixValAddr, va.Bytes()) - if err != nil { - panic(err) + var key = conv.UnsafeBytesToStr(va) + if addr, ok := valAddrCache.Get(key); ok { + return addr.(string) } - - return bech32Addr + return cacheBech32Addr(GetConfig().GetBech32ValidatorAddrPrefix(), + va, &valAddrMu, valAddrCache, key) } // Format implements the fmt.Formatter interface. @@ -572,14 +566,12 @@ func (ca ConsAddress) String() string { return "" } - bech32PrefixConsAddr := GetConfig().GetBech32ConsensusAddrPrefix() - - bech32Addr, err := bech32.ConvertAndEncode(bech32PrefixConsAddr, ca.Bytes()) - if err != nil { - panic(err) + var key = conv.UnsafeBytesToStr(ca) + if addr, ok := consAddrCache.Get(key); ok { + return addr.(string) } - - return bech32Addr + return cacheBech32Addr(GetConfig().GetBech32ConsensusAddrPrefix(), + ca, &consAddrMu, consAddrCache, key) } // Bech32ifyAddressBytes returns a bech32 representation of address bytes. @@ -724,3 +716,14 @@ func addressBytesFromHexString(address string) ([]byte, error) { return hex.DecodeString(address) } + +func cacheBech32Addr(prefix string, addr []byte, m *sync.Mutex, cache *simplelru.LRU, cacheKey string) string { + bech32Addr, err := bech32.ConvertAndEncode(prefix, addr) + if err != nil { + panic(err) + } + m.Lock() + cache.Add(cacheKey, bech32Addr) + m.Unlock() + return bech32Addr +} From 0a3ff3fd8ceb5759f99283acf6fa0edfb71de2f0 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 1 Mar 2021 21:42:17 +0100 Subject: [PATCH 07/10] fix linter issue --- types/address.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/address.go b/types/address.go index e1bd2e76fd11..6004788d7423 100644 --- a/types/address.go +++ b/types/address.go @@ -717,7 +717,7 @@ func addressBytesFromHexString(address string) ([]byte, error) { return hex.DecodeString(address) } -func cacheBech32Addr(prefix string, addr []byte, m *sync.Mutex, cache *simplelru.LRU, cacheKey string) string { +func cacheBech32Addr(prefix string, addr []byte, m sync.Locker, cache *simplelru.LRU, cacheKey string) string { bech32Addr, err := bech32.ConvertAndEncode(prefix, addr) if err != nil { panic(err) From a02c0d4e472b00760fd042d8b11083f8c6907fd9 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 2 Mar 2021 14:55:08 +0100 Subject: [PATCH 08/10] add a non trivial address for Address.String benchmark --- types/address_bench_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/types/address_bench_test.go b/types/address_bench_test.go index 0784bcddb105..14195a9fe64b 100644 --- a/types/address_bench_test.go +++ b/types/address_bench_test.go @@ -15,12 +15,19 @@ func BenchmarkAccAddressString(b *testing.B) { b.ReportAllocs() pkBz := make([]byte, ed25519.PubKeySize) pk := &ed25519.PubKey{Key: pkBz} - aa := pk.Address() - var str string + a := pk.Address() + pk2 := make([]byte, ed25519.PubKeySize) + for i = 1; i <= ed25519.PubKeySize; i++ { + pk2[i] = i + } + a2 := pk.Address() + var str, str2 string for i := 0; i < b.N; i++ { - str = aa.String() + str = a.String() + str2 = a2.String() } require.NotEmpty(b, str) + require.NotEmpty(b, str2) } func BenchmarkBech32ifyPubKey(b *testing.B) { From 4d26f579de0fb8baf204990dd1788adbcccdaed6 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 2 Mar 2021 16:28:51 +0100 Subject: [PATCH 09/10] add comment about cache size and update cashe size to 60k --- types/address.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/types/address.go b/types/address.go index 6004788d7423..d96c2f324739 100644 --- a/types/address.go +++ b/types/address.go @@ -88,7 +88,9 @@ var ( func init() { var err error - if accAddrCache, err = simplelru.NewLRU(25000, nil); err != nil { + // in total the cache size is 61k entries. Key is 32 bytes and value is around 50-70 bytes. + // That will make around 92 * 61k * 2 (LRU) bytes ~ 11 MB + if accAddrCache, err = simplelru.NewLRU(60000, nil); err != nil { panic(err) } if consAddrCache, err = simplelru.NewLRU(500, nil); err != nil { From cbbf8b38129f705253d97ca7be2df3b6fbc219a8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 3 Mar 2021 04:38:07 +0100 Subject: [PATCH 10/10] fix syntax --- types/address_bench_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/types/address_bench_test.go b/types/address_bench_test.go index 14195a9fe64b..377cffad2395 100644 --- a/types/address_bench_test.go +++ b/types/address_bench_test.go @@ -17,8 +17,8 @@ func BenchmarkAccAddressString(b *testing.B) { pk := &ed25519.PubKey{Key: pkBz} a := pk.Address() pk2 := make([]byte, ed25519.PubKeySize) - for i = 1; i <= ed25519.PubKeySize; i++ { - pk2[i] = i + for i := 1; i <= ed25519.PubKeySize; i++ { + pk2[i] = byte(i) } a2 := pk.Address() var str, str2 string