Browse Source

codec: document concurrency guarantees and fix analyze script warnings

- clearly document on Handle, Decoder and Encoder their concurrency guarantees and usage model
- comment out encWriter and decReader interfaces
  (though the encWriterSwitch and decReaderSwitch honor them still)
- ensure all top level objects have word sizes that are multiples of 4 (preferably 8)
- fix warnings found by megacheck, so we don't ignore any of the warnings anymore
  - remove spurious returns
  - comment out unused variables, consts and functions
  - remove variables which were set but never used
Ugorji Nwoke 7 years ago
parent
commit
16061014c5
10 changed files with 86 additions and 56 deletions
  1. 2 2
      codec/binc.go
  2. 0 1
      codec/cbor.go
  3. 23 10
      codec/decode.go
  4. 13 2
      codec/encode.go
  5. 40 30
      codec/helper.go
  6. 3 3
      codec/helper_not_unsafe.go
  7. 3 3
      codec/helper_unsafe.go
  8. 2 3
      codec/json.go
  9. 0 1
      codec/msgpack.go
  10. 0 1
      codec/simple.go

+ 2 - 2
codec/binc.go

@@ -110,6 +110,7 @@ type bincEncDriver struct {
 	encDriverTrackContainerWriter
 	noBuiltInTypes
 	// encNoSeparator
+	_ [1]uint64 // padding
 }
 
 func (e *bincEncDriver) EncodeNil() {
@@ -182,7 +183,7 @@ func (e *bincEncDriver) encIntegerPrune(bd byte, pos bool, v uint64, lim uint8)
 }
 
 func (e *bincEncDriver) EncodeInt(v int64) {
-	const nbd byte = bincVdNegInt << 4
+	// const nbd byte = bincVdNegInt << 4
 	if v >= 0 {
 		e.encUint(bincVdPosInt<<4, true, uint64(v))
 	} else if v == -1 {
@@ -966,7 +967,6 @@ func (d *bincDecDriver) DecodeNaked() {
 		n.v = valueTypeInt
 		n.i = int64(n.u)
 	}
-	return
 }
 
 //------------------------------------

+ 0 - 1
codec/cbor.go

@@ -705,7 +705,6 @@ func (d *cborDecDriver) DecodeNaked() {
 	if !decodeFurther {
 		d.bdRead = false
 	}
-	return
 }
 
 // -------------------------

+ 23 - 10
codec/decode.go

@@ -16,8 +16,8 @@ import (
 
 // Some tagging information for error messages.
 const (
-	msgBadDesc            = "unrecognized descriptor byte"
-	msgDecCannotExpandArr = "cannot expand go array from %v to stream length: %v"
+	msgBadDesc = "unrecognized descriptor byte"
+	// msgDecCannotExpandArr = "cannot expand go array from %v to stream length: %v"
 )
 
 const (
@@ -42,11 +42,13 @@ var (
 	errMaxDepthExceeded             = errors.New("maximum decoding depth exceeded")
 )
 
+/*
+
 // decReader abstracts the reading source, allowing implementations that can
 // read from an io.Reader or directly off a byte slice with zero-copying.
 //
 // Deprecated: Use decReaderSwitch instead.
-type __decReader interface {
+type decReader interface {
 	unreadn1()
 	// readx will use the implementation scratch buffer if possible i.e. n < len(scratchbuf), OR
 	// just return a view of the []byte being decoded from.
@@ -66,6 +68,8 @@ type __decReader interface {
 	readUntil(in []byte, stop byte) (out []byte)
 }
 
+*/
+
 type decDriver interface {
 	// this will check if the next token is a break.
 	CheckBreak() bool
@@ -263,6 +267,8 @@ type bufioDecReader struct {
 	c   int // cursor
 	buf []byte
 	err error
+
+	_ [3]uint64 // padding
 }
 
 func (z *bufioDecReader) reset(r io.Reader) {
@@ -615,6 +621,7 @@ type ioDecReader struct {
 	br io.ByteScanner
 
 	x [scratchByteArrayLen]byte // for: get struct field name, swallow valueTypeBytes, etc
+	_ [1]uint64                 // padding
 }
 
 func (z *ioDecReader) reset(r io.Reader) {
@@ -864,7 +871,6 @@ func (z *bytesDecReader) unreadn1() {
 	}
 	z.c--
 	// z.a++
-	return
 }
 
 // // go:noinline
@@ -1926,10 +1932,10 @@ func (n *decNaked) reset() {
 	n.li, n.lm, n.ln, n.ls = 0, 0, 0, 0
 }
 
-type rtid2rv struct {
-	rtid uintptr
-	rv   reflect.Value
-}
+// type rtid2rv struct {
+// 	rtid uintptr
+// 	rv   reflect.Value
+// }
 
 // --------------
 
@@ -2198,7 +2204,14 @@ func (z *decReaderSwitch) readUntilIO(in []byte, stop byte) (out []byte) {
 	return z.ri.readUntil(in, stop)
 }
 
-// A Decoder reads and decodes an object from an input stream in the codec format.
+// Decoder reads and decodes an object from an input stream in a supported format.
+//
+// Decoder is NOT safe for concurrent use i.e. a Decoder cannot be used
+// concurrently in multiple goroutines.
+//
+// However, as Decoder could be allocation heavy to initialize, a Reset method is provided
+// so its state can be reused to decode new input streams repeatedly.
+// This is the idiomatic way to use.
 type Decoder struct {
 	panicHdl
 	// hopefully, reduce derefencing cost by laying the decReader inside the Decoder.
@@ -2259,7 +2272,7 @@ func NewDecoderBytes(in []byte, h Handle) *Decoder {
 	return d
 }
 
-var defaultDecNaked decNaked
+// var defaultDecNaked decNaked
 
 func newDecoder(h Handle) *Decoder {
 	d := &Decoder{h: h.getBasicHandle(), err: errDecoderNotInitialized}

+ 13 - 2
codec/encode.go

@@ -19,11 +19,13 @@ const defEncByteBufSize = 1 << 6 // 4:16, 6:64, 8:256, 10:1024
 
 var errEncoderNotInitialized = errors.New("Encoder not initialized")
 
+/*
+
 // encWriter abstracts writing to a byte array or to an io.Writer.
 //
 //
 // Deprecated: Use encWriterSwitch instead.
-type __encWriter interface {
+type encWriter interface {
 	writeb([]byte)
 	writestr(string)
 	writen1(byte)
@@ -31,6 +33,8 @@ type __encWriter interface {
 	atEndOfEncode()
 }
 
+*/
+
 // encDriver abstracts the actual codec (binc vs msgpack, etc)
 type encDriver interface {
 	EncodeNil()
@@ -1214,7 +1218,14 @@ func (z *encWriterSwitch) atEndOfEncode() {
 
 */
 
-// An Encoder writes an object to an output stream in the codec format.
+// Encoder writes an object to an output stream in a supported format.
+//
+// Encoder is NOT safe for concurrent use i.e. a Encoder cannot be used
+// concurrently in multiple goroutines.
+//
+// However, as Encoder could be allocation heavy to initialize, a Reset method is provided
+// so its state can be reused to decode new input streams repeatedly.
+// This is the idiomatic way to use.
 type Encoder struct {
 	panicHdl
 	// hopefully, reduce derefencing cost by laying the encWriter inside the Encoder

+ 40 - 30
codec/helper.go

@@ -389,7 +389,7 @@ var (
 	intBitsize  = uint8(intTyp.Bits())
 	uintBitsize = uint8(uintTyp.Bits())
 
-	bsAll0x00 = []byte{0, 0, 0, 0, 0, 0, 0, 0}
+	// bsAll0x00 = []byte{0, 0, 0, 0, 0, 0, 0, 0}
 	bsAll0xff = []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
 
 	chkOvf checkOverflow
@@ -546,11 +546,18 @@ func (x *BasicHandle) getTypeInfo(rtid uintptr, rt reflect.Type) (pti *typeInfo)
 	return x.TypeInfos.get(rtid, rt)
 }
 
-// Handle is the interface for a specific encoding format.
+// Handle defines a specific encoding format. It also stores any runtime state
+// used during an Encoding or Decoding session e.g. stored state about Types, etc.
 //
-// Typically, a Handle is pre-configured before first time use,
-// and not modified while in use. Such a pre-configured Handle
-// is safe for concurrent access.
+// Once a handle is configured, it can be shared across multiple Encoders and Decoders.
+//
+// Note that a Handle is NOT safe for concurrent modification.
+// Consequently, do not modify it after it is configured if shared among
+// multiple Encoders and Decoders in different goroutines.
+//
+// Consequently, the typical usage model is that a Handle is pre-configured
+// before first time use, and not modified while in use.
+// Such a pre-configured Handle is safe for concurrent access.
 type Handle interface {
 	Name() string
 	getBasicHandle() *BasicHandle
@@ -1242,7 +1249,7 @@ func (x *TypeInfos) get(rtid uintptr, rt reflect.Type) (pti *typeInfo) {
 	sp := x.infos.load()
 	var idx int
 	if sp != nil {
-		idx, pti = x.find(sp, rtid)
+		_, pti = x.find(sp, rtid)
 		if pti != nil {
 			return
 		}
@@ -1740,6 +1747,7 @@ func (c *codecFner) reset(hh Handle) {
 	if !hhSame {
 		// c.hh = hh
 		c.h, bh = bh, c.h // swap both
+		_ = bh
 		_, c.js = hh.(*JsonHandle)
 		c.be = hh.isBinary()
 		if len(c.s) > 0 {
@@ -2293,9 +2301,11 @@ type bitset256 [32]byte
 func (x *bitset256) isset(pos byte) bool {
 	return x[pos>>3]&(1<<(pos&7)) != 0
 }
-func (x *bitset256) issetv(pos byte) byte {
-	return x[pos>>3] & (1 << (pos & 7))
-}
+
+// func (x *bitset256) issetv(pos byte) byte {
+// 	return x[pos>>3] & (1 << (pos & 7))
+// }
+
 func (x *bitset256) set(pos byte) {
 	x[pos>>3] |= (1 << (pos & 7))
 }
@@ -2353,7 +2363,7 @@ type pooler struct {
 	cfn                                         sync.Pool // for codecFner
 	tiload                                      sync.Pool // for type info loading
 	strRv8, strRv16, strRv32, strRv64, strRv128 sync.Pool // for stringRV
-	buf64, buf128, buf256, buf512, buf1024      sync.Pool // for [...]byte
+	// buf64, buf128, buf256, buf512, buf1024      sync.Pool // for [...]byte
 }
 
 func (p *pooler) init() {
@@ -2363,11 +2373,11 @@ func (p *pooler) init() {
 	p.strRv64.New = func() interface{} { return new([64]sfiRv) }
 	p.strRv128.New = func() interface{} { return new([128]sfiRv) }
 
-	p.buf64.New = func() interface{} { return new([64]byte) }
-	p.buf128.New = func() interface{} { return new([128]byte) }
-	p.buf256.New = func() interface{} { return new([256]byte) }
-	p.buf512.New = func() interface{} { return new([512]byte) }
-	p.buf1024.New = func() interface{} { return new([1024]byte) }
+	// p.buf64.New = func() interface{} { return new([64]byte) }
+	// p.buf128.New = func() interface{} { return new([128]byte) }
+	// p.buf256.New = func() interface{} { return new([256]byte) }
+	// p.buf512.New = func() interface{} { return new([512]byte) }
+	// p.buf1024.New = func() interface{} { return new([1024]byte) }
 
 	p.dn.New = func() interface{} { x := new(decNaked); x.init(); return x }
 
@@ -2392,21 +2402,21 @@ func (p *pooler) sfiRv128() (sp *sync.Pool, v interface{}) {
 	return &p.strRv128, p.strRv128.Get()
 }
 
-func (p *pooler) bytes64() (sp *sync.Pool, v interface{}) {
-	return &p.buf64, p.buf64.Get()
-}
-func (p *pooler) bytes128() (sp *sync.Pool, v interface{}) {
-	return &p.buf128, p.buf128.Get()
-}
-func (p *pooler) bytes256() (sp *sync.Pool, v interface{}) {
-	return &p.buf256, p.buf256.Get()
-}
-func (p *pooler) bytes512() (sp *sync.Pool, v interface{}) {
-	return &p.buf512, p.buf512.Get()
-}
-func (p *pooler) bytes1024() (sp *sync.Pool, v interface{}) {
-	return &p.buf1024, p.buf1024.Get()
-}
+// func (p *pooler) bytes64() (sp *sync.Pool, v interface{}) {
+// 	return &p.buf64, p.buf64.Get()
+// }
+// func (p *pooler) bytes128() (sp *sync.Pool, v interface{}) {
+// 	return &p.buf128, p.buf128.Get()
+// }
+// func (p *pooler) bytes256() (sp *sync.Pool, v interface{}) {
+// 	return &p.buf256, p.buf256.Get()
+// }
+// func (p *pooler) bytes512() (sp *sync.Pool, v interface{}) {
+// 	return &p.buf512, p.buf512.Get()
+// }
+// func (p *pooler) bytes1024() (sp *sync.Pool, v interface{}) {
+// 	return &p.buf1024, p.buf1024.Get()
+// }
 
 func (p *pooler) decNaked() (sp *sync.Pool, v interface{}) {
 	return &p.dn, p.dn.Get()

+ 3 - 3
codec/helper_not_unsafe.go

@@ -47,9 +47,9 @@ func rt2id(rt reflect.Type) uintptr {
 	return reflect.ValueOf(rt).Pointer()
 }
 
-func rv2rtid(rv reflect.Value) uintptr {
-	return reflect.ValueOf(rv.Type()).Pointer()
-}
+// func rv2rtid(rv reflect.Value) uintptr {
+// 	return reflect.ValueOf(rv.Type()).Pointer()
+// }
 
 func i2rtid(i interface{}) uintptr {
 	return reflect.ValueOf(reflect.TypeOf(i)).Pointer()

+ 3 - 3
codec/helper_unsafe.go

@@ -97,9 +97,9 @@ func rt2id(rt reflect.Type) uintptr {
 	return uintptr(((*unsafeIntf)(unsafe.Pointer(&rt))).word)
 }
 
-func rv2rtid(rv reflect.Value) uintptr {
-	return uintptr((*unsafeReflectValue)(unsafe.Pointer(&rv)).typ)
-}
+// func rv2rtid(rv reflect.Value) uintptr {
+// 	return uintptr((*unsafeReflectValue)(unsafe.Pointer(&rv)).typ)
+// }
 
 func i2rtid(i interface{}) uintptr {
 	return uintptr(((*unsafeIntf)(unsafe.Pointer(&i))).typ)

+ 2 - 3
codec/json.go

@@ -46,8 +46,8 @@ const (
 	jsonLitTrue   = 1
 	jsonLitFalseQ = 6
 	jsonLitFalse  = 7
-	jsonLitNullQ  = 13
-	jsonLitNull   = 14
+	// jsonLitNullQ  = 13
+	jsonLitNull = 14
 )
 
 const (
@@ -1240,7 +1240,6 @@ func (d *jsonDecDriver) DecodeNaked() {
 	// if decodeFurther {
 	// 	d.s.sc.retryRead()
 	// }
-	return
 }
 
 //----------------------

+ 0 - 1
codec/msgpack.go

@@ -557,7 +557,6 @@ func (d *msgpackDecDriver) DecodeNaked() {
 		n.v = valueTypeInt
 		n.i = int64(n.u)
 	}
-	return
 }
 
 // int can be decoded from msgpack type: intXXX or uintXXX

+ 0 - 1
codec/simple.go

@@ -601,7 +601,6 @@ func (d *simpleDecDriver) DecodeNaked() {
 	if !decodeFurther {
 		d.bdRead = false
 	}
-	return
 }
 
 //------------------------------------