From d487b546123894b4736a441b4e148d92d93b75d8 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 4 Aug 2021 23:13:58 +0300 Subject: [PATCH 1/5] transaction: don't recalculate size when decoding from buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit name old time/op new time/op delta DecodeBinary-8 3.17µs ± 6% 3.14µs ± 5% ~ (p=0.579 n=10+10) DecodeJSON-8 12.8µs ± 3% 12.6µs ± 3% ~ (p=0.105 n=10+10) DecodeFromBytes-8 3.45µs ± 4% 2.73µs ± 2% -20.70% (p=0.000 n=10+9) name old alloc/op new alloc/op delta DecodeBinary-8 1.82kB ± 0% 1.82kB ± 0% ~ (all equal) DecodeJSON-8 3.49kB ± 0% 3.49kB ± 0% ~ (all equal) DecodeFromBytes-8 1.82kB ± 0% 1.44kB ± 0% -21.05% (p=0.000 n=10+10) name old allocs/op new allocs/op delta DecodeBinary-8 29.0 ± 0% 29.0 ± 0% ~ (all equal) DecodeJSON-8 58.0 ± 0% 58.0 ± 0% ~ (all equal) DecodeFromBytes-8 29.0 ± 0% 21.0 ± 0% -27.59% (p=0.000 n=10+10) --- pkg/core/transaction/bench_test.go | 55 +++++++++++++++++++++++++++++ pkg/core/transaction/transaction.go | 13 +++++-- 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 pkg/core/transaction/bench_test.go diff --git a/pkg/core/transaction/bench_test.go b/pkg/core/transaction/bench_test.go new file mode 100644 index 000000000..1cf85a1cb --- /dev/null +++ b/pkg/core/transaction/bench_test.go @@ -0,0 +1,55 @@ +package transaction + +import ( + "encoding/base64" + "testing" + + "github.com/nspcc-dev/neo-go/pkg/io" + "github.com/stretchr/testify/require" +) + +// Some typical transfer tx from mainnet. +var ( + benchTx []byte + benchTxB64 = "AK9KzFu0P5gAAAAAAIjOEgAAAAAA7jAAAAGIDdjSt7aj2J+dktSobkC9j0/CJwEAWwsCAMLrCwwUtXfkIuockX9HAVMNeEuQMxMlYkMMFIgN2NK3tqPYn52S1KhuQL2PT8InFMAfDAh0cmFuc2ZlcgwUz3bii9AGLEpHjuNVYQETGfPPpNJBYn1bUjkBQgxAUiZNae4OTSu2EOGW+6fwslLIpVsczOAR9o6R796tFf2KG+nLzs709tCQ7NELZOQ7zUzfF19ADLvH/efNT4v9LygMIQNT96/wFdPSBO7NUI9Kpn9EffTRXsS6ZJ9PqRvbenijVEFW57Mn" + benchTxJSON []byte +) + +func init() { + var err error + benchTx, err = base64.StdEncoding.DecodeString(benchTxB64) + if err != nil { + panic(err) + } + t, err := NewTransactionFromBytes(benchTx) + if err != nil { + panic(err) + } + benchTxJSON, err = t.MarshalJSON() + if err != nil { + panic(err) + } +} + +func BenchmarkDecodeBinary(t *testing.B) { + for n := 0; n < t.N; n++ { + r := io.NewBinReaderFromBuf(benchTx) + tx := &Transaction{} + tx.DecodeBinary(r) + require.NoError(t, r.Err) + } +} + +func BenchmarkDecodeJSON(t *testing.B) { + for n := 0; n < t.N; n++ { + tx := &Transaction{} + require.NoError(t, tx.UnmarshalJSON(benchTxJSON)) + } +} + +func BenchmarkDecodeFromBytes(t *testing.B) { + for n := 0; n < t.N; n++ { + _, err := NewTransactionFromBytes(benchTx) + require.NoError(t, err) + } +} diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index 25809657c..61f91ab48 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -143,8 +143,7 @@ func (t *Transaction) decodeHashableFields(br *io.BinReader) { } } -// DecodeBinary implements Serializable interface. -func (t *Transaction) DecodeBinary(br *io.BinReader) { +func (t *Transaction) decodeBinaryNoSize(br *io.BinReader) { t.decodeHashableFields(br) if br.Err != nil { return @@ -159,6 +158,14 @@ func (t *Transaction) DecodeBinary(br *io.BinReader) { // to do it anymore. if br.Err == nil { br.Err = t.createHash() + } +} + +// DecodeBinary implements Serializable interface. +func (t *Transaction) DecodeBinary(br *io.BinReader) { + t.decodeBinaryNoSize(br) + + if br.Err == nil { _ = t.Size() } } @@ -240,7 +247,7 @@ func (t *Transaction) Bytes() []byte { func NewTransactionFromBytes(b []byte) (*Transaction, error) { tx := &Transaction{} r := io.NewBinReaderFromBuf(b) - tx.DecodeBinary(r) + tx.decodeBinaryNoSize(r) if r.Err != nil { return nil, r.Err } From d2732a71d85d7068ce0500ff45a3fb9da6f8cebe Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 4 Aug 2021 23:17:50 +0300 Subject: [PATCH 2/5] transaction: don't overwrite error and witnesses length check ReadArray() can return some error and we shouldn't overwrite it. At the same time limiting ReadArray() to the number of Signers can make it return wrong error if the number of witnesses actually is bigger than the number of signers, so use MaxAttributes. --- pkg/core/transaction/transaction.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index 61f91ab48..c0344dcc6 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -148,8 +148,8 @@ func (t *Transaction) decodeBinaryNoSize(br *io.BinReader) { if br.Err != nil { return } - br.ReadArray(&t.Scripts, len(t.Signers)) - if len(t.Signers) != len(t.Scripts) { + br.ReadArray(&t.Scripts, MaxAttributes) + if br.Err == nil && len(t.Signers) != len(t.Scripts) { br.Err = fmt.Errorf("%w: %d vs %d", ErrInvalidWitnessNum, len(t.Signers), len(t.Scripts)) return } From 6d10cdc2f6ed974bda66792100ccfb494e4c55f9 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 4 Aug 2021 23:34:57 +0300 Subject: [PATCH 3/5] transaction: avoid ReadArray() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reflection adds some real cost to it: name old time/op new time/op delta DecodeBinary-8 3.14µs ± 5% 2.89µs ± 3% -8.19% (p=0.000 n=10+10) DecodeJSON-8 12.6µs ± 3% 13.0µs ± 1% +3.77% (p=0.000 n=10+10) DecodeFromBytes-8 2.73µs ± 2% 2.37µs ± 1% -13.12% (p=0.000 n=9+9) name old alloc/op new alloc/op delta DecodeBinary-8 1.82kB ± 0% 1.75kB ± 0% -3.95% (p=0.000 n=10+10) DecodeJSON-8 3.49kB ± 0% 3.49kB ± 0% ~ (all equal) DecodeFromBytes-8 1.44kB ± 0% 1.37kB ± 0% -5.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta DecodeBinary-8 29.0 ± 0% 26.0 ± 0% -10.34% (p=0.000 n=10+10) DecodeJSON-8 58.0 ± 0% 58.0 ± 0% ~ (all equal) DecodeFromBytes-8 21.0 ± 0% 18.0 ± 0% -14.29% (p=0.000 n=10+10) --- pkg/core/transaction/transaction.go | 37 +++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index c0344dcc6..4c57d0012 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -135,8 +135,30 @@ func (t *Transaction) decodeHashableFields(br *io.BinReader) { t.SystemFee = int64(br.ReadU64LE()) t.NetworkFee = int64(br.ReadU64LE()) t.ValidUntilBlock = br.ReadU32LE() - br.ReadArray(&t.Signers, MaxAttributes) - br.ReadArray(&t.Attributes, MaxAttributes-len(t.Signers)) + nsigners := br.ReadVarUint() + if br.Err != nil { + return + } + if nsigners > MaxAttributes { + br.Err = errors.New("too many signers") + return + } else if nsigners == 0 { + br.Err = errors.New("missing signers") + return + } + t.Signers = make([]Signer, nsigners) + for i := 0; i < int(nsigners); i++ { + t.Signers[i].DecodeBinary(br) + } + nattrs := br.ReadVarUint() + if nattrs > MaxAttributes-nsigners { + br.Err = errors.New("too many attributes") + return + } + t.Attributes = make([]Attribute, nattrs) + for i := 0; i < int(nattrs); i++ { + t.Attributes[i].DecodeBinary(br) + } t.Script = br.ReadVarBytes(MaxScriptLength) if br.Err == nil { br.Err = t.isValid() @@ -148,11 +170,18 @@ func (t *Transaction) decodeBinaryNoSize(br *io.BinReader) { if br.Err != nil { return } - br.ReadArray(&t.Scripts, MaxAttributes) - if br.Err == nil && len(t.Signers) != len(t.Scripts) { + nscripts := br.ReadVarUint() + if nscripts > MaxAttributes { + br.Err = errors.New("too many witnesses") + return + } else if int(nscripts) != len(t.Signers) { br.Err = fmt.Errorf("%w: %d vs %d", ErrInvalidWitnessNum, len(t.Signers), len(t.Scripts)) return } + t.Scripts = make([]Witness, nscripts) + for i := 0; i < int(nscripts); i++ { + t.Scripts[i].DecodeBinary(br) + } // Create the hash of the transaction at decode, so we dont need // to do it anymore. From 892c9785ad43101c893e08949bfa39905de47937 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 4 Aug 2021 23:43:20 +0300 Subject: [PATCH 4/5] transaction: don't allocate new buffer to calculate hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can write directly to hash.Hash. name old time/op new time/op delta DecodeBinary-8 2.89µs ± 3% 2.82µs ± 5% ~ (p=0.052 n=10+10) DecodeJSON-8 13.0µs ± 1% 12.8µs ± 1% -1.54% (p=0.002 n=10+8) DecodeFromBytes-8 2.37µs ± 1% 2.25µs ± 5% -5.25% (p=0.000 n=9+10) name old alloc/op new alloc/op delta DecodeBinary-8 1.75kB ± 0% 1.53kB ± 0% -12.79% (p=0.000 n=10+10) DecodeJSON-8 3.49kB ± 0% 3.26kB ± 0% -6.42% (p=0.000 n=10+10) DecodeFromBytes-8 1.37kB ± 0% 1.14kB ± 0% -16.37% (p=0.000 n=10+10) name old allocs/op new allocs/op delta DecodeBinary-8 26.0 ± 0% 23.0 ± 0% -11.54% (p=0.000 n=10+10) DecodeJSON-8 58.0 ± 0% 55.0 ± 0% -5.17% (p=0.000 n=10+10) DecodeFromBytes-8 18.0 ± 0% 15.0 ± 0% -16.67% (p=0.000 n=10+10) --- pkg/core/transaction/transaction.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index 4c57d0012..7dd9b8ccf 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -1,6 +1,7 @@ package transaction import ( + "crypto/sha256" "encoding/json" "errors" "fmt" @@ -234,13 +235,14 @@ func (t *Transaction) EncodeHashableFields() ([]byte, error) { // createHash creates the hash of the transaction. func (t *Transaction) createHash() error { - buf := io.NewBufBinWriter() - t.encodeHashableFields(buf.BinWriter) - if buf.Err != nil { - return buf.Err + shaHash := sha256.New() + bw := io.NewBinWriterFromIO(shaHash) + t.encodeHashableFields(bw) + if bw.Err != nil { + return bw.Err } - t.hash = hash.Sha256(buf.Bytes()) + shaHash.Sum(t.hash[:0]) return nil } From 1b186e046b90e6cd029ddc2f894e8a079789336f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 4 Aug 2021 23:11:53 +0300 Subject: [PATCH 5/5] network: use optimized decoder for transactions NewTransactionFromBytes() works a bit faster and uses less memory. --- pkg/network/message.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/network/message.go b/pkg/network/message.go index f2892aae9..5eb360d9d 100644 --- a/pkg/network/message.go +++ b/pkg/network/message.go @@ -130,7 +130,6 @@ func (m *Message) decodePayload() error { buf = d } - r := io.NewBinReaderFromBuf(buf) var p payload.Payload switch m.Command { case CMDVersion: @@ -154,7 +153,12 @@ func (m *Message) decodePayload() error { case CMDHeaders: p = &payload.Headers{StateRootInHeader: m.StateRootInHeader} case CMDTX: - p = &transaction.Transaction{} + p, err := transaction.NewTransactionFromBytes(buf) + if err != nil { + return err + } + m.Payload = p + return nil case CMDMerkleBlock: p = &payload.MerkleBlock{} case CMDPing, CMDPong: @@ -164,6 +168,7 @@ func (m *Message) decodePayload() error { default: return fmt.Errorf("can't decode command %s", m.Command.String()) } + r := io.NewBinReaderFromBuf(buf) p.DecodeBinary(r) if r.Err == nil || r.Err == payload.ErrTooManyHeaders { m.Payload = p