From 3509523baf7621da1c4b7cc9ff68f785947d4b26 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 11:44:12 +0300 Subject: [PATCH 1/9] contract: tune isolation logic Wrap when we have outer exception handler and the method is not safe. Otherwise we wrap almost always, since non-readonly methods are very common. --- pkg/core/interop/contract/call.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index 41f2f588f..a322bcc7c 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -118,8 +118,8 @@ func callExFromNative(ic *interop.Context, caller util.Uint160, cs *state.Contra ic.Invocations[cs.Hash]++ f = ic.VM.Context().GetCallFlags() & f - wrapped := f&(callflag.All^callflag.ReadOnly) != 0 || // If the method is safe, then it's read-only and doesn't perform storage changes or emit notifications. - ic.VM.Context().HasTryBlock() // If the method is not wrapped into try-catch block, then changes should be discarded anyway if exception occurs. + wrapped := ic.VM.Context().HasTryBlock() && // If the method is not wrapped into try-catch block, then changes should be discarded anyway if exception occurs. + f&(callflag.All^callflag.ReadOnly) != 0 // If the method is safe, then it's read-only and doesn't perform storage changes or emit notifications. baseNtfCount := len(ic.Notifications) baseDAO := ic.DAO if wrapped { From 3945e8185773c1001ad698f94271a082260aacbe Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 12:45:34 +0300 Subject: [PATCH 2/9] bigint: don't allocate in ToPreallocatedBytes() for negative numbers In-place modifications are somewhat dangerous, but yet another allocation is quite costly. --- pkg/encoding/bigint/bigint.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/encoding/bigint/bigint.go b/pkg/encoding/bigint/bigint.go index 41f3c599e..26a6849f8 100644 --- a/pkg/encoding/bigint/bigint.go +++ b/pkg/encoding/bigint/bigint.go @@ -112,18 +112,16 @@ func ToPreallocatedBytes(n *big.Int, data []byte) []byte { return data } - var ws []big.Word - if sign == 1 { - ws = n.Bits() - } else { - n1 := new(big.Int).Add(n, bigOne) - if n1.Sign() == 0 { // n == -1 + if sign < 0 { + n.Add(n, bigOne) + defer func() { n.Sub(n, bigOne) }() + if n.Sign() == 0 { // n == -1 return append(data, 0xFF) } - - ws = n1.Bits() } + var ws = n.Bits() + lb := len(ws) * wordSizeBytes if c := cap(data); c < lb { data = make([]byte, lb, lb+1) From 10110d4e700c7263dcc19a64ce1e6d0dab83c88c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 16:51:19 +0300 Subject: [PATCH 3/9] bigint: correct MaxBytesLen It can't be 33, positive signed int256 all fit into 32 bytes (no need for leading zero), negative ones fit into 32 bytes as well. --- pkg/encoding/bigint/bigint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/encoding/bigint/bigint.go b/pkg/encoding/bigint/bigint.go index 26a6849f8..35081992a 100644 --- a/pkg/encoding/bigint/bigint.go +++ b/pkg/encoding/bigint/bigint.go @@ -10,7 +10,7 @@ import ( const ( // MaxBytesLen is the maximum length of a serialized integer suitable for Neo VM. - MaxBytesLen = 33 // 32 bytes for a 256-bit integer plus 1 if padding needed + MaxBytesLen = 32 // 256-bit signed integer // wordSizeBytes is a size of a big.Word (uint) in bytes. wordSizeBytes = bits.UintSize / 8 ) From 3d4076ca36e67e52472cb02e21c4a19ac8f69f35 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 18:53:05 +0300 Subject: [PATCH 4/9] vm: microoptimize new estack creation Subslice won't reach element -1, but it can reuse the same buffer space more effectively. --- pkg/vm/stack.go | 8 ++++++++ pkg/vm/vm.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/vm/stack.go b/pkg/vm/stack.go index 503c25571..b0eb41fc1 100644 --- a/pkg/vm/stack.go +++ b/pkg/vm/stack.go @@ -132,6 +132,14 @@ func newStack(n string, refc *refCounter) *Stack { initStack(s, n, refc) return s } + +func subStack(old *Stack) *Stack { + s := new(Stack) + *s = *old + s.elems = s.elems[len(s.elems):] + return s +} + func initStack(s *Stack, n string, refc *refCounter) { s.name = n s.refs = refc diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index c9843a48d..2ea18796b 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -322,7 +322,7 @@ func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, caller util.Uint v.checkInvocationStackSize() ctx := NewContextWithParams(b, rvcount, offset) if rvcount != -1 || v.estack.Len() != 0 { - v.estack = newStack("evaluation", &v.refs) + v.estack = subStack(v.estack) } ctx.estack = v.estack initStack(&ctx.tryStack, "exception", nil) From c3d989ebdaeada5107a96bf20aa2b4423b95c384 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 20:10:20 +0300 Subject: [PATCH 5/9] stackitem: reusable serialization context We serialize items a lot and this allows to avoid a number of allocations. --- pkg/core/blockchain.go | 1 + pkg/core/dao/dao.go | 19 +++++++++-- pkg/core/interop/runtime/engine.go | 2 +- pkg/core/interop/runtime/engine_test.go | 3 +- pkg/core/native/native_neo.go | 12 +++---- pkg/core/native/neo_types.go | 4 +-- pkg/core/native/oracle.go | 3 +- pkg/core/native/std.go | 6 ++-- pkg/core/native/std_test.go | 9 +++--- pkg/core/native/util.go | 6 +++- pkg/core/state/native_state.go | 17 ++++------ pkg/core/state/notification_event.go | 28 ++++++++++++++-- pkg/vm/stackitem/serialization.go | 43 +++++++++++++++++++++---- 13 files changed, 111 insertions(+), 42 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index a1616389b..1f881156b 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1094,6 +1094,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error } close(aerdone) }() + _ = cache.GetItemCtx() // Prime serialization context cache (it'll be reused by upper layer DAOs). aer, err := bc.runPersist(bc.contracts.GetPersistScript(), block, cache, trigger.OnPersist) if err != nil { // Release goroutines, don't care about errors, we already have one. diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index daf2e60c2..6995612ce 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -17,6 +17,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/util/slice" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) // HasTransaction errors. @@ -44,6 +45,7 @@ type Simple struct { nativeCachePS *Simple private bool + serCtx *stackitem.SerializationContext keyBuf []byte dataBuf *io.BufBinWriter } @@ -96,6 +98,7 @@ func (dao *Simple) GetPrivate() *Simple { Version: dao.Version, keyBuf: dao.keyBuf, dataBuf: dao.dataBuf, + serCtx: dao.serCtx, } // Inherit everything... d.Store = storage.NewPrivateMemCachedStore(dao.Store) // except storage, wrap another layer. d.private = true @@ -710,10 +713,10 @@ func (dao *Simple) StoreAsBlock(block *block.Block, aer1 *state.AppExecResult, a buf.WriteB(storage.ExecBlock) block.EncodeTrimmed(buf.BinWriter) if aer1 != nil { - aer1.EncodeBinary(buf.BinWriter) + aer1.EncodeBinaryWithContext(buf.BinWriter, dao.GetItemCtx()) } if aer2 != nil { - aer2.EncodeBinary(buf.BinWriter) + aer2.EncodeBinaryWithContext(buf.BinWriter, dao.GetItemCtx()) } if buf.Err != nil { return buf.Err @@ -790,7 +793,7 @@ func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32, buf.WriteU32LE(index) tx.EncodeBinary(buf.BinWriter) if aer != nil { - aer.EncodeBinary(buf.BinWriter) + aer.EncodeBinaryWithContext(buf.BinWriter, dao.GetItemCtx()) } if buf.Err != nil { return buf.Err @@ -835,6 +838,16 @@ func (dao *Simple) getDataBuf() *io.BufBinWriter { return io.NewBufBinWriter() } +func (dao *Simple) GetItemCtx() *stackitem.SerializationContext { + if dao.private { + if dao.serCtx == nil { + dao.serCtx = stackitem.NewSerializationContext() + } + return dao.serCtx + } + return stackitem.NewSerializationContext() +} + // Persist flushes all the changes made into the (supposedly) persistent // underlying store. It doesn't block accesses to DAO from other threads. func (dao *Simple) Persist() (int, error) { diff --git a/pkg/core/interop/runtime/engine.go b/pkg/core/interop/runtime/engine.go index f34358479..749d704dd 100644 --- a/pkg/core/interop/runtime/engine.go +++ b/pkg/core/interop/runtime/engine.go @@ -61,7 +61,7 @@ func Notify(ic *interop.Context) error { // But it has to be serializable, otherwise we either have some broken // (recursive) structure inside or an interop item that can't be used // outside of the interop subsystem anyway. - bytes, err := stackitem.Serialize(elem.Item()) + bytes, err := ic.DAO.GetItemCtx().Serialize(elem.Item(), false) if err != nil { return fmt.Errorf("bad notification: %w", err) } diff --git a/pkg/core/interop/runtime/engine_test.go b/pkg/core/interop/runtime/engine_test.go index 41b1c2dc9..fffe8900f 100644 --- a/pkg/core/interop/runtime/engine_test.go +++ b/pkg/core/interop/runtime/engine_test.go @@ -8,6 +8,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/random" "github.com/nspcc-dev/neo-go/pkg/core/block" + "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" @@ -133,7 +134,7 @@ func TestLog(t *testing.T) { func TestNotify(t *testing.T) { h := random.Uint160() newIC := func(name string, args interface{}) *interop.Context { - ic := &interop.Context{VM: vm.New()} + ic := &interop.Context{VM: vm.New(), DAO: &dao.Simple{}} ic.VM.LoadScriptWithHash([]byte{1}, h, callflag.NoneFlag) ic.VM.Estack().PushVal(args) ic.VM.Estack().PushVal(name) diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 734667014..7e601d3c8 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -281,7 +281,7 @@ func (n *NEO) Initialize(ic *interop.Context) error { return err } - ic.DAO.PutStorageItem(n.ID, prefixCommittee, cvs.Bytes()) + ic.DAO.PutStorageItem(n.ID, prefixCommittee, cvs.Bytes(ic.DAO.GetItemCtx())) h, err := getStandbyValidatorsHash(ic) if err != nil { @@ -355,7 +355,7 @@ func (n *NEO) updateCache(cache *NeoCache, cvs keysWithVotes, blockHeight uint32 func (n *NEO) updateCommittee(cache *NeoCache, ic *interop.Context) error { if !cache.votesChanged { // We need to put in storage anyway, as it affects dumps - ic.DAO.PutStorageItem(n.ID, prefixCommittee, cache.committee.Bytes()) + ic.DAO.PutStorageItem(n.ID, prefixCommittee, cache.committee.Bytes(ic.DAO.GetItemCtx())) return nil } @@ -367,7 +367,7 @@ func (n *NEO) updateCommittee(cache *NeoCache, ic *interop.Context) error { return err } cache.votesChanged = false - ic.DAO.PutStorageItem(n.ID, prefixCommittee, cvs.Bytes()) + ic.DAO.PutStorageItem(n.ID, prefixCommittee, cvs.Bytes(ic.DAO.GetItemCtx())) return nil } @@ -495,7 +495,7 @@ func (n *NEO) increaseBalance(ic *interop.Context, h util.Uint160, si *state.Sto postF = func() { n.GAS.mint(ic, h, newGas, true) } } if amount.Sign() == 0 { - *si = acc.Bytes() + *si = acc.Bytes(ic.DAO.GetItemCtx()) return postF, nil } if err := n.ModifyAccountVotes(acc, ic.DAO, amount, false); err != nil { @@ -508,7 +508,7 @@ func (n *NEO) increaseBalance(ic *interop.Context, h util.Uint160, si *state.Sto } acc.Balance.Add(&acc.Balance, amount) if acc.Balance.Sign() != 0 { - *si = acc.Bytes() + *si = acc.Bytes(ic.DAO.GetItemCtx()) } else { *si = nil } @@ -872,7 +872,7 @@ func (n *NEO) VoteInternal(ic *interop.Context, h util.Uint160, pub *keys.Public if err := n.ModifyAccountVotes(acc, ic.DAO, &acc.Balance, true); err != nil { return err } - ic.DAO.PutStorageItem(n.ID, key, acc.Bytes()) + ic.DAO.PutStorageItem(n.ID, key, acc.Bytes(ic.DAO.GetItemCtx())) ic.AddNotification(n.Hash, "Vote", stackitem.NewArray([]stackitem.Item{ stackitem.NewByteArray(h.BytesBE()), diff --git a/pkg/core/native/neo_types.go b/pkg/core/native/neo_types.go index 7c27e45bc..530f7b935 100644 --- a/pkg/core/native/neo_types.go +++ b/pkg/core/native/neo_types.go @@ -82,8 +82,8 @@ func (k *keysWithVotes) fromStackItem(item stackitem.Item) error { } // Bytes serializes keys with votes slice. -func (k keysWithVotes) Bytes() []byte { - buf, err := stackitem.Serialize(k.toStackItem()) +func (k keysWithVotes) Bytes(sc *stackitem.SerializationContext) []byte { + buf, err := sc.Serialize(k.toStackItem(), false) if err != nil { panic(err) } diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 800423656..0f819104a 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -364,13 +364,14 @@ func (o *Oracle) RequestInternal(ic *interop.Context, url string, filter *string return err } - data, err := stackitem.Serialize(userData) + data, err := ic.DAO.GetItemCtx().Serialize(userData, false) if err != nil { return err } if len(data) > maxUserDataLength { return ErrBigArgument } + data = slice.Copy(data) // Serialization context will be used in PutRequestInternal again. var filterNotif stackitem.Item if filter != nil { diff --git a/pkg/core/native/std.go b/pkg/core/native/std.go index 00ff22562..733fd7338 100644 --- a/pkg/core/native/std.go +++ b/pkg/core/native/std.go @@ -160,8 +160,8 @@ func newStd() *Std { return s } -func (s *Std) serialize(_ *interop.Context, args []stackitem.Item) stackitem.Item { - data, err := stackitem.Serialize(args[0]) +func (s *Std) serialize(ic *interop.Context, args []stackitem.Item) stackitem.Item { + data, err := ic.DAO.GetItemCtx().Serialize(args[0], false) if err != nil { panic(err) } @@ -169,7 +169,7 @@ func (s *Std) serialize(_ *interop.Context, args []stackitem.Item) stackitem.Ite panic(errors.New("too big item")) } - return stackitem.NewByteArray(data) + return stackitem.NewByteArray(slice.Copy(data)) // Serialization context can be reused. } func (s *Std) deserialize(_ *interop.Context, args []stackitem.Item) stackitem.Item { diff --git a/pkg/core/native/std_test.go b/pkg/core/native/std_test.go index da56a4f90..2ee87f31a 100644 --- a/pkg/core/native/std_test.go +++ b/pkg/core/native/std_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/mr-tron/base58" + "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" base58neogo "github.com/nspcc-dev/neo-go/pkg/encoding/base58" "github.com/nspcc-dev/neo-go/pkg/vm" @@ -19,7 +20,7 @@ import ( func TestStdLibItoaAtoi(t *testing.T) { s := newStd() - ic := &interop.Context{VM: vm.New()} + ic := &interop.Context{VM: vm.New(), DAO: &dao.Simple{}} var actual stackitem.Item t.Run("itoa-atoi", func(t *testing.T) { @@ -251,7 +252,7 @@ func TestStdLibEncodeDecode(t *testing.T) { func TestStdLibSerialize(t *testing.T) { s := newStd() - ic := &interop.Context{VM: vm.New()} + ic := &interop.Context{VM: vm.New(), DAO: &dao.Simple{}} t.Run("recursive", func(t *testing.T) { arr := stackitem.NewArray(nil) @@ -294,7 +295,7 @@ func TestStdLibSerialize(t *testing.T) { func TestStdLibSerializeDeserialize(t *testing.T) { s := newStd() - ic := &interop.Context{VM: vm.New()} + ic := &interop.Context{VM: vm.New(), DAO: &dao.Simple{}} var actual stackitem.Item checkSerializeDeserialize := func(t *testing.T, value interface{}, expected stackitem.Item) { @@ -381,7 +382,7 @@ func TestStdLibSerializeDeserialize(t *testing.T) { func TestMemoryCompare(t *testing.T) { s := newStd() - ic := &interop.Context{VM: vm.New()} + ic := &interop.Context{VM: vm.New(), DAO: &dao.Simple{}} check := func(t *testing.T, result int64, s1, s2 string) { actual := s.memoryCompare(ic, []stackitem.Item{stackitem.Make(s1), stackitem.Make(s2)}) diff --git a/pkg/core/native/util.go b/pkg/core/native/util.go index 38fe03eee..a9d85ba9e 100644 --- a/pkg/core/native/util.go +++ b/pkg/core/native/util.go @@ -24,7 +24,11 @@ func getConvertibleFromDAO(id int32, d *dao.Simple, key []byte, conv stackitem.C } func putConvertibleToDAO(id int32, d *dao.Simple, key []byte, conv stackitem.Convertible) error { - data, err := stackitem.SerializeConvertible(conv) + item, err := conv.ToStackItem() + if err != nil { + return err + } + data, err := d.GetItemCtx().Serialize(item, false) if err != nil { return err } diff --git a/pkg/core/state/native_state.go b/pkg/core/state/native_state.go index 0f255b01c..48eef1a0f 100644 --- a/pkg/core/state/native_state.go +++ b/pkg/core/state/native_state.go @@ -70,14 +70,6 @@ func balanceFromBytes(b []byte, item stackitem.Convertible) error { return stackitem.DeserializeConvertible(b, item) } -func balanceToBytes(item stackitem.Convertible) []byte { - data, err := stackitem.SerializeConvertible(item) - if err != nil { - panic(err) - } - return data -} - // ToStackItem implements stackitem.Convertible. It never returns an error. func (s *NEP17Balance) ToStackItem() (stackitem.Item, error) { return stackitem.NewStruct([]stackitem.Item{stackitem.NewBigInteger(&s.Balance)}), nil @@ -111,8 +103,13 @@ func NEOBalanceFromBytes(b []byte) (*NEOBalance, error) { } // Bytes returns a serialized NEOBalance. -func (s *NEOBalance) Bytes() []byte { - return balanceToBytes(s) +func (s *NEOBalance) Bytes(sc *stackitem.SerializationContext) []byte { + item, _ := s.ToStackItem() // Never returns an error. + data, err := sc.Serialize(item, false) + if err != nil { + panic(err) + } + return data } // ToStackItem implements stackitem.Convertible interface. It never returns an error. diff --git a/pkg/core/state/notification_event.go b/pkg/core/state/notification_event.go index 709d7d165..d87bb40e0 100644 --- a/pkg/core/state/notification_event.go +++ b/pkg/core/state/notification_event.go @@ -29,9 +29,20 @@ type AppExecResult struct { // EncodeBinary implements the Serializable interface. func (ne *NotificationEvent) EncodeBinary(w *io.BinWriter) { + ne.EncodeBinaryWithContext(w, stackitem.NewSerializationContext()) +} + +// EncodeBinaryWithContext is the same as EncodeBinary, but allows to efficiently reuse +// stack item serialization context. +func (ne *NotificationEvent) EncodeBinaryWithContext(w *io.BinWriter, sc *stackitem.SerializationContext) { ne.ScriptHash.EncodeBinary(w) w.WriteString(ne.Name) - stackitem.EncodeBinary(ne.Item, w) + b, err := sc.Serialize(ne.Item, false) + if err != nil { + w.Err = err + return + } + w.WriteBytes(b) } // DecodeBinary implements the Serializable interface. @@ -52,6 +63,12 @@ func (ne *NotificationEvent) DecodeBinary(r *io.BinReader) { // EncodeBinary implements the Serializable interface. func (aer *AppExecResult) EncodeBinary(w *io.BinWriter) { + aer.EncodeBinaryWithContext(w, stackitem.NewSerializationContext()) +} + +// EncodeBinaryWithContext is the same as EncodeBinary, but allows to efficiently reuse +// stack item serialization context. +func (aer *AppExecResult) EncodeBinaryWithContext(w *io.BinWriter, sc *stackitem.SerializationContext) { w.WriteBytes(aer.Container[:]) w.WriteB(byte(aer.Trigger)) w.WriteB(byte(aer.VMState)) @@ -59,11 +76,16 @@ func (aer *AppExecResult) EncodeBinary(w *io.BinWriter) { // Stack items are expected to be marshaled one by one. w.WriteVarUint(uint64(len(aer.Stack))) for _, it := range aer.Stack { - stackitem.EncodeBinaryProtected(it, w) + b, err := sc.Serialize(it, true) + if err != nil { + w.Err = err + return + } + w.WriteBytes(b) } w.WriteVarUint(uint64(len(aer.Events))) for i := range aer.Events { - aer.Events[i].EncodeBinary(w) + aer.Events[i].EncodeBinaryWithContext(w, sc) } w.WriteVarBytes([]byte(aer.FaultException)) } diff --git a/pkg/vm/stackitem/serialization.go b/pkg/vm/stackitem/serialization.go index fcc0c3c2b..0ddccfe61 100644 --- a/pkg/vm/stackitem/serialization.go +++ b/pkg/vm/stackitem/serialization.go @@ -27,8 +27,8 @@ var ErrRecursive = errors.New("recursive item") // be serialized (like Interop item or Pointer). var ErrUnserializable = errors.New("unserializable") -// serContext is an internal serialization context. -type serContext struct { +// SerializationContext is a serialization context. +type SerializationContext struct { uv [9]byte data []byte allowInvalid bool @@ -44,7 +44,7 @@ type deserContext struct { // Serialize encodes the given Item into a byte slice. func Serialize(item Item) ([]byte, error) { - sc := serContext{ + sc := SerializationContext{ allowInvalid: false, seen: make(map[Item]sliceNoPointer, typicalNumOfItems), } @@ -73,7 +73,7 @@ func EncodeBinary(item Item, w *io.BinWriter) { // (like recursive array) is encountered, it just writes the special InvalidT // type of an element to the w. func EncodeBinaryProtected(item Item, w *io.BinWriter) { - sc := serContext{ + sc := SerializationContext{ allowInvalid: true, seen: make(map[Item]sliceNoPointer, typicalNumOfItems), } @@ -85,7 +85,7 @@ func EncodeBinaryProtected(item Item, w *io.BinWriter) { w.WriteBytes(sc.data) } -func (w *serContext) writeArray(item Item, arr []Item, start int) error { +func (w *SerializationContext) writeArray(item Item, arr []Item, start int) error { w.seen[item] = sliceNoPointer{} w.appendVarUint(uint64(len(arr))) for i := range arr { @@ -97,7 +97,36 @@ func (w *serContext) writeArray(item Item, arr []Item, start int) error { return nil } -func (w *serContext) serialize(item Item) error { +// NewSerializationContext returns reusable stack item serialization context. +func NewSerializationContext() *SerializationContext { + return &SerializationContext{ + seen: make(map[Item]sliceNoPointer, typicalNumOfItems), + } +} + +// Serialize returns flat slice of bytes with the given item. The process can be protected +// from bad elements if appropriate flag is given (otherwise an error is returned on +// encountering any of them). The buffer returned is only valid until the call to Serialize. +func (w *SerializationContext) Serialize(item Item, protected bool) ([]byte, error) { + w.allowInvalid = protected + if w.data != nil { + w.data = w.data[:0] + } + for k := range w.seen { + delete(w.seen, k) + } + err := w.serialize(item) + if err != nil && protected { + if w.data == nil { + w.data = make([]byte, 0, 1) + } + w.data = append(w.data[:0], byte(InvalidT)) + err = nil + } + return w.data, err +} + +func (w *SerializationContext) serialize(item Item) error { if v, ok := w.seen[item]; ok { if v.start == v.end { return ErrRecursive @@ -180,7 +209,7 @@ func (w *serContext) serialize(item Item) error { return nil } -func (w *serContext) appendVarUint(val uint64) { +func (w *SerializationContext) appendVarUint(val uint64) { n := io.PutVarUint(w.uv[:], val) w.data = append(w.data, w.uv[:n]...) } From 9a06995460815e05a189a42f224a099dd881502d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 May 2022 23:10:56 +0300 Subject: [PATCH 6/9] bigint: don't allocate in ToPreallocatedBytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out, it's almost always allocating because we're mostly dealing with small integers while the buffer size is calculated in 8-byte chunks here, so preallocated buffer is always insufficient. name old time/op new time/op delta ToPreallocatedBytes-8 28.5ns ± 7% 19.7ns ± 5% -30.72% (p=0.000 n=10+10) name old alloc/op new alloc/op delta ToPreallocatedBytes-8 16.0B ± 0% 0.0B -100.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta ToPreallocatedBytes-8 1.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10) Fix StorageItem reuse at the same time. We don't copy when getting values from the storage, but we don when we're putting them, so buffer reuse could corrupt old values. --- pkg/core/dao/dao.go | 10 +++++++++ pkg/core/native/management.go | 3 +-- pkg/core/native/native_neo.go | 7 +++--- pkg/core/native/native_nep17.go | 3 +-- pkg/core/native/oracle.go | 3 +-- pkg/core/native/util.go | 2 +- pkg/encoding/bigint/bench_test.go | 15 +++++++++++++ pkg/encoding/bigint/bigint.go | 37 +++++-------------------------- 8 files changed, 38 insertions(+), 42 deletions(-) create mode 100644 pkg/encoding/bigint/bench_test.go diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index 6995612ce..c2563f24d 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -7,12 +7,14 @@ import ( "errors" "fmt" iocore "io" + "math/big" "sync" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" @@ -387,6 +389,14 @@ func (dao *Simple) PutStorageItem(id int32, key []byte, si state.StorageItem) { dao.Store.Put(stKey, si) } +// PutBigInt serializaed and puts the given integer for the given id with the given +// key into the given store. +func (dao *Simple) PutBigInt(id int32, key []byte, n *big.Int) { + var buf [bigint.MaxBytesLen]byte + stData := bigint.ToPreallocatedBytes(n, buf[:]) + dao.PutStorageItem(id, key, stData) +} + // DeleteStorageItem drops a storage item for the given id with the // given key from the store. func (dao *Simple) DeleteStorageItem(id int32, key []byte) { diff --git a/pkg/core/native/management.go b/pkg/core/native/management.go index 92de9015d..b30266624 100644 --- a/pkg/core/native/management.go +++ b/pkg/core/native/management.go @@ -601,8 +601,7 @@ func (m *Management) getNextContractID(d *dao.Simple) (int32, error) { id := bigint.FromBytes(si) ret := int32(id.Int64()) id.Add(id, intOne) - si = bigint.ToPreallocatedBytes(id, si) - d.PutStorageItem(m.ID, keyNextAvailableID, si) + d.PutBigInt(m.ID, keyNextAvailableID, id) return ret, nil } diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 7e601d3c8..cde774216 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -440,7 +440,7 @@ func (n *NEO) PostPersist(ic *interop.Context) error { } cache.gasPerVoteCache[cs[i].Key] = *tmp - ic.DAO.PutStorageItem(n.ID, key, bigint.ToBytes(tmp)) + ic.DAO.PutBigInt(n.ID, key, tmp) } } } @@ -1088,8 +1088,7 @@ func (n *NEO) modifyVoterTurnout(d *dao.Simple, amount *big.Int) error { } votersCount := bigint.FromBytes(si) votersCount.Add(votersCount, amount) - si = bigint.ToPreallocatedBytes(votersCount, si) - d.PutStorageItem(n.ID, key, si) + d.PutBigInt(n.ID, key, votersCount) return nil } @@ -1218,5 +1217,5 @@ func (n *NEO) putGASRecord(dao *dao.Simple, index uint32, value *big.Int) { key := make([]byte, 5) key[0] = prefixGASPerBlock binary.BigEndian.PutUint32(key[1:], index) - dao.PutStorageItem(n.ID, key, bigint.ToBytes(value)) + dao.PutBigInt(n.ID, key, value) } diff --git a/pkg/core/native/native_nep17.go b/pkg/core/native/native_nep17.go index e0963530e..6a2c42c02 100644 --- a/pkg/core/native/native_nep17.go +++ b/pkg/core/native/native_nep17.go @@ -108,8 +108,7 @@ func (c *nep17TokenNative) getTotalSupply(d *dao.Simple) (state.StorageItem, *bi } func (c *nep17TokenNative) saveTotalSupply(d *dao.Simple, si state.StorageItem, supply *big.Int) { - si = state.StorageItem(bigint.ToPreallocatedBytes(supply, si)) - d.PutStorageItem(c.ID, totalSupplyKey, si) + d.PutBigInt(c.ID, totalSupplyKey, supply) } func (c *nep17TokenNative) Transfer(ic *interop.Context, args []stackitem.Item) stackitem.Item { diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 0f819104a..505a2680c 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -355,8 +355,7 @@ func (o *Oracle) RequestInternal(ic *interop.Context, url string, filter *string itemID := bigint.FromBytes(si) id := itemID.Uint64() itemID.Add(itemID, intOne) - si = bigint.ToPreallocatedBytes(itemID, si) - ic.DAO.PutStorageItem(o.ID, prefixRequestID, si) + ic.DAO.PutBigInt(o.ID, prefixRequestID, itemID) // Should be executed from the contract. _, err := ic.GetContract(ic.VM.GetCallingScriptHash()) diff --git a/pkg/core/native/util.go b/pkg/core/native/util.go index a9d85ba9e..0666b9eac 100644 --- a/pkg/core/native/util.go +++ b/pkg/core/native/util.go @@ -37,7 +37,7 @@ func putConvertibleToDAO(id int32, d *dao.Simple, key []byte, conv stackitem.Con } func setIntWithKey(id int32, dao *dao.Simple, key []byte, value int64) { - dao.PutStorageItem(id, key, bigint.ToBytes(big.NewInt(value))) + dao.PutBigInt(id, key, big.NewInt(value)) } func getIntWithKey(id int32, dao *dao.Simple, key []byte) int64 { diff --git a/pkg/encoding/bigint/bench_test.go b/pkg/encoding/bigint/bench_test.go new file mode 100644 index 000000000..411a10b85 --- /dev/null +++ b/pkg/encoding/bigint/bench_test.go @@ -0,0 +1,15 @@ +package bigint + +import ( + "math/big" + "testing" +) + +func BenchmarkToPreallocatedBytes(b *testing.B) { + v := big.NewInt(100500) + buf := make([]byte, 4) + + for i := 0; i < b.N; i++ { + _ = ToPreallocatedBytes(v, buf[:0]) + } +} diff --git a/pkg/encoding/bigint/bigint.go b/pkg/encoding/bigint/bigint.go index 35081992a..ffb11a591 100644 --- a/pkg/encoding/bigint/bigint.go +++ b/pkg/encoding/bigint/bigint.go @@ -1,7 +1,6 @@ package bigint import ( - "encoding/binary" "math/big" "math/bits" @@ -109,36 +108,26 @@ func ToBytes(n *big.Int) []byte { func ToPreallocatedBytes(n *big.Int, data []byte) []byte { sign := n.Sign() if sign == 0 { - return data + return data[:0] } if sign < 0 { n.Add(n, bigOne) defer func() { n.Sub(n, bigOne) }() if n.Sign() == 0 { // n == -1 - return append(data, 0xFF) + return append(data[:0], 0xFF) } } - var ws = n.Bits() + lb := n.BitLen()/8 + 1 - lb := len(ws) * wordSizeBytes if c := cap(data); c < lb { - data = make([]byte, lb, lb+1) + data = make([]byte, lb) } else { data = data[:lb] } - data = wordsToBytes(ws, data) - - size := len(data) - for ; data[size-1] == 0; size-- { - } - - data = data[:size] - - if data[size-1]&0x80 != 0 { - data = append(data, 0) - } + _ = n.FillBytes(data) + slice.Reverse(data) if sign == -1 { for i := range data { @@ -148,17 +137,3 @@ func ToPreallocatedBytes(n *big.Int, data []byte) []byte { return data } - -func wordsToBytes(ws []big.Word, bs []byte) []byte { - if wordSizeBytes == 8 { - for i := range ws { - binary.LittleEndian.PutUint64(bs[i*wordSizeBytes:], uint64(ws[i])) - } - } else { - for i := range ws { - binary.LittleEndian.PutUint32(bs[i*wordSizeBytes:], uint32(ws[i])) - } - } - - return bs -} From b0744c2b2122bdd98898bab25e38bf2fdfb87529 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 1 Jun 2022 11:26:36 +0300 Subject: [PATCH 7/9] util: optimize MarshalJSON allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit name old time/op new time/op delta Uint256MarshalJSON-8 230ns ± 6% 83ns ±13% -63.78% (p=0.000 n=8+10) name old alloc/op new alloc/op delta Uint256MarshalJSON-8 320B ± 0% 80B ± 0% -75.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta Uint256MarshalJSON-8 5.00 ± 0% 1.00 ± 0% -80.00% (p=0.000 n=10+10) --- pkg/util/bench_test.go | 13 +++++++++++++ pkg/util/uint160.go | 7 ++++++- pkg/util/uint256.go | 7 ++++++- 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 pkg/util/bench_test.go diff --git a/pkg/util/bench_test.go b/pkg/util/bench_test.go new file mode 100644 index 000000000..916d5b41e --- /dev/null +++ b/pkg/util/bench_test.go @@ -0,0 +1,13 @@ +package util + +import ( + "testing" +) + +func BenchmarkUint256MarshalJSON(b *testing.B) { + v := Uint256{0x01, 0x02, 0x03} + + for i := 0; i < b.N; i++ { + _, _ = v.MarshalJSON() + } +} diff --git a/pkg/util/uint160.go b/pkg/util/uint160.go index 3ac8d0eec..2f107e05e 100644 --- a/pkg/util/uint160.go +++ b/pkg/util/uint160.go @@ -132,7 +132,12 @@ func (u *Uint160) UnmarshalJSON(data []byte) (err error) { // MarshalJSON implements the json marshaller interface. func (u Uint160) MarshalJSON() ([]byte, error) { - return []byte(`"0x` + u.StringLE() + `"`), nil + r := make([]byte, 3+Uint160Size*2+1) + copy(r, `"0x`) + r[len(r)-1] = '"' + slice.Reverse(u[:]) // u is a copy, so we can mangle it in any way. + hex.Encode(r[3:], u[:]) + return r, nil } // UnmarshalYAML implements the YAML Unmarshaler interface. diff --git a/pkg/util/uint256.go b/pkg/util/uint256.go index 303ba5221..8886a261d 100644 --- a/pkg/util/uint256.go +++ b/pkg/util/uint256.go @@ -109,7 +109,12 @@ func (u *Uint256) UnmarshalJSON(data []byte) (err error) { // MarshalJSON implements the json marshaller interface. func (u Uint256) MarshalJSON() ([]byte, error) { - return []byte(`"0x` + u.StringLE() + `"`), nil + r := make([]byte, 3+Uint256Size*2+1) + copy(r, `"0x`) + r[len(r)-1] = '"' + slice.Reverse(u[:]) // u is a copy, so we can mangle it in any way. + hex.Encode(r[3:], u[:]) + return r, nil } // CompareTo compares two Uint256 with each other. Possible output: 1, -1, 0 From 54f75bb99906a43b7b62766a98dd0899bdba0301 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 1 Jun 2022 11:55:16 +0300 Subject: [PATCH 8/9] consensus: don't use WriteArray for PrepareRequests It's convenient, but it's not efficient due to reflection use. --- pkg/consensus/prepare_request.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/consensus/prepare_request.go b/pkg/consensus/prepare_request.go index 3f883c1fd..1c21348f3 100644 --- a/pkg/consensus/prepare_request.go +++ b/pkg/consensus/prepare_request.go @@ -26,7 +26,10 @@ func (p *prepareRequest) EncodeBinary(w *io.BinWriter) { w.WriteBytes(p.prevHash[:]) w.WriteU64LE(p.timestamp) w.WriteU64LE(p.nonce) - w.WriteArray(p.transactionHashes) + w.WriteVarUint(uint64(len(p.transactionHashes))) + for i := range p.transactionHashes { + w.WriteBytes(p.transactionHashes[i][:]) + } if p.stateRootEnabled { w.WriteBytes(p.stateRoot[:]) } From 400cbde32cbb2b6839dc535f00d9f6ef7a476da5 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 1 Jun 2022 15:45:11 +0300 Subject: [PATCH 9/9] state: optimize allocations in (*NEP17Transfer).EncodeBinary --- pkg/core/state/tokens.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/core/state/tokens.go b/pkg/core/state/tokens.go index 6faa69edc..c81d4e32d 100644 --- a/pkg/core/state/tokens.go +++ b/pkg/core/state/tokens.go @@ -177,13 +177,15 @@ func (lg *TokenTransferLog) Size() int { // EncodeBinary implements the io.Serializable interface. func (t *NEP17Transfer) EncodeBinary(w *io.BinWriter) { + var buf [bigint.MaxBytesLen]byte + w.WriteU32LE(uint32(t.Asset)) w.WriteBytes(t.Tx[:]) w.WriteBytes(t.From[:]) w.WriteBytes(t.To[:]) w.WriteU32LE(t.Block) w.WriteU64LE(t.Timestamp) - amount := bigint.ToBytes(&t.Amount) + amount := bigint.ToPreallocatedBytes(&t.Amount, buf[:]) w.WriteVarBytes(amount) }