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 all 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 @@ -64,6 +64,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.
* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.
* (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.
Expand Down
104 changes: 101 additions & 3 deletions codec/address/bech32_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,78 @@ package address

import (
"errors"
"fmt"
"strings"
"sync"

"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"
)

var errEmptyAddress = errors.New("empty address string is not allowed")

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

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
}

type CachedCodecOptions struct {
Mu *sync.Mutex
Lru *simplelru.LRU
}

func NewBech32Codec(prefix string) address.Codec {
return Bech32Codec{prefix}
return &Bech32Codec{Bech32Prefix: prefix}
}

func NewCachedBech32Codec(prefix string, opts CachedCodecOptions) (address.Codec, error) {
var err error
ac := Bech32Codec{prefix}
if opts.Mu == nil && opts.Lru == nil {
opts.Mu = new(sync.Mutex)
opts.Lru, err = simplelru.NewLRU(256, nil)
if err != nil {
return nil, fmt.Errorf("failed to create LRU cache: %w", err)
}
} else if opts.Mu == nil && opts.Lru != nil {
// The LRU cache uses a map internally. Without a mutex, concurrent access to this map can lead to race conditions.
// Therefore, a mutex is required to ensure thread-safe operations on the LRU cache.
return nil, errors.New("mutex must be provided alongside the LRU cache")
} else if opts.Mu != nil && opts.Lru == nil {
opts.Lru, err = simplelru.NewLRU(256, nil)
if err != nil {
return nil, fmt.Errorf("failed to create LRU cache: %w", err)
}
}

return cachedBech32Codec{
codec: ac,
cache: opts.Lru,
mu: opts.Mu,
}, nil
}

// StringToBytes encodes text to bytes
func (bc Bech32Codec) StringToBytes(text string) ([]byte, error) {
if len(strings.TrimSpace(text)) == 0 {
return []byte{}, errors.New("empty address string is not allowed")
return []byte{}, errEmptyAddress
}

hrp, bz, err := bech32.DecodeAndConvert(text)
Expand Down Expand Up @@ -61,3 +109,53 @@ func (bc Bech32Codec) BytesToString(bz []byte) (string, error) {

return text, nil
}

func (cbc cachedBech32Codec) BytesToString(bz []byte) (string, error) {
if len(bz) == 0 {
return "", nil
}

key := conv.UnsafeBytesToStr(bz)
cbc.mu.Lock()
defer cbc.mu.Unlock()

addrs, ok := cbc.cache.Get(key)
if !ok {
addrs = make(map[string]string)
cbc.cache.Add(key, addrs)
}

addrMap, ok := addrs.(map[string]string)
if !ok {
return "", fmt.Errorf("cache contains non-map[string]string value for key %s", key)
}

addr, ok := addrMap[cbc.codec.Bech32Prefix]
if !ok {
var err error
addr, err = cbc.codec.BytesToString(bz)
if err != nil {
return "", err
}
addrMap[cbc.codec.Bech32Prefix] = addr
}

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

import (
"crypto/rand"
"sync"
"sync/atomic"
"testing"

"github.com/hashicorp/golang-lru/simplelru"
"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/internal/conv"
)

var (
lru, _ = simplelru.NewLRU(500, nil)
cacheOptions = CachedCodecOptions{
Mu: &sync.Mutex{},
Lru: lru,
}
)

func generateAddresses(totalAddresses int) ([][]byte, error) {
keys := make([][]byte, totalAddresses)
addr := make([]byte, 32)
for i := 0; i < totalAddresses; i++ {
_, err := rand.Read(addr)
if err != nil {
return nil, err
}
keys[i] = addr
}

return keys, nil
}

func TestNewBech32Codec(t *testing.T) {
tests := []struct {
name string
mu *sync.Mutex
lru func(t *testing.T) *simplelru.LRU
error bool
}{
{
name: "lru and mutex provided",
mu: &sync.Mutex{},
lru: func(t *testing.T) *simplelru.LRU {
t.Helper()
newLru, err := simplelru.NewLRU(500, nil)
require.NoError(t, err)
return newLru
},
},
{
name: "both empty",
mu: nil,
lru: func(t *testing.T) *simplelru.LRU {
t.Helper()
return nil
},
},
{
name: "only lru provided",
mu: nil,
lru: func(t *testing.T) *simplelru.LRU {
t.Helper()
newLru, err := simplelru.NewLRU(500, nil)
require.NoError(t, err)
return newLru
},
error: true,
},
{
name: "only mutex provided",
mu: &sync.Mutex{},
lru: func(t *testing.T) *simplelru.LRU {
t.Helper()
return nil
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
newLru := tt.lru(t)
got, err := NewCachedBech32Codec("cosmos", CachedCodecOptions{
Mu: tt.mu,
Lru: newLru,
})
if tt.error {
require.Error(t, err)
} else {
require.NoError(t, err)
require.NotNil(t, got)
}
})
}
}

func TestBech32Codec(t *testing.T) {
tests := []struct {
name string
prefix string
lru *simplelru.LRU
address string
}{
{
name: "create accounts cached bech32 codec",
prefix: "cosmos",
lru: lru,
address: "cosmos1p8s0p6gqc6c9gt77lgr2qqujz49huhu6a80smx",
},
{
name: "create validator cached bech32 codec",
prefix: "cosmosvaloper",
lru: lru,
address: "cosmosvaloper1sjllsnramtg3ewxqwwrwjxfgc4n4ef9u2lcnj0",
},
{
name: "create consensus cached bech32 codec",
prefix: "cosmosvalcons",
lru: lru,
address: "cosmosvalcons1ntk8eualewuprz0gamh8hnvcem2nrcdsgz563h",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ac, err := NewCachedBech32Codec(tt.prefix, cacheOptions)
require.NoError(t, err)

cached, ok := ac.(cachedBech32Codec)
require.True(t, ok)
require.Equal(t, cached.cache, tt.lru)

addr, err := ac.StringToBytes(tt.address)
require.NoError(t, err)

cachedAddr, ok := tt.lru.Get(tt.address)
require.True(t, ok)
require.Equal(t, addr, cachedAddr)

accAddr, err := ac.BytesToString(addr)
require.NoError(t, err)

cachedStrAddr, ok := tt.lru.Get(conv.UnsafeBytesToStr(addr))
require.True(t, ok)
cachedStrAddrMap, ok := cachedStrAddr.(map[string]string)
require.True(t, ok)
require.Equal(t, accAddr, cachedStrAddrMap[tt.prefix])
})
}
}
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
cAc, err := NewCachedBech32Codec("cosmos", cacheOptions)
require.NoError(t, err)
cosmosAc, ok := cAc.(cachedBech32Codec)
require.True(t, ok)
sAc, err := NewCachedBech32Codec("stake", cacheOptions)
require.NoError(t, err)
stakeAc, ok := sAc.(cachedBech32Codec)
require.True(t, ok)
require.Equal(t, cosmosAc.cache, stakeAc.cache)

addr := make([]byte, 32)
_, err = rand.Read(addr)
require.NoError(t, err)

cosmosAddr, err := cosmosAc.BytesToString(addr)
require.NoError(t, err)
stakeAddr, err := stakeAc.BytesToString(addr)
require.NoError(t, err)
require.True(t, cosmosAddr != stakeAddr)

cachedCosmosAddr, err := cosmosAc.BytesToString(addr)
require.NoError(t, err)
require.Equal(t, cosmosAddr, cachedCosmosAddr)

cachedStakeAddr, err := stakeAc.BytesToString(addr)
require.NoError(t, err)
require.Equal(t, stakeAddr, cachedStakeAddr)
}

func TestBech32CodecRace(t *testing.T) {
ac, err := NewCachedBech32Codec("cosmos", cacheOptions)
require.NoError(t, err)
myAddrBz := []byte{0x1, 0x2, 0x3, 0x4, 0x5}

var (
wgStart, wgDone sync.WaitGroup
errCount atomic.Uint32
)
const n = 3
wgStart.Add(n)
wgDone.Add(n)
for i := 0; i < n; i++ {
go func() {
wgStart.Done()
wgStart.Wait() // wait for all routines started

got, err := ac.BytesToString(myAddrBz)
if err != nil || got != "cosmos1qypqxpq9dc9msf" {
errCount.Add(1)
}
wgDone.Done()
}()
}
wgDone.Wait() // wait for all routines completed
require.Equal(t, errCount.Load(), uint32(0))
}
35 changes: 35 additions & 0 deletions codec/address/bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package address

import (
"testing"

"github.com/stretchr/testify/require"

"cosmossdk.io/core/address"
)

func BenchmarkCodecWithCache(b *testing.B) {
cdc, err := NewCachedBech32Codec("cosmos", cacheOptions)
require.NoError(b, err)
bytesToString(b, cdc)
}

func BenchmarkCodecWithoutCache(b *testing.B) {
cdc := Bech32Codec{Bech32Prefix: "cosmos"}
bytesToString(b, cdc)
}

func bytesToString(b *testing.B, cdc address.Codec) {
b.Helper()
addresses, err := generateAddresses(10)
require.NoError(b, err)

b.Helper()
b.ReportAllocs()
b.ResetTimer()

for i := 0; i < b.N; i++ {
_, err := cdc.BytesToString(addresses[i%len(addresses)])
require.NoError(b, err)
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
}
Loading
Loading