From ff7d594bef6c25a7bcfb7084f9be6d956febec6c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 11 Aug 2021 13:25:58 +0300 Subject: [PATCH 1/3] vm: store refcounter directly in VM VM always has it, so allocating yet another object makes no sense. --- pkg/vm/slot.go | 2 +- pkg/vm/vm.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/vm/slot.go b/pkg/vm/slot.go index 22e4d8d5c..75601b309 100644 --- a/pkg/vm/slot.go +++ b/pkg/vm/slot.go @@ -26,7 +26,7 @@ func (s *Slot) init(n int) { } func (v *VM) newSlot(n int) *Slot { - s := newSlot(v.refs) + s := newSlot(&v.refs) s.init(n) return s } diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index be47455a7..9fe81d69d 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -73,7 +73,7 @@ type VM struct { uncaughtException stackitem.Item // exception being handled - refs *refCounter + refs refCounter gasConsumed int64 GasLimit int64 @@ -100,14 +100,14 @@ func NewWithTrigger(t trigger.Type) *VM { vm := &VM{ state: NoneState, istack: newStack("invocation", nil), - refs: newRefCounter(), trigger: t, SyscallHandler: defaultSyscallHandler, Invocations: make(map[util.Uint160]int), } - vm.estack = newStack("evaluation", vm.refs) + vm.refs.items = make(map[stackitem.Item]int) + vm.estack = newStack("evaluation", &vm.refs) return vm } @@ -281,11 +281,11 @@ func (v *VM) LoadScript(b []byte) { func (v *VM) LoadScriptWithFlags(b []byte, f callflag.CallFlag) { v.checkInvocationStackSize() ctx := NewContextWithParams(b, 0, -1, 0) - v.estack = newStack("evaluation", v.refs) + v.estack = newStack("evaluation", &v.refs) ctx.estack = v.estack ctx.tryStack = newStack("exception", nil) ctx.callFlag = f - ctx.static = newSlot(v.refs) + ctx.static = newSlot(&v.refs) ctx.callingScriptHash = v.GetCurrentScriptHash() v.istack.PushVal(ctx) } From bdb2d24a5a581beee725910f508d1f76a3cc554d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 11 Aug 2021 14:40:41 +0300 Subject: [PATCH 2/3] vm: remove istack redirection in VM VM always has istack and it doesn't even change, so doing this microallocation makes no sense. Notice that estack is a bit harder to change we do replace it in some cases and we compare pointers to it as well. --- pkg/vm/stack.go | 12 +++++++----- pkg/vm/vm.go | 10 +++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/vm/stack.go b/pkg/vm/stack.go index fca28215b..659ff0191 100644 --- a/pkg/vm/stack.go +++ b/pkg/vm/stack.go @@ -162,13 +162,15 @@ func NewStack(n string) *Stack { } func newStack(n string, refc *refCounter) *Stack { - s := &Stack{ - name: n, - refs: refc, - } + s := new(Stack) + initStack(s, n, refc) + return s +} +func initStack(s *Stack, n string, refc *refCounter) { + s.name = n + s.refs = refc s.top.next = &s.top s.top.prev = &s.top - return s } // Clear clears all elements on the stack and set its length to 0. diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 9fe81d69d..4856270e4 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -68,7 +68,7 @@ type VM struct { // callback to get interop price getPrice func(opcode.Opcode, []byte) int64 - istack *Stack // invocation stack. + istack Stack // invocation stack. estack *Stack // execution stack. uncaughtException stackitem.Item // exception being handled @@ -99,7 +99,6 @@ func New() *VM { func NewWithTrigger(t trigger.Type) *VM { vm := &VM{ state: NoneState, - istack: newStack("invocation", nil), trigger: t, SyscallHandler: defaultSyscallHandler, @@ -107,6 +106,7 @@ func NewWithTrigger(t trigger.Type) *VM { } vm.refs.items = make(map[stackitem.Item]int) + initStack(&vm.istack, "invocation", nil) vm.estack = newStack("evaluation", &vm.refs) return vm } @@ -135,7 +135,7 @@ func (v *VM) Estack() *Stack { // Istack returns the invocation stack so interop hooks can utilize this. func (v *VM) Istack() *Stack { - return v.istack + return &v.istack } // LoadArgs loads in the arguments used in the Mian entry point. @@ -340,7 +340,7 @@ func (v *VM) PopResult() interface{} { func (v *VM) Stack(n string) string { var s *Stack if n == "istack" { - s = v.istack + s = &v.istack } if n == "estack" { s = v.estack @@ -1786,7 +1786,7 @@ func (v *VM) GetCallingScriptHash() util.Uint160 { // GetEntryScriptHash implements ScriptHashGetter interface. func (v *VM) GetEntryScriptHash() util.Uint160 { - return v.getContextScriptHash(v.Istack().Len() - 1) + return v.getContextScriptHash(v.istack.len - 1) } // GetCurrentScriptHash implements ScriptHashGetter interface. From 13da1b62fbdc0f0bfd80f36501935bc9ddffb1fc Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 11 Aug 2021 15:42:23 +0300 Subject: [PATCH 3/3] interop: fetch baseExecFee once and keep it in the Context It never changes during single execution, so we can cache it and avoid going to Policer via Chain for every instruction. --- pkg/core/interop/context.go | 12 ++++++++---- pkg/core/interop/crypto/ecdsa_test.go | 16 ++++++++++------ pkg/core/interop/gas_price.go | 2 +- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index 1b0f003f8..0160c9b1e 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -48,14 +48,20 @@ type Context struct { VM *vm.VM Functions []Function getContract func(dao.DAO, util.Uint160) (*state.Contract, error) + baseExecFee int64 } // NewContext returns new interop context. func NewContext(trigger trigger.Type, bc blockchainer.Blockchainer, d dao.DAO, getContract func(dao.DAO, util.Uint160) (*state.Contract, error), natives []Contract, block *block.Block, tx *transaction.Transaction, log *zap.Logger) *Context { + baseExecFee := int64(DefaultBaseExecFee) dao := d.GetWrapped() nes := make([]state.NotificationEvent, 0) + + if bc != nil && (block == nil || block.Index != 0) { + baseExecFee = bc.GetPolicer().GetBaseExecFee() + } return &Context{ Chain: bc, Network: uint32(bc.GetConfig().Magic), @@ -69,6 +75,7 @@ func NewContext(trigger trigger.Type, bc blockchainer.Blockchainer, d dao.DAO, // Functions is a slice of interops sorted by ID. Functions: []Function{}, getContract: getContract, + baseExecFee: baseExecFee, } } @@ -254,10 +261,7 @@ func (ic *Context) GetFunction(id uint32) *Function { // BaseExecFee represents factor to multiply syscall prices with. func (ic *Context) BaseExecFee() int64 { - if ic.Chain == nil || (ic.Block != nil && ic.Block.Index == 0) { - return DefaultBaseExecFee - } - return ic.Chain.GetPolicer().GetBaseExecFee() + return ic.baseExecFee } // SyscallHandler handles syscall with id. diff --git a/pkg/core/interop/crypto/ecdsa_test.go b/pkg/core/interop/crypto/ecdsa_test.go index c5ca3ab32..a5b39d21a 100644 --- a/pkg/core/interop/crypto/ecdsa_test.go +++ b/pkg/core/interop/crypto/ecdsa_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/nspcc-dev/neo-go/internal/fakechain" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/fee" @@ -67,12 +68,15 @@ func initCheckMultisigVMNoArgs(container *transaction.Transaction) *vm.VM { buf[0] = byte(opcode.SYSCALL) binary.LittleEndian.PutUint32(buf[1:], neoCryptoCheckMultisigID) - ic := &interop.Context{ - Network: uint32(netmode.UnitTestNet), - Trigger: trigger.Verification, - Container: container, - Functions: Interops, - } + ic := interop.NewContext( + trigger.Verification, + fakechain.NewFakeChain(), + dao.NewSimple(storage.NewMemoryStore(), false), + nil, nil, nil, + container, + nil) + ic.Container = container + ic.Functions = Interops v := ic.SpawnVM() v.LoadScript(buf) return v diff --git a/pkg/core/interop/gas_price.go b/pkg/core/interop/gas_price.go index c72671411..342dd9e7a 100644 --- a/pkg/core/interop/gas_price.go +++ b/pkg/core/interop/gas_price.go @@ -7,5 +7,5 @@ import ( // GetPrice returns a price for executing op with the provided parameter. func (ic *Context) GetPrice(op opcode.Opcode, parameter []byte) int64 { - return fee.Opcode(ic.BaseExecFee(), op) + return fee.Opcode(ic.baseExecFee, op) }