diff options
author | Robert Griesemer <gri@golang.org> | 2014-10-02 12:53:51 -0700 |
---|---|---|
committer | Robert Griesemer <gri@golang.org> | 2014-10-02 12:53:51 -0700 |
commit | e42bc852f86867440d6cf61ae351b2d66b24c272 (patch) | |
tree | a1921cde6262367ffe741113168d85e1441abacc | |
parent | d160479b0b146594d35acc6b6f708df197ed2453 (diff) | |
download | go-e42bc852f86867440d6cf61ae351b2d66b24c272.tar.gz |
encoding/binary: fix error message
In the process, simplified internal sizeOf and
dataSize functions. Minor positive impact on
performance. Added test case.
benchmark old ns/op new ns/op delta
BenchmarkReadSlice1000Int32s 14006 14122 +0.83%
BenchmarkReadStruct 2508 2447 -2.43%
BenchmarkReadInts 921 928 +0.76%
BenchmarkWriteInts 2086 2081 -0.24%
BenchmarkWriteSlice1000Int32s 13440 13497 +0.42%
BenchmarkPutUvarint32 28.5 26.3 -7.72%
BenchmarkPutUvarint64 81.3 76.7 -5.66%
benchmark old MB/s new MB/s speedup
BenchmarkReadSlice1000Int32s 285.58 283.24 0.99x
BenchmarkReadStruct 27.90 28.60 1.03x
BenchmarkReadInts 32.57 32.31 0.99x
BenchmarkWriteInts 14.38 14.41 1.00x
BenchmarkWriteSlice1000Int32s 297.60 296.36 1.00x
BenchmarkPutUvarint32 140.55 151.92 1.08x
BenchmarkPutUvarint64 98.36 104.33 1.06x
Fixes issue 6818.
LGTM=r
R=r
CC=golang-codereviews
https://codereview.appspot.com/149290045
-rw-r--r-- | src/encoding/binary/binary.go | 70 | ||||
-rw-r--r-- | src/encoding/binary/binary_test.go | 23 |
2 files changed, 53 insertions, 40 deletions
diff --git a/src/encoding/binary/binary.go b/src/encoding/binary/binary.go index b5a377430..466bf97c9 100644 --- a/src/encoding/binary/binary.go +++ b/src/encoding/binary/binary.go @@ -200,18 +200,17 @@ func Read(r io.Reader, order ByteOrder, data interface{}) error { } // Fallback to reflect-based decoding. - var v reflect.Value - switch d := reflect.ValueOf(data); d.Kind() { + v := reflect.ValueOf(data) + size := -1 + switch v.Kind() { case reflect.Ptr: - v = d.Elem() + v = v.Elem() + size = dataSize(v) case reflect.Slice: - v = d - default: - return errors.New("binary.Read: invalid type " + d.Type().String()) + size = dataSize(v) } - size, err := dataSize(v) - if err != nil { - return errors.New("binary.Read: " + err.Error()) + if size < 0 { + return errors.New("binary.Read: invalid type " + reflect.TypeOf(data).String()) } d := &decoder{order: order, buf: make([]byte, size)} if _, err := io.ReadFull(r, d.buf); err != nil { @@ -324,68 +323,64 @@ func Write(w io.Writer, order ByteOrder, data interface{}) error { // Fallback to reflect-based encoding. v := reflect.Indirect(reflect.ValueOf(data)) - size, err := dataSize(v) - if err != nil { - return errors.New("binary.Write: " + err.Error()) + size := dataSize(v) + if size < 0 { + return errors.New("binary.Write: invalid type " + reflect.TypeOf(data).String()) } buf := make([]byte, size) e := &encoder{order: order, buf: buf} e.value(v) - _, err = w.Write(buf) + _, err := w.Write(buf) return err } // Size returns how many bytes Write would generate to encode the value v, which // must be a fixed-size value or a slice of fixed-size values, or a pointer to such data. +// If v is neither of these, Size returns -1. func Size(v interface{}) int { - n, err := dataSize(reflect.Indirect(reflect.ValueOf(v))) - if err != nil { - return -1 - } - return n + return dataSize(reflect.Indirect(reflect.ValueOf(v))) } // dataSize returns the number of bytes the actual data represented by v occupies in memory. // For compound structures, it sums the sizes of the elements. Thus, for instance, for a slice // it returns the length of the slice times the element size and does not count the memory -// occupied by the header. -func dataSize(v reflect.Value) (int, error) { +// occupied by the header. If the type of v is not acceptable, dataSize returns -1. +func dataSize(v reflect.Value) int { if v.Kind() == reflect.Slice { - elem, err := sizeof(v.Type().Elem()) - if err != nil { - return 0, err + if s := sizeof(v.Type().Elem()); s >= 0 { + return s * v.Len() } - return v.Len() * elem, nil + return -1 } return sizeof(v.Type()) } -func sizeof(t reflect.Type) (int, error) { +// sizeof returns the size >= 0 of variables for the given type or -1 if the type is not acceptable. +func sizeof(t reflect.Type) int { switch t.Kind() { case reflect.Array: - n, err := sizeof(t.Elem()) - if err != nil { - return 0, err + if s := sizeof(t.Elem()); s >= 0 { + return s * t.Len() } - return t.Len() * n, nil case reflect.Struct: sum := 0 for i, n := 0, t.NumField(); i < n; i++ { - s, err := sizeof(t.Field(i).Type) - if err != nil { - return 0, err + s := sizeof(t.Field(i).Type) + if s < 0 { + return -1 } sum += s } - return sum, nil + return sum case reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128: - return int(t.Size()), nil + return int(t.Size()) } - return 0, errors.New("invalid type " + t.String()) + + return -1 } type coder struct { @@ -595,12 +590,11 @@ func (e *encoder) value(v reflect.Value) { } func (d *decoder) skip(v reflect.Value) { - n, _ := dataSize(v) - d.buf = d.buf[n:] + d.buf = d.buf[dataSize(v):] } func (e *encoder) skip(v reflect.Value) { - n, _ := dataSize(v) + n := dataSize(v) for i := range e.buf[0:n] { e.buf[i] = 0 } diff --git a/src/encoding/binary/binary_test.go b/src/encoding/binary/binary_test.go index c80c90383..8ee595fa4 100644 --- a/src/encoding/binary/binary_test.go +++ b/src/encoding/binary/binary_test.go @@ -289,6 +289,26 @@ func TestUnexportedRead(t *testing.T) { Read(&buf, LittleEndian, &u2) } +func TestReadErrorMsg(t *testing.T) { + var buf bytes.Buffer + read := func(data interface{}) { + err := Read(&buf, LittleEndian, data) + want := "binary.Read: invalid type " + reflect.TypeOf(data).String() + if err == nil { + t.Errorf("%T: got no error; want %q", data, want) + return + } + if got := err.Error(); got != want { + t.Errorf("%T: got %q; want %q", data, got, want) + } + } + read(0) + s := new(struct{}) + read(&s) + p := &s + read(&p) +} + type byteSliceReader struct { remain []byte } @@ -315,8 +335,7 @@ func BenchmarkReadStruct(b *testing.B) { bsr := &byteSliceReader{} var buf bytes.Buffer Write(&buf, BigEndian, &s) - n, _ := dataSize(reflect.ValueOf(s)) - b.SetBytes(int64(n)) + b.SetBytes(int64(dataSize(reflect.ValueOf(s)))) t := s b.ResetTimer() for i := 0; i < b.N; i++ { |