From 16c48a69fdc7b4c164e6c3fcbc3dfc2d119994d0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Sun, 23 May 2021 20:01:08 +0200 Subject: [PATCH 1/3] rlp: optimize big.Int decoding for size <= 32 bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change grows the static integer buffer in Stream to 32 bytes, making it possible to decode 256bit integers without allocating a temporary buffer. In the recent commit 088da24ebfb1, Stream struct size decreased from 120 bytes down to 88 bytes. This commit grows the struct to 112 bytes again, but the size change will not degrade performance because Stream instances are internally cached in sync.Pool. name old time/op new time/op delta DecodeBigInts-8 12.2µs ± 0% 8.6µs ± 4% -29.58% (p=0.000 n=9+10) name old speed new speed delta DecodeBigInts-8 230MB/s ± 0% 326MB/s ± 4% +42.04% (p=0.000 n=9+10) --- rlp/decode.go | 60 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 8121ab2e72a5..ac04d5d56949 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -220,20 +220,51 @@ func decodeBigIntNoPtr(s *Stream, val reflect.Value) error { } func decodeBigInt(s *Stream, val reflect.Value) error { - b, err := s.Bytes() - if err != nil { + var buffer []byte + kind, size, err := s.Kind() + switch { + case err != nil: return wrapStreamError(err, val.Type()) + case kind == List: + return wrapStreamError(ErrExpectedString, val.Type()) + case kind == Byte: + buffer = s.uintbuf[:1] + buffer[0] = s.byteval + s.kind = -1 // re-arm Kind + case size == 0: + // Avoid zero-length read. + s.kind = -1 + case size <= uint64(len(s.uintbuf)): + // For integers smaller than s.uintbuf, allocating a buffer + // can be avoided. + buffer = s.uintbuf[:size] + if err := s.readFull(buffer); err != nil { + return wrapStreamError(err, val.Type()) + } + // Reject inputs where single byte encoding should have been used. + if size == 1 && buffer[0] < 128 { + return wrapStreamError(ErrCanonSize, val.Type()) + } + default: + // For large integers, a temporary buffer is needed. + buffer = make([]byte, size) + if err := s.readFull(buffer); err != nil { + return wrapStreamError(err, val.Type()) + } + } + + // Reject leading zero bytes. + if len(buffer) > 0 && buffer[0] == 0 { + return wrapStreamError(ErrCanonInt, val.Type()) } + + // Set the integer bytes. i := val.Interface().(*big.Int) if i == nil { i = new(big.Int) val.Set(reflect.ValueOf(i)) } - // Reject leading zero bytes. - if len(b) > 0 && b[0] == 0 { - return wrapStreamError(ErrCanonInt, val.Type()) - } - i.SetBytes(b) + i.SetBytes(buffer) return nil } @@ -563,7 +594,7 @@ type Stream struct { size uint64 // size of value ahead kinderr error // error from last readKind stack []uint64 // list sizes - uintbuf [8]byte // auxiliary buffer for integer decoding + uintbuf [32]byte // auxiliary buffer for integer decoding kind Kind // kind of value ahead byteval byte // value of single byte in type tag limited bool // true if input limit is in effect @@ -817,7 +848,7 @@ func (s *Stream) Reset(r io.Reader, inputLimit uint64) { s.kind = -1 s.kinderr = nil s.byteval = 0 - s.uintbuf = [8]byte{} + s.uintbuf = [32]byte{} } // Kind returns the kind and size of the next value in the @@ -927,17 +958,20 @@ func (s *Stream) readUint(size byte) (uint64, error) { b, err := s.readByte() return uint64(b), err default: + buffer := s.uintbuf[:8] + for i := range buffer { + buffer[i] = 0 + } start := int(8 - size) - s.uintbuf = [8]byte{} - if err := s.readFull(s.uintbuf[start:]); err != nil { + if err := s.readFull(buffer[start:]); err != nil { return 0, err } - if s.uintbuf[start] == 0 { + if buffer[start] == 0 { // Note: readUint is also used to decode integer values. // The error needs to be adjusted to become ErrCanonInt in this case. return 0, ErrCanonSize } - return binary.BigEndian.Uint64(s.uintbuf[:]), nil + return binary.BigEndian.Uint64(buffer[:]), nil } } From 7770f259e82ec24b6864fd98cbd84546597a589c Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Sun, 23 May 2021 21:15:07 +0200 Subject: [PATCH 2/3] rlp: add additional test cases for big.Int --- rlp/decode_test.go | 10 +++++++--- rlp/encode_test.go | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 36d254e18eb9..2946a42e93f4 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -405,10 +405,11 @@ type ignoredField struct { } var ( - veryBigInt = big.NewInt(0).Add( + veryBigInt = new(big.Int).Add( big.NewInt(0).Lsh(big.NewInt(0xFFFFFFFFFFFFFF), 16), big.NewInt(0xFFFF), ) + veryVeryBigInt = new(big.Int).Exp(veryBigInt, big.NewInt(8), nil) ) var decodeTests = []decodeTest{ @@ -479,12 +480,15 @@ var decodeTests = []decodeTest{ {input: "C0", ptr: new(string), error: "rlp: expected input string or byte for string"}, // big ints + {input: "80", ptr: new(*big.Int), value: big.NewInt(0)}, {input: "01", ptr: new(*big.Int), value: big.NewInt(1)}, {input: "89FFFFFFFFFFFFFFFFFF", ptr: new(*big.Int), value: veryBigInt}, + {input: "B848FFFFFFFFFFFFFFFFF800000000000000001BFFFFFFFFFFFFFFFFC8000000000000000045FFFFFFFFFFFFFFFFC800000000000000001BFFFFFFFFFFFFFFFFF8000000000000000001", ptr: new(*big.Int), value: veryVeryBigInt}, {input: "10", ptr: new(big.Int), value: *big.NewInt(16)}, // non-pointer also works {input: "C0", ptr: new(*big.Int), error: "rlp: expected input string or byte for *big.Int"}, - {input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, - {input: "8105", ptr: new(big.Int), error: "rlp: non-canonical size information for *big.Int"}, + {input: "00", ptr: new(*big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, + {input: "820001", ptr: new(*big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, + {input: "8105", ptr: new(*big.Int), error: "rlp: non-canonical size information for *big.Int"}, // structs { diff --git a/rlp/encode_test.go b/rlp/encode_test.go index 08a2a84c623a..25d4aac26726 100644 --- a/rlp/encode_test.go +++ b/rlp/encode_test.go @@ -131,6 +131,14 @@ var encTests = []encTest{ val: big.NewInt(0).SetBytes(unhex("010000000000000000000000000000000000000000000000000000000000000000")), output: "A1010000000000000000000000000000000000000000000000000000000000000000", }, + { + val: veryBigInt, + output: "89FFFFFFFFFFFFFFFFFF", + }, + { + val: veryVeryBigInt, + output: "B848FFFFFFFFFFFFFFFFF800000000000000001BFFFFFFFFFFFFFFFFC8000000000000000045FFFFFFFFFFFFFFFFC800000000000000001BFFFFFFFFFFFFFFFFF8000000000000000001", + }, // non-pointer big.Int {val: *big.NewInt(0), output: "80"}, From 00a22fa564ea38d100f9a0cd1401be8a466f20a8 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Sun, 23 May 2021 21:19:53 +0200 Subject: [PATCH 3/3] rlp: add test case for zero-length big.Int in struct My initial implementation of the optimization contained a bug: the Stream didn't advance when encountering an empty big.Int because s.kind was not re-armed. The test checks for this. --- rlp/decode_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 2946a42e93f4..7c3dafeac44d 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -329,6 +329,11 @@ type recstruct struct { Child *recstruct `rlp:"nil"` } +type bigIntStruct struct { + I *big.Int + B string +} + type invalidNilTag struct { X []byte `rlp:"nil"` } @@ -501,6 +506,13 @@ var decodeTests = []decodeTest{ ptr: new(recstruct), value: recstruct{1, &recstruct{2, &recstruct{3, nil}}}, }, + { + // This checks that empty big.Int works correctly in struct context. It's easy to + // miss the update of s.kind for this case, so it needs its own test. + input: "C58083343434", + ptr: new(bigIntStruct), + value: bigIntStruct{new(big.Int), "444"}, + }, // struct errors {