From 32151e506ea50d046265fe15f850b319df301ccc Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Sat, 28 Oct 2023 13:04:00 -0700 Subject: [PATCH] perf(x/group/internal/orm): move expensive prefix.NewStore calls to close usage (#18286) --- x/group/CHANGELOG.md | 4 ++++ x/group/internal/orm/index.go | 9 ++++++--- x/group/internal/orm/table.go | 7 +++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/x/group/CHANGELOG.md b/x/group/CHANGELOG.md index 45917d3391..d54a62e71c 100644 --- a/x/group/CHANGELOG.md +++ b/x/group/CHANGELOG.md @@ -25,6 +25,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Improvements + +* [18286](https://github.com/cosmos/cosmos-sdk/pull/18286) Move prefix store creation down after error checks. + ### Features ### API Breaking Changes diff --git a/x/group/internal/orm/index.go b/x/group/internal/orm/index.go index 8ade4be4d4..5227684327 100644 --- a/x/group/internal/orm/index.go +++ b/x/group/internal/orm/index.go @@ -73,11 +73,12 @@ func newIndex(tb Indexable, prefix byte, indexer *Indexer, indexerF IndexerFunc, // Has checks if a key exists. Returns an error on nil key. func (i MultiKeyIndex) Has(store types.KVStore, key interface{}) (bool, error) { - pStore := prefix.NewStore(store, []byte{i.prefix}) encodedKey, err := keyPartBytes(key, false) if err != nil { return false, err } + + pStore := prefix.NewStore(store, []byte{i.prefix}) it := pStore.Iterator(PrefixRange(encodedKey)) defer it.Close() return it.Valid(), nil @@ -85,11 +86,12 @@ func (i MultiKeyIndex) Has(store types.KVStore, key interface{}) (bool, error) { // Get returns a result iterator for the searchKey. Parameters must not be nil. func (i MultiKeyIndex) Get(store types.KVStore, searchKey interface{}) (Iterator, error) { - pStore := prefix.NewStore(store, []byte{i.prefix}) encodedKey, err := keyPartBytes(searchKey, false) if err != nil { return nil, err } + + pStore := prefix.NewStore(store, []byte{i.prefix}) it := pStore.Iterator(PrefixRange(encodedKey)) return indexIterator{store: store, it: it, rowGetter: i.rowGetter, indexKey: i.indexKey}, nil } @@ -98,7 +100,6 @@ func (i MultiKeyIndex) Get(store types.KVStore, searchKey interface{}) (Iterator // starting from pageRequest.Key if provided. // The pageRequest.Key is the rowID while searchKey is a MultiKeyIndex key. func (i MultiKeyIndex) GetPaginated(store types.KVStore, searchKey interface{}, pageRequest *query.PageRequest) (Iterator, error) { - pStore := prefix.NewStore(store, []byte{i.prefix}) encodedKey, err := keyPartBytes(searchKey, false) if err != nil { return nil, err @@ -112,6 +113,8 @@ func (i MultiKeyIndex) GetPaginated(store types.KVStore, searchKey interface{}, return nil, err } } + + pStore := prefix.NewStore(store, []byte{i.prefix}) it := pStore.Iterator(start, end) return indexIterator{store: store, it: it, rowGetter: i.rowGetter, indexKey: i.indexKey}, nil } diff --git a/x/group/internal/orm/table.go b/x/group/internal/orm/table.go index f4c3d7df5d..34de1c3a29 100644 --- a/x/group/internal/orm/table.go +++ b/x/group/internal/orm/table.go @@ -112,8 +112,6 @@ func (a table) Set(store types.KVStore, rowID RowID, newValue proto.Message) err return err } - pStore := prefix.NewStore(store, a.prefix[:]) - var oldValue proto.Message if a.Has(store, rowID) { oldValue = reflect.New(a.model).Interface().(proto.Message) @@ -129,6 +127,7 @@ func (a table) Set(store types.KVStore, rowID RowID, newValue proto.Message) err return errorsmod.Wrapf(err, "failed to serialize %T", newValue) } + pStore := prefix.NewStore(store, a.prefix[:]) pStore.Set(rowID, newValueEncoded) for i, itc := range a.afterSet { if err := itc(store, rowID, newValue, oldValue); err != nil { @@ -154,12 +153,12 @@ func assertValid(obj proto.Message) error { // Delete iterates through the registered callbacks that remove secondary index // keys. func (a table) Delete(store types.KVStore, rowID RowID) error { - pStore := prefix.NewStore(store, a.prefix[:]) - oldValue := reflect.New(a.model).Interface().(proto.Message) if err := a.GetOne(store, rowID, oldValue); err != nil { return errorsmod.Wrap(err, "load old value") } + + pStore := prefix.NewStore(store, a.prefix[:]) pStore.Delete(rowID) for i, itc := range a.afterDelete {