From 324f4c265b11be1817cf4c7b7da46e3192a35697 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sat, 22 Aug 2020 23:36:38 +0300 Subject: [PATCH] stackitem: don't copy existing slices for TryBytes Most often we only need to read them and it doesn't require copying. Make an explicit copy (and copy only things we need!) where needed. After the recent neo-vm tests update our vm package testing time jumped to ~12s, with this change it's now more like ~8s. --- pkg/vm/stackitem/item.go | 16 ++++++++-------- pkg/vm/vm.go | 19 ++++++++++++++----- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/vm/stackitem/item.go b/pkg/vm/stackitem/item.go index a737dff29..d928c1a62 100644 --- a/pkg/vm/stackitem/item.go +++ b/pkg/vm/stackitem/item.go @@ -33,7 +33,8 @@ type Item interface { Dup() Item // TryBool converts Item to a boolean value. TryBool() (bool, error) - // TryBytes converts Item to a byte slice. + // TryBytes converts Item to a byte slice. If the underlying type is a + // byte slice, it's returned as is without copying. TryBytes() ([]byte, error) // TryInteger converts Item to an integer. TryInteger() (*big.Int, error) @@ -151,8 +152,11 @@ func convertPrimitive(item Item, typ Type) (Item, error) { return nil, err } if typ == BufferT { - return NewBuffer(b), nil + newb := make([]byte, len(b)) + copy(newb, b) + return NewBuffer(newb), nil } + // ByteArray can't really be changed, so it's OK to reuse `b`. return NewByteArray(b), nil case BooleanT: b, err := item.TryBool() @@ -519,9 +523,7 @@ func (i *ByteArray) TryBool() (bool, error) { // TryBytes implements Item interface. func (i *ByteArray) TryBytes() ([]byte, error) { - val := make([]byte, len(i.value)) - copy(val, i.value) - return val, nil + return i.value, nil } // TryInteger implements Item interface. @@ -982,9 +984,7 @@ func (i *Buffer) TryBool() (bool, error) { // TryBytes implements Item interface. func (i *Buffer) TryBytes() ([]byte, error) { - val := make([]byte, len(i.value)) - copy(val, i.value) - return val, nil + return i.value, nil } // TryInteger implements Item interface. diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 92965c80e..cfeeeb235 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -663,10 +663,13 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case opcode.CAT: b := v.estack.Pop().Bytes() a := v.estack.Pop().Bytes() - if l := len(a) + len(b); l > stackitem.MaxSize { + l := len(a) + len(b) + if l > stackitem.MaxSize { panic(fmt.Sprintf("too big item: %d", l)) } - ab := append(a, b...) + ab := make([]byte, l) + copy(ab, a) + copy(ab[len(a):], b) v.estack.PushVal(stackitem.NewBuffer(ab)) case opcode.SUBSTR: @@ -683,7 +686,9 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro if last > len(s) { panic("invalid offset") } - v.estack.PushVal(stackitem.NewBuffer(s[o:last])) + res := make([]byte, l) + copy(res, s[o:last]) + v.estack.PushVal(stackitem.NewBuffer(res)) case opcode.LEFT: l := int(v.estack.Pop().BigInt().Int64()) @@ -694,7 +699,9 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro if t := len(s); l > t { panic("size is too big") } - v.estack.PushVal(stackitem.NewBuffer(s[:l])) + res := make([]byte, l) + copy(res, s[:l]) + v.estack.PushVal(stackitem.NewBuffer(res)) case opcode.RIGHT: l := int(v.estack.Pop().BigInt().Int64()) @@ -702,7 +709,9 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro panic("negative length") } s := v.estack.Pop().Bytes() - v.estack.PushVal(stackitem.NewBuffer(s[len(s)-l:])) + res := make([]byte, l) + copy(res, s[len(s)-l:]) + v.estack.PushVal(stackitem.NewBuffer(res)) case opcode.DEPTH: v.estack.PushVal(v.estack.Len())