From 9f377cde12e3265c25429c6843ce230f1039982e Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Mon, 12 Jul 2021 14:14:14 +0300 Subject: [PATCH 1/4] storage: convert key once in `MemoryStore.seek` There is no need in additional allocations. Signed-off-by: Evgeniy Stratonikov --- pkg/core/storage/memory_store.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/core/storage/memory_store.go b/pkg/core/storage/memory_store.go index 3d542dd82..0d3e9eae2 100644 --- a/pkg/core/storage/memory_store.go +++ b/pkg/core/storage/memory_store.go @@ -121,8 +121,9 @@ func (s *MemoryStore) SeekAll(key []byte, f func(k, v []byte)) { // seek is an internal unlocked implementation of Seek. func (s *MemoryStore) seek(key []byte, f func(k, v []byte)) { + sk := string(key) for k, v := range s.mem { - if strings.HasPrefix(k, string(key)) { + if strings.HasPrefix(k, sk) { f([]byte(k), v) } } From 0d5ede0bffd434c0e58acfe2904188f2530eb09b Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Mon, 12 Jul 2021 14:19:13 +0300 Subject: [PATCH 2/4] core/test: use testing.TB in constructors This allows to use chain in benchmarks. Signed-off-by: Evgeniy Stratonikov --- pkg/core/helper_test.go | 15 ++++++++++----- pkg/core/interop_system_test.go | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/core/helper_test.go b/pkg/core/helper_test.go index 6ae741619..8847bb92a 100644 --- a/pkg/core/helper_test.go +++ b/pkg/core/helper_test.go @@ -41,6 +41,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" + "go.uber.org/zap" "go.uber.org/zap/zaptest" ) @@ -74,22 +75,22 @@ func newTestChainWithNS(t *testing.T) (*Blockchain, util.Uint160) { // newTestChain should be called before newBlock invocation to properly setup // global state. -func newTestChain(t *testing.T) *Blockchain { +func newTestChain(t testing.TB) *Blockchain { return newTestChainWithCustomCfg(t, nil) } -func newTestChainWithCustomCfg(t *testing.T, f func(*config.Config)) *Blockchain { +func newTestChainWithCustomCfg(t testing.TB, f func(*config.Config)) *Blockchain { return newTestChainWithCustomCfgAndStore(t, nil, f) } -func newTestChainWithCustomCfgAndStore(t *testing.T, st storage.Store, f func(*config.Config)) *Blockchain { +func newTestChainWithCustomCfgAndStore(t testing.TB, st storage.Store, f func(*config.Config)) *Blockchain { chain := initTestChain(t, st, f) go chain.Run() t.Cleanup(chain.Close) return chain } -func initTestChain(t *testing.T, st storage.Store, f func(*config.Config)) *Blockchain { +func initTestChain(t testing.TB, st storage.Store, f func(*config.Config)) *Blockchain { unitTestNetCfg, err := config.Load("../../config", testchain.Network()) require.NoError(t, err) if f != nil { @@ -98,7 +99,11 @@ func initTestChain(t *testing.T, st storage.Store, f func(*config.Config)) *Bloc if st == nil { st = storage.NewMemoryStore() } - chain, err := NewBlockchain(st, unitTestNetCfg.ProtocolConfiguration, zaptest.NewLogger(t)) + log := zaptest.NewLogger(t) + if _, ok := t.(*testing.B); ok { + log = zap.NewNop() + } + chain, err := NewBlockchain(st, unitTestNetCfg.ProtocolConfiguration, log) require.NoError(t, err) return chain } diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go index a64b45aa1..1b462baf4 100644 --- a/pkg/core/interop_system_test.go +++ b/pkg/core/interop_system_test.go @@ -450,7 +450,7 @@ func createVM(t *testing.T) (*vm.VM, *interop.Context, *Blockchain) { return v, context, chain } -func createVMAndContractState(t *testing.T) (*vm.VM, *state.Contract, *interop.Context, *Blockchain) { +func createVMAndContractState(t testing.TB) (*vm.VM, *state.Contract, *interop.Context, *Blockchain) { script := []byte("testscript") m := manifest.NewManifest("Test") ne, err := nef.NewFile(script) From b8b272c8c45ffad05abdb472f62d714555cbbeb7 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Mon, 12 Jul 2021 14:20:37 +0300 Subject: [PATCH 3/4] core/test: add benchmark for `storageFind` Signed-off-by: Evgeniy Stratonikov --- pkg/core/interop_system_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go index 1b462baf4..c6519ef9e 100644 --- a/pkg/core/interop_system_test.go +++ b/pkg/core/interop_system_test.go @@ -258,6 +258,36 @@ func TestStorageDelete(t *testing.T) { }) } +func BenchmarkStorageFind(b *testing.B) { + v, contractState, context, chain := createVMAndContractState(b) + require.NoError(b, chain.contracts.Management.PutContractState(chain.dao, contractState)) + + const count = 100 + + items := make(map[string]state.StorageItem) + for i := 0; i < count; i++ { + items["abc"+random.String(10)] = random.Bytes(10) + } + for k, v := range items { + require.NoError(b, context.DAO.PutStorageItem(contractState.ID, []byte(k), v)) + require.NoError(b, context.DAO.PutStorageItem(contractState.ID+1, []byte(k), v)) + } + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + b.StopTimer() + v.Estack().PushVal(istorage.FindDefault) + v.Estack().PushVal("abc") + v.Estack().PushVal(stackitem.NewInterop(&StorageContext{ID: contractState.ID})) + b.StartTimer() + err := storageFind(context) + if err != nil { + b.FailNow() + } + } +} + func TestStorageFind(t *testing.T) { v, contractState, context, chain := createVMAndContractState(t) From eb7bd7b99bd0770f7f9b241c525c58040a7ee14f Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Mon, 12 Jul 2021 14:12:00 +0300 Subject: [PATCH 4/4] core: optimize `storageFind` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because `Map` stores elements in arbitrary order, addition of new element takes linear time (`Index` iterates over all keys). Thus our `storageFind` is actually quadratic in time. Optimize this by creating map from sorted slice. ``` name old time/op new time/op delta StorageFind-8 157µs ± 2% 112µs ± 1% -28.60% (p=0.000 n=10+10) name old alloc/op new alloc/op delta StorageFind-8 69.4kB ± 0% 60.5kB ± 0% -12.90% (p=0.000 n=9+10) name old allocs/op new allocs/op delta StorageFind-8 2.21k ± 0% 2.00k ± 0% -9.37% (p=0.000 n=10+7) ``` Signed-off-by: Evgeniy Stratonikov --- pkg/core/interop_system.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/core/interop_system.go b/pkg/core/interop_system.go index e009fa778..e7578180f 100644 --- a/pkg/core/interop_system.go +++ b/pkg/core/interop_system.go @@ -192,18 +192,23 @@ func storageFind(ic *interop.Context) error { return err } - filteredMap := stackitem.NewMap() + arr := make([]stackitem.MapElement, 0, len(siMap)) for k, v := range siMap { - key := append(prefix, []byte(k)...) - keycopy := make([]byte, len(key)) - copy(keycopy, key) - filteredMap.Add(stackitem.NewByteArray(keycopy), stackitem.NewByteArray(v)) + keycopy := make([]byte, len(k)+len(prefix)) + copy(keycopy, prefix) + copy(keycopy[len(prefix):], k) + arr = append(arr, stackitem.MapElement{ + Key: stackitem.NewByteArray(keycopy), + Value: stackitem.NewByteArray(v), + }) } - sort.Slice(filteredMap.Value().([]stackitem.MapElement), func(i, j int) bool { - return bytes.Compare(filteredMap.Value().([]stackitem.MapElement)[i].Key.Value().([]byte), - filteredMap.Value().([]stackitem.MapElement)[j].Key.Value().([]byte)) == -1 + sort.Slice(arr, func(i, j int) bool { + k1 := arr[i].Key.Value().([]byte) + k2 := arr[j].Key.Value().([]byte) + return bytes.Compare(k1, k2) == -1 }) + filteredMap := stackitem.NewMapWithValue(arr) item := istorage.NewIterator(filteredMap, len(prefix), opts) ic.VM.Estack().PushVal(stackitem.NewInterop(item))