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

feat(x/params): add lru caching #3120

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
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 contribs/gnodev/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ require (
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions contribs/gnodev/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions contribs/gnofaucet/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contribs/gnogenesis/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
github.com/gnolang/overflow v0.0.0-20170615021017-4d914c927216 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions contribs/gnogenesis/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contribs/gnokeykc/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ require (
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions contribs/gnokeykc/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contribs/gnomigrate/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ require (
github.com/gnolang/overflow v0.0.0-20170615021017-4d914c927216 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/websocket v1.5.3 // indirect
Expand Down
2 changes: 2 additions & 0 deletions contribs/gnomigrate/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions gno.land/pkg/gnoland/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ import (

// AppOptions contains the options to create the gno.land ABCI application.
type AppOptions struct {
DB dbm.DB // required
Logger *slog.Logger // required
EventSwitch events.EventSwitch // required
VMOutput io.Writer // optional
InitChainerConfig // options related to InitChainer
DB dbm.DB // required
Logger *slog.Logger // required
EventSwitch events.EventSwitch // required
VMOutput io.Writer // optional
InitChainerConfig // options related to InitChainer
ParamsMaxCacheSize int // optional
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the int representing? Kb? Mb? The number of items?

}

// TestAppOptions provides a "ready" default [AppOptions] for use with
Expand All @@ -53,6 +54,7 @@ func TestAppOptions(db dbm.DB) *AppOptions {
StdlibDir: filepath.Join(gnoenv.RootDir(), "gnovm", "stdlibs"),
CacheStdlibLoad: true,
},
ParamsMaxCacheSize: 100,
}
}

Expand Down Expand Up @@ -91,7 +93,7 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) {
// Construct keepers.
acctKpr := auth.NewAccountKeeper(mainKey, ProtoGnoAccount)
bankKpr := bank.NewBankKeeper(acctKpr)
paramsKpr := params.NewParamsKeeper(mainKey, "vm")
paramsKpr := params.NewParamsKeeper(mainKey, "vm", cfg.ParamsMaxCacheSize)
vmk := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, paramsKpr)
vmk.Output = cfg.VMOutput

Expand Down Expand Up @@ -185,6 +187,7 @@ func NewApp(
GenesisTxResultHandler: PanicOnFailingTxResultHandler,
StdlibDir: filepath.Join(gnoenv.RootDir(), "gnovm", "stdlibs"),
},
ParamsMaxCacheSize: 100,
}
if skipFailingGenesisTxs {
cfg.GenesisTxResultHandler = NoopGenesisTxResultHandler
Expand Down
11 changes: 6 additions & 5 deletions gno.land/pkg/gnoland/node_inmemory.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ func NewInMemoryNode(logger *slog.Logger, cfg *InMemoryNodeConfig) (*node.Node,

// Initialize the application with the provided options
gnoApp, err := NewAppWithOptions(&AppOptions{
Logger: logger,
DB: cfg.DB,
EventSwitch: evsw,
InitChainerConfig: cfg.InitChainerConfig,
VMOutput: cfg.VMOutput,
Logger: logger,
DB: cfg.DB,
EventSwitch: evsw,
InitChainerConfig: cfg.InitChainerConfig,
VMOutput: cfg.VMOutput,
ParamsMaxCacheSize: 100,
})
if err != nil {
return nil, fmt.Errorf("error initializing new app: %w", err)
Expand Down
3 changes: 2 additions & 1 deletion gno.land/pkg/sdk/vm/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func _setupTestEnv(cacheStdlibs bool) testEnv {
ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{ChainID: "test-chain-id"}, log.NewNoopLogger())
acck := authm.NewAccountKeeper(iavlCapKey, std.ProtoBaseAccount)
bank := bankm.NewBankKeeper(acck)
prmk := paramsm.NewParamsKeeper(iavlCapKey, "params")
maxCacheSize := 100
prmk := paramsm.NewParamsKeeper(iavlCapKey, "params", maxCacheSize)
vmk := NewVMKeeper(baseCapKey, iavlCapKey, acck, bank, prmk)

mcw := ms.MultiCacheWrap()
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0
github.com/fortytw2/leaktest v1.3.0
github.com/gnolang/overflow v0.0.0-20170615021017-4d914c927216
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da
github.com/google/gofuzz v1.2.0
github.com/gorilla/mux v1.8.1
github.com/gorilla/websocket v1.5.3
Expand Down
2 changes: 2 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions misc/autocounterd/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
github.com/gnolang/overflow v0.0.0-20170615021017-4d914c927216 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions misc/autocounterd/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions misc/loop/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ require (
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions misc/loop/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion tm2/pkg/sdk/params/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"github.com/gnolang/gno/tm2/pkg/amino"
"github.com/gnolang/gno/tm2/pkg/sdk"
"github.com/gnolang/gno/tm2/pkg/store"
"github.com/golang/groupcache/lru"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this lib is more maintained than the one used: https://github.com/hashicorp/golang-lru

)

const (
Expand Down Expand Up @@ -39,13 +40,15 @@
type ParamsKeeper struct {
key store.StoreKey
prefix string
cache *lru.Cache
}

// NewParamsKeeper returns a new ParamsKeeper.
func NewParamsKeeper(key store.StoreKey, prefix string) ParamsKeeper {
func NewParamsKeeper(key store.StoreKey, prefix string, maxCacheSize int) ParamsKeeper {
return ParamsKeeper{
key: key,
prefix: prefix,
cache: lru.New(maxCacheSize),
}
}

Expand Down Expand Up @@ -116,6 +119,14 @@
}

func (pk ParamsKeeper) getIfExists(ctx sdk.Context, key string, ptr interface{}) {
if bz, ok := pk.cache.Get(key); ok {
if bz == nil {
return
}

Check warning on line 125 in tm2/pkg/sdk/params/keeper.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/params/keeper.go#L124-L125

Added lines #L124 - L125 were not covered by tests
Comment on lines +123 to +125
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the meaning of that code? can we have nil values in our cache? why?

amino.UnmarshalJSON(bz.([]byte), ptr)
return
}

stor := ctx.Store(pk.key)
bz := stor.Get([]byte(key))
if bz == nil {
Expand All @@ -125,15 +136,25 @@
if err != nil {
panic(err)
}
pk.cache.Add(key, bz)
}

func (pk ParamsKeeper) get(ctx sdk.Context, key string, ptr interface{}) {
if bz, ok := pk.cache.Get(key); ok {
err := amino.UnmarshalJSON(bz.([]byte), ptr)
if err != nil {
panic(err)
}
return
}

stor := ctx.Store(pk.key)
bz := stor.Get([]byte(key))
err := amino.UnmarshalJSON(bz, ptr)
if err != nil {
panic(err)
}
pk.cache.Add(key, bz)

Check warning on line 157 in tm2/pkg/sdk/params/keeper.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/params/keeper.go#L157

Added line #L157 was not covered by tests
}

func (pk ParamsKeeper) set(ctx sdk.Context, key string, value interface{}) {
Expand All @@ -143,6 +164,7 @@
panic(err)
}
stor.Set([]byte(key), bz)
pk.cache.Remove(key)
}

func checkSuffix(key, expectedSuffix string) {
Expand Down
60 changes: 60 additions & 0 deletions tm2/pkg/sdk/params/keeper_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package params

import (
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -137,6 +138,65 @@ func TestKeeper_internal(t *testing.T) {
}
}

func TestKeeper_cache(t *testing.T) {
env := setupTestEnv() // maxCacheSize=10 by default
ctx, keeper := env.ctx, env.keeper

// fill store with 20 items
for i := 0; i < 20; i++ {
key := fmt.Sprintf("k%d", i)
param := fmt.Sprintf("v%d", i)
require.NotPanics(t, func() { keeper.set(ctx, key, param) }, "keeper.Set panics, tc #%d", i)
}

// test cache miss
_, ok := keeper.cache.Get("k0")
require.False(t, ok)

// get + test value is valid
var target string
require.NotPanics(t, func() { keeper.getIfExists(ctx, "k0", &target) })
require.Equal(t, target, "v0")

// test cache hit + value is valid
ret, ok := keeper.cache.Get("k0")
require.True(t, ok)
require.Equal(t, ret.([]byte), []byte(`"v0"`))

// get again (from cache) + test value is valid
require.NotPanics(t, func() { keeper.getIfExists(ctx, "k0", &target) })
require.Equal(t, target, "v0")

// get 9 items to fill the lru cache
for i := 1; i < 10; i++ {
key := fmt.Sprintf("k%d", i)
val := fmt.Sprintf("v%d", i)
require.NotPanics(t, func() { keeper.getIfExists(ctx, key, &target) })
require.Equal(t, target, val)
}

// test cache hit for first item
ret, ok = keeper.cache.Get("k0")
require.True(t, ok)
require.Equal(t, ret.([]byte), []byte(`"v0"`))

// get 1 item
require.NotPanics(t, func() { keeper.getIfExists(ctx, "k10", &target) })
require.Equal(t, target, "v10")

// test cache hit for first item (because accessed recently)
_, ok = keeper.cache.Get("k0")
require.True(t, ok)

// test cache miss for second item (because oldest and never accessed)
_, ok = keeper.cache.Get("k1")
require.False(t, ok)

// get second item + test value is valid
require.NotPanics(t, func() { keeper.getIfExists(ctx, "k1", &target) })
require.Equal(t, target, "v1")
}

type s struct{ I int }

func indirect(ptr interface{}) interface{} { return reflect.ValueOf(ptr).Elem().Interface() }
3 changes: 2 additions & 1 deletion tm2/pkg/sdk/params/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ func setupTestEnv() testEnv {
ms.LoadLatestVersion()

prefix := "params_test"
keeper := NewParamsKeeper(paramsCapKey, prefix)
maxCacheSize := 10
keeper := NewParamsKeeper(paramsCapKey, prefix, maxCacheSize)

ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{Height: 1, ChainID: "test-chain-id"}, log.NewNoopLogger())
// XXX: context key?
Expand Down
Loading