codec: honor go's selection rule for using field at shallowest depth.

To solve this, we need to
- gather all the fields into a list (regardless of depth),
- iterate on them to eliminate fields with same name but at larger depths

In addition,
- we now hardcode fastpathEnabled in the files directly.
  Remove it from helper.go, as it's now in fastpath.not.go and in fastpath.generated.go
- rearrange slice update to reduce bounds-checks

Fixes #156

codec/encode.go

@@ -242,8 +242,8 @@ func (z *bytesEncWriter) writen1(b1 byte) {
 
 func (z *bytesEncWriter) writen2(b1 byte, b2 byte) {
 	c := z.grow(2)
-	z.b[c] = b1
 	z.b[c+1] = b2
+	z.b[c] = b1
 }
 
 func (z *bytesEncWriter) atEndOfEncode() {

codec/fast-path.generated.go

@@ -38,6 +38,8 @@ import (
 	"sort"
 )
 
+const fastpathEnabled = true
+
 const fastpathCheckNilFalse = false // for reflect
 const fastpathCheckNilTrue = true   // for type switch
 
@@ -81,9 +83,6 @@ var fastpathAV fastpathA
 
 // due to possible initialization loop error, make fastpath in an init()
 func init() {
-	if !fastpathEnabled {
-		return
-	}
 	i := 0
 	fn := func(v interface{}, fe func(*encFnInfo, reflect.Value), fd func(*decFnInfo, reflect.Value)) (f fastpathE) {
 		xrt := reflect.TypeOf(v)
@@ -373,9 +372,6 @@ func init() {
 
 // -- -- fast path type switch
 func fastpathEncodeTypeSwitch(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 
 	case []interface{}:
@@ -1741,9 +1737,6 @@ func fastpathEncodeTypeSwitch(iv interface{}, e *Encoder) bool {
 }
 
 func fastpathEncodeTypeSwitchSlice(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 
 	case []interface{}:
@@ -1829,9 +1822,6 @@ func fastpathEncodeTypeSwitchSlice(iv interface{}, e *Encoder) bool {
 }
 
 func fastpathEncodeTypeSwitchMap(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 
 	case map[interface{}]interface{}:
@@ -15954,9 +15944,6 @@ func (_ fastpathT) EncMapBoolBoolV(v map[bool]bool, checkNil bool, e *Encoder) {
 
 // -- -- fast path type switch
 func fastpathDecodeTypeSwitch(iv interface{}, d *Decoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 
 	case []interface{}:

codec/fast-path.go.tmpl

@@ -38,6 +38,8 @@ import (
 	"sort"
 )
 
+const fastpathEnabled = true
+
 const fastpathCheckNilFalse = false // for reflect
 const fastpathCheckNilTrue = true // for type switch
 
@@ -81,9 +83,6 @@ var fastpathAV fastpathA
 
 // due to possible initialization loop error, make fastpath in an init()
 func init() {
-	if !fastpathEnabled {
-		return
-	}
 	i := 0
 	fn := func(v interface{}, fe func(*encFnInfo, reflect.Value), fd func(*decFnInfo, reflect.Value)) (f fastpathE) {
 		xrt := reflect.TypeOf(v)
@@ -106,9 +105,6 @@ func init() {
 
 // -- -- fast path type switch
 func fastpathEncodeTypeSwitch(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 {{range .Values}}{{if not .Primitive}}{{if not .MapKey }}
 	case []{{ .Elem }}:{{else}}
@@ -126,9 +122,6 @@ func fastpathEncodeTypeSwitch(iv interface{}, e *Encoder) bool {
 }
 
 func fastpathEncodeTypeSwitchSlice(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 {{range .Values}}{{if not .Primitive}}{{if not .MapKey }}
 	case []{{ .Elem }}:
@@ -144,9 +137,6 @@ func fastpathEncodeTypeSwitchSlice(iv interface{}, e *Encoder) bool {
 }
 
 func fastpathEncodeTypeSwitchMap(iv interface{}, e *Encoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 {{range .Values}}{{if not .Primitive}}{{if .MapKey }}
 	case map[{{ .MapKey }}]{{ .Elem }}:
@@ -286,9 +276,6 @@ func (_ fastpathT) {{ .MethodNamePfx "Enc" false }}V(v map[{{ .MapKey }}]{{ .Ele
 
 // -- -- fast path type switch
 func fastpathDecodeTypeSwitch(iv interface{}, d *Decoder) bool {
-	if !fastpathEnabled {
-		return false
-	}
 	switch v := iv.(type) {
 {{range .Values}}{{if not .Primitive}}{{if not .MapKey }}
 	case []{{ .Elem }}:{{else}}

codec/fast-path.not.go

@@ -4,6 +4,8 @@ package codec
 
 import "reflect"
 
+const fastpathEnabled = false
+
 // The generated fast-path code is very large, and adds a few seconds to the build time.
 // This causes test execution, execution of small tools which use codec, etc
 // to take a long time.

codec/helper.go

@@ -137,10 +137,6 @@ const (
 	// Note that this will always cause rpc tests to fail, since they need io.EOF sent via panic.
 	recoverPanicToErr = true
 
-	// Fast path functions try to create a fast path encode or decode implementation
-	// for common maps and slices, by by-passing reflection altogether.
-	fastpathEnabled = true
-
 	// if checkStructForEmptyValue, check structs fields to see if an empty value.
 	// This could be an expensive call, so possibly disable it.
 	checkStructForEmptyValue = false
@@ -217,16 +213,21 @@ const (
 	containerArrayEnd
 )
 
-type rgetPoolT struct {
-	encNames [8]string
-	fNames   [8]string
-	etypes   [8]uintptr
-	sfis     [8]*structFieldInfo
+// sfiIdx used for tracking where a fieldName is seen in a []*structFieldInfo
+type sfiIdx struct {
+	fieldName string
+	index     int
 }
 
-var rgetPool = sync.Pool{
-	New: func() interface{} { return new(rgetPoolT) },
-}
+// do not recurse if a containing type refers to an embedded type
+// which refers back to its containing type (via a pointer).
+// The second time this back-reference happens, break out,
+// so as not to cause an infinite loop.
+const rgetMaxRecursion = 2
+
+// Anecdotally, we believe most types have <= 12 fields.
+// Java's PMD rules set TooManyFields threshold to 15.
+const rgetPoolTArrayLen = 12
 
 type rgetT struct {
 	fNames   []string
@@ -235,6 +236,18 @@ type rgetT struct {
 	sfis     []*structFieldInfo
 }
 
+type rgetPoolT struct {
+	fNames   [rgetPoolTArrayLen]string
+	encNames [rgetPoolTArrayLen]string
+	etypes   [rgetPoolTArrayLen]uintptr
+	sfis     [rgetPoolTArrayLen]*structFieldInfo
+	sfiidx   [rgetPoolTArrayLen]sfiIdx
+}
+
+var rgetPool = sync.Pool{
+	New: func() interface{} { return new(rgetPoolT) },
+}
+
 type containerStateRecv interface {
 	sendContainerState(containerState)
 }
@@ -545,7 +558,7 @@ func (o *extHandle) AddExt(
 func (o *extHandle) SetExt(rt reflect.Type, tag uint64, ext Ext) (err error) {
 	// o is a pointer, because we may need to initialize it
 	if rt.PkgPath() == "" || rt.Kind() == reflect.Interface {
-		err = fmt.Errorf("codec.Handle.AddExt: Takes named type, especially not a pointer or interface: %T",
+		err = fmt.Errorf("codec.Handle.AddExt: Takes named type, not a pointer or interface: %T",
 			reflect.Zero(rt).Interface())
 		return
 	}
@@ -588,7 +601,8 @@ func (o extHandle) getExtForTag(tag uint64) *extTypeTagFn {
 }
 
 type structFieldInfo struct {
-	encName string // encode name
+	encName   string // encode name
+	fieldName string // field name
 
 	// only one of 'i' or 'is' can be set. If 'i' is -1, then 'is' has been set.
 
@@ -849,21 +863,18 @@ func (x *TypeInfos) get(rtid uintptr, rt reflect.Type) (pti *typeInfo) {
 	}
 
 	if rt.Kind() == reflect.Struct {
-		var siInfo *structFieldInfo
+		var omitEmpty bool
 		if f, ok := rt.FieldByName(structInfoFieldName); ok {
-			siInfo = parseStructFieldInfo(structInfoFieldName, x.structTag(f.Tag))
+			siInfo := parseStructFieldInfo(structInfoFieldName, x.structTag(f.Tag))
 			ti.toArray = siInfo.toArray
+			omitEmpty = siInfo.omitEmpty
 		}
 		pi := rgetPool.Get()
 		pv := pi.(*rgetPoolT)
 		pv.etypes[0] = ti.baseId
 		vv := rgetT{pv.fNames[:0], pv.encNames[:0], pv.etypes[:1], pv.sfis[:0]}
-		x.rget(rt, rtid, nil, &vv, siInfo)
-		ti.sfip = make([]*structFieldInfo, len(vv.sfis))
-		ti.sfi = make([]*structFieldInfo, len(vv.sfis))
-		copy(ti.sfip, vv.sfis)
-		sort.Sort(sfiSortedByEncName(vv.sfis))
-		copy(ti.sfi, vv.sfis)
+		x.rget(rt, rtid, omitEmpty, nil, &vv)
+		ti.sfip, ti.sfi = rgetResolveSFI(vv.sfis, pv.sfiidx[:0])
 		rgetPool.Put(pi)
 	}
 	// sfi = sfip
@@ -877,26 +888,17 @@ func (x *TypeInfos) get(rtid uintptr, rt reflect.Type) (pti *typeInfo) {
 	return
 }
 
-func (x *TypeInfos) rget(rt reflect.Type, rtid uintptr,
-	indexstack []int, pv *rgetT, siInfo *structFieldInfo,
+func (x *TypeInfos) rget(rt reflect.Type, rtid uintptr, omitEmpty bool,
+	indexstack []int, pv *rgetT,
 ) {
-	// This will read up the fields and store how to access the value.
-	// It uses the go language's rules for embedding, as below:
-	//   - if a field has been seen while traversing, skip it
-	//   - if an encName has been seen while traversing, skip it
-	//   - if an embedded type has been seen, skip it
+	// Read up fields and store how to access the value.
 	//
-	// Also, per Go's rules, embedded fields must be analyzed AFTER all top-level fields.
+	// It uses go's rules for message selectors,
+	// which say that the field with the shallowest depth is selected.
 	//
 	// Note: we consciously use slices, not a map, to simulate a set.
-	//       Typically, types have < 16 fields, and iteration using equals is faster than maps there
-
-	type anonField struct {
-		ft  reflect.Type
-		idx int
-	}
-
-	var anonFields []anonField
+	//       Typically, types have < 16 fields,
+	//       and iteration using equals is faster than maps there
 
 LOOP:
 	for j, jlen := 0, rt.NumField(); j < jlen; j++ {
@@ -908,7 +910,8 @@ LOOP:
 			continue LOOP
 		}
 
-		// if r1, _ := utf8.DecodeRuneInString(f.Name); r1 == utf8.RuneError || !unicode.IsUpper(r1) {
+		// if r1, _ := utf8.DecodeRuneInString(f.Name);
+		// r1 == utf8.RuneError || !unicode.IsUpper(r1) {
 		if f.PkgPath != "" && !f.Anonymous { // unexported, not embedded
 			continue
 		}
@@ -917,7 +920,8 @@ LOOP:
 			continue
 		}
 		var si *structFieldInfo
-		// if anonymous and no struct tag (or it's blank), and a struct (or pointer to struct), inline it.
+		// if anonymous and no struct tag (or it's blank),
+		// and a struct (or pointer to struct), inline it.
 		if f.Anonymous && fkind != reflect.Interface {
 			doInline := stag == ""
 			if !doInline {
@@ -931,8 +935,31 @@ LOOP:
 					ft = ft.Elem()
 				}
 				if ft.Kind() == reflect.Struct {
-					// handle anonymous fields after handling all the non-anon fields
-					anonFields = append(anonFields, anonField{ft, j})
+					// if etypes contains this, don't call rget again (as fields are already seen here)
+					ftid := reflect.ValueOf(ft).Pointer()
+					// We cannot recurse forever, but we need to track other field depths.
+					// So - we break if we see a type twice (not the first time).
+					// This should be sufficient to handle an embedded type that refers to its
+					// owning type, which then refers to its embedded type.
+					processIt := true
+					numk := 0
+					for _, k := range pv.etypes {
+						if k == ftid {
+							numk++
+							if numk == rgetMaxRecursion {
+								processIt = false
+								break
+							}
+						}
+					}
+					if processIt {
+						pv.etypes = append(pv.etypes, ftid)
+						indexstack2 := make([]int, len(indexstack)+1)
+						copy(indexstack2, indexstack)
+						indexstack2[len(indexstack)] = j
+						// indexstack2 := append(append(make([]int, 0, len(indexstack)+4), indexstack...), j)
+						x.rget(ft, ftid, omitEmpty, indexstack2, pv)
+					}
 					continue
 				}
 			}
@@ -947,11 +974,6 @@ LOOP:
 			panic(noFieldNameToStructFieldInfoErr)
 		}
 
-		for _, k := range pv.fNames {
-			if k == f.Name {
-				continue LOOP
-			}
-		}
 		pv.fNames = append(pv.fNames, f.Name)
 
 		if si == nil {
@@ -959,12 +981,8 @@ LOOP:
 		} else if si.encName == "" {
 			si.encName = f.Name
 		}
+		si.fieldName = f.Name
 
-		for _, k := range pv.encNames {
-			if k == si.encName {
-				continue LOOP
-			}
-		}
 		pv.encNames = append(pv.encNames, si.encName)
 
 		// si.ikind = int(f.Type.Kind())
@@ -978,32 +996,51 @@ LOOP:
 			// si.is = append(append(make([]int, 0, len(indexstack)+4), indexstack...), j)
 		}
 
-		if siInfo != nil {
-			if siInfo.omitEmpty {
-				si.omitEmpty = true
-			}
+		if omitEmpty {
+			si.omitEmpty = true
 		}
 		pv.sfis = append(pv.sfis, si)
 	}
+}
 
-	// now handle anonymous fields
-LOOP2:
-	for _, af := range anonFields {
-		// if etypes contains this, then do not call rget again (as the fields are already seen here)
-		ftid := reflect.ValueOf(af.ft).Pointer()
-		for _, k := range pv.etypes {
-			if k == ftid {
-				continue LOOP2
+// resolves the struct field info get from a call to rget2.
+// Returns a trimmed, unsorted and sorted []*structFieldInfo.
+func rgetResolveSFI(x []*structFieldInfo, pv []sfiIdx) (y, z []*structFieldInfo) {
+	var n int
+	for i, v := range x {
+		xf := v.fieldName
+		var found bool
+		for _, k := range pv {
+			if k.fieldName == xf {
+				if len(v.is) < len(x[k.index].is) {
+					x[k.index] = nil
+					k.index = i
+					n++
+				}
+				found = true
+				break
 			}
 		}
-		pv.etypes = append(pv.etypes, ftid)
+		if !found {
+			pv = append(pv, sfiIdx{xf, i})
+		}
+	}
 
-		indexstack2 := make([]int, len(indexstack)+1)
-		copy(indexstack2, indexstack)
-		indexstack2[len(indexstack)] = af.idx
-		// indexstack2 := append(append(make([]int, 0, len(indexstack)+4), indexstack...), j)
-		x.rget(af.ft, ftid, indexstack2, pv, siInfo)
+	// remove all the nils
+	y = make([]*structFieldInfo, len(x)-n)
+	n = 0
+	for _, v := range x {
+		if v == nil {
+			continue
+		}
+		y[n] = v
+		n++
 	}
+
+	z = make([]*structFieldInfo, len(y))
+	copy(z, y)
+	sort.Sort(sfiSortedByEncName(z))
+	return
 }
 
 func panicToErr(err *error) {