Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: add cache to address codec #20122

Merged
merged 23 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i

### Improvements

* (codec) [#20122](https://github.com/cosmos/cosmos-sdk/pull/20122) Added a cache to address codec.
* (types) [#19869](https://github.com/cosmos/cosmos-sdk/pull/19869) Removed `Any` type from `codec/types` and replaced it with an alias for `cosmos/gogoproto/types/any`.
* (server) [#19854](https://github.com/cosmos/cosmos-sdk/pull/19854) Add customizability to start command.
* Add `StartCmdOptions` in `server.AddCommands` instead of `servertypes.ModuleInitFlags`. To set custom flags set them in the `StartCmdOptions` struct on the `AddFlags` field.
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
109 changes: 107 additions & 2 deletions codec/address/bech32_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,92 @@ package address
import (
"errors"
"strings"
"sync"
"sync/atomic"

"github.com/hashicorp/golang-lru/simplelru"

"cosmossdk.io/core/address"
errorsmod "cosmossdk.io/errors"

"github.com/cosmos/cosmos-sdk/internal/conv"
sdkAddress "github.com/cosmos/cosmos-sdk/types/address"
"github.com/cosmos/cosmos-sdk/types/bech32"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

const (
// TODO: ideally sdk.GetBech32PrefixValAddr("") should be used but currently there's a cyclical import.
// Once globals are deleted the cyclical import won't happen.
suffixValAddr = "valoper"
suffixConsAddr = "valcons"
)

// cache variables
var (
accAddrMu sync.Mutex
Copy link
Member

@kocubinski kocubinski May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to avoid globals, we should consider adding a NewBech32CodecWithOptions constructor which keeps references to the cache in the codec struct. This would (I think) solve the problem of what looks like 3 global caches; we'd instead of 3 instances of an address codec in the application. This is easily done with depinject but can be handled manually too.

accAddrCache *simplelru.LRU
consAddrMu sync.Mutex
consAddrCache *simplelru.LRU
valAddrMu sync.Mutex
valAddrCache *simplelru.LRU

isCachingEnabled atomic.Bool
)

func init() {
var err error
isCachingEnabled.Store(true)
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

// 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 {
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
panic(err)
}
if consAddrCache, err = simplelru.NewLRU(500, nil); err != nil {
panic(err)
}
if valAddrCache, err = simplelru.NewLRU(500, nil); err != nil {
panic(err)
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
}

type Bech32Codec struct {
Bech32Prefix string
}

var _ address.Codec = &Bech32Codec{}
type cachedBech32Codec struct {
codec Bech32Codec
mu *sync.Mutex
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
cache *simplelru.LRU
}

var (
_ address.Codec = &Bech32Codec{}
_ address.Codec = &cachedBech32Codec{}
)

func NewBech32Codec(prefix string) address.Codec {
return Bech32Codec{prefix}
ac := Bech32Codec{prefix}
if !isCachingEnabled.Load() {
return ac
}

lru := accAddrCache
mu := &accAddrMu
if strings.HasSuffix(prefix, suffixValAddr) {
lru = valAddrCache
mu = &valAddrMu
} else if strings.HasSuffix(prefix, suffixConsAddr) {
lru = consAddrCache
mu = &consAddrMu
}

return cachedBech32Codec{
codec: ac,
cache: lru,
mu: mu,
}
}

// StringToBytes encodes text to bytes
Expand Down Expand Up @@ -61,3 +130,39 @@ func (bc Bech32Codec) BytesToString(bz []byte) (string, error) {

return text, nil
}

func (cbc cachedBech32Codec) BytesToString(bz []byte) (string, error) {
// caches prefix is added to the key to make sure keys are unique in case codecs with different bech32 prefix are defined.
key := cbc.codec.Bech32Prefix + conv.UnsafeBytesToStr(bz)
cbc.mu.Lock()
defer cbc.mu.Unlock()

if addr, ok := cbc.cache.Get(key); ok {
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
return addr.(string), nil
}

addr, err := cbc.codec.BytesToString(bz)
if err != nil {
return "", err
}
cbc.cache.Add(key, addr)

return addr, nil
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

func (cbc cachedBech32Codec) StringToBytes(text string) ([]byte, error) {
cbc.mu.Lock()
defer cbc.mu.Unlock()

if addr, ok := cbc.cache.Get(text); ok {
return addr.([]byte), nil
}

addr, err := cbc.codec.StringToBytes(text)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 it can be worth to cache failures, too. Some benchmarks would be interesting.

}
cbc.cache.Add(text, addr)

return addr, nil
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
140 changes: 140 additions & 0 deletions codec/address/bech32_codec_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package address

import (
"encoding/binary"
"testing"
"time"

"github.com/hashicorp/golang-lru/simplelru"
"gotest.tools/v3/assert"
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

"cosmossdk.io/core/address"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/internal/conv"
)

func TestNewBech32Codec(t *testing.T) {
tests := []struct {
name string
prefix string
lru *simplelru.LRU
address string
}{
{
name: "create accounts cached bech32 codec",
prefix: "cosmos",
lru: accAddrCache,
address: "cosmos1p8s0p6gqc6c9gt77lgr2qqujz49huhu6a80smx",
},
{
name: "create validator cached bech32 codec",
prefix: "cosmosvaloper",
lru: valAddrCache,
address: "cosmosvaloper1sjllsnramtg3ewxqwwrwjxfgc4n4ef9u2lcnj0",
},
{
name: "create consensus cached bech32 codec",
prefix: "cosmosvalcons",
lru: consAddrCache,
address: "cosmosvalcons1ntk8eualewuprz0gamh8hnvcem2nrcdsgz563h",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.lru.Len(), 0)
ac := NewBech32Codec(tt.prefix)
cached, ok := ac.(cachedBech32Codec)
assert.Assert(t, ok)
assert.Equal(t, cached.cache, tt.lru)

addr, err := ac.StringToBytes(tt.address)
assert.NilError(t, err)
assert.Equal(t, tt.lru.Len(), 1)

cachedAddr, ok := tt.lru.Get(tt.address)
assert.Assert(t, ok)
assert.DeepEqual(t, addr, cachedAddr)

accAddr, err := ac.BytesToString(addr)
assert.NilError(t, err)
assert.Equal(t, tt.lru.Len(), 2)

cachedStrAddr, ok := tt.lru.Get(conv.UnsafeBytesToStr(addr))
assert.Assert(t, ok)
assert.DeepEqual(t, accAddr, cachedStrAddr)
})
}
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

func TestMultipleBech32Codec(t *testing.T) {
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
cosmosAc, ok := NewBech32Codec("cosmos").(cachedBech32Codec)
assert.Assert(t, ok)
stakeAc := NewBech32Codec("stake").(cachedBech32Codec)
assert.Assert(t, ok)
assert.Equal(t, cosmosAc.cache, stakeAc.cache)

addr := secp256k1.GenPrivKey().PubKey().Address()
cosmosAddr, err := cosmosAc.BytesToString(addr)
assert.NilError(t, err)
stakeAddr, err := stakeAc.BytesToString(addr)
assert.NilError(t, err)
assert.Assert(t, cosmosAddr != stakeAddr)

cachedCosmosAddr, err := cosmosAc.BytesToString(addr)
assert.NilError(t, err)
assert.Equal(t, cosmosAddr, cachedCosmosAddr)

cachedStakeAddr, err := stakeAc.BytesToString(addr)
assert.NilError(t, err)
assert.Equal(t, stakeAddr, cachedStakeAddr)

}

func TestBech32CodecRace(t *testing.T) {
ac := NewBech32Codec("cosmos")

workers := 4
done := make(chan bool, workers)
cancel := make(chan bool)

for i := byte(1); i <= 2; i++ { // works which will loop in first 100 addresses
go bytesToStringCaller(t, ac, i, 100, cancel, done)
}

for i := byte(1); i <= 2; i++ { // works which will generate 1e6 new addresses
go bytesToStringCaller(t, ac, i, 1000000, cancel, done)
}

<-time.After(time.Millisecond * 30)
close(cancel)

// cleanup
for i := 0; i < 4; i++ {
<-done
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
}
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved

// generates AccAddress calling BytesToString
func bytesToStringCaller(t *testing.T, ac address.Codec, prefix byte, max uint32, cancel chan bool, done chan<- bool) {
t.Helper()

bz := make([]byte, 5) // prefix + 4 bytes for uint
bz[0] = prefix
for i := uint32(0); ; i++ {
if i >= max {
i = 0
}
select {
case <-cancel:
done <- true
return
default:
binary.BigEndian.PutUint32(bz[1:], i)
str, err := ac.BytesToString(bz)
assert.NilError(t, err)
assert.Assert(t, str != "")
}

}
}
8 changes: 2 additions & 6 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,8 @@ func NewSimApp(
interfaceRegistry, _ := types.NewInterfaceRegistryWithOptions(types.InterfaceRegistryOptions{
ProtoFiles: proto.HybridResolver,
SigningOptions: signing.Options{
AddressCodec: address.Bech32Codec{
Bech32Prefix: sdk.GetConfig().GetBech32AccountAddrPrefix(),
},
ValidatorAddressCodec: address.Bech32Codec{
Bech32Prefix: sdk.GetConfig().GetBech32ValidatorAddrPrefix(),
},
AddressCodec: address.NewBech32Codec(sdk.GetConfig().GetBech32AccountAddrPrefix()),
ValidatorAddressCodec: address.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()),
},
})
appCodec := codec.NewProtoCodec(interfaceRegistry)
Expand Down
Loading