codec: streamline encoding of nil values

Previously, we checked for nil values at the top of each fastpath function only.

However, we missed checking for nil values for
- pointers e.g. *int, *float32, *map, *[]byte, etc

We now comprehensively check for nil values and encode them as nil accordingly.

Also, in codecgen, let call to Read(Map|Array)End be inline with Read(Map|Array)Start

codec/decode.go

@@ -1427,7 +1427,10 @@ func (d *Decoder) swallow() {
 }
 
 func setZero(iv interface{}) {
-	if iv == nil || definitelyNil(iv) {
+	if iv == nil {
+		return
+	}
+	if _, ok := isNilRef(iv); ok {
 		return
 	}
 	var canDecode bool

codec/encode.go

@@ -1075,11 +1075,19 @@ func (e *Encoder) encode(iv interface{}) {
 	// a switch with only concrete types can be optimized.
 	// consequently, we deal with nil and interfaces outside the switch.
 
-	if iv == nil || definitelyNil(iv) {
+	if iv == nil {
 		e.e.EncodeNil()
 		return
 	}
 
+	rv, ok := isNilRef(iv)
+	if ok {
+		e.e.EncodeNil()
+		return
+	}
+
+	var vself Selfer
+
 	switch v := iv.(type) {
 	// case nil:
 	// case Selfer:
@@ -1125,11 +1133,13 @@ func (e *Encoder) encode(iv interface{}) {
 	case time.Time:
 		e.e.EncodeTime(v)
 	case []uint8:
-		e.e.EncodeStringBytesRaw(v)
-
+		if v == nil {
+			e.e.EncodeNil()
+		} else {
+			e.e.EncodeStringBytesRaw(v)
+		}
 	case *Raw:
 		e.rawBytes(*v)
-
 	case *string:
 		if e.h.StringToRaw {
 			e.e.EncodeStringBytesRaw(bytesView(*v))
@@ -1166,16 +1176,20 @@ func (e *Encoder) encode(iv interface{}) {
 		e.e.EncodeFloat64(*v)
 	case *time.Time:
 		e.e.EncodeTime(*v)
-
 	case *[]uint8:
-		e.e.EncodeStringBytesRaw(*v)
-
+		if *v == nil {
+			e.e.EncodeNil()
+		} else {
+			e.e.EncodeStringBytesRaw(*v)
+		}
 	default:
-		if v, ok := iv.(Selfer); ok {
-			v.CodecEncodeSelf(e)
+		if vself, ok = iv.(Selfer); ok {
+			vself.CodecEncodeSelf(e)
 		} else if !fastpathEncodeTypeSwitch(iv, e) {
-			// checkfastpath=true (not false), as underlying slice/map type may be fast-path
-			e.encodeValue(reflect.ValueOf(iv), nil)
+			if !rv.IsValid() {
+				rv = reflect.ValueOf(iv)
+			}
+			e.encodeValue(rv, nil)
 		}
 	}
 }

codec/fast-path.go.tmpl

@@ -113,16 +113,28 @@ func fastpathEncodeTypeSwitch(iv interface{}, e *Encoder) bool {
 	switch v := iv.(type) {
 {{range .Values}}{{if not .Primitive}}{{if not .MapKey }}{{if ne .Elem "uint8" -}}
 	case []{{ .Elem }}:
-		fastpathTV.{{ .MethodNamePfx "Enc" false }}V(v, e)
+		if v == nil {
+			e.e.EncodeNil()
+		} else {
+			fastpathTV.{{ .MethodNamePfx "Enc" false }}V(v, e)
+		}
 	case *[]{{ .Elem }}:
-		fastpathTV.{{ .MethodNamePfx "Enc" false }}V(*v, e)
+		if *v == nil {
+			e.e.EncodeNil()
+		} else {
+			fastpathTV.{{ .MethodNamePfx "Enc" false }}V(*v, e)
+		}
 {{end}}{{end}}{{end}}{{end -}}
 
 {{range .Values}}{{if not .Primitive}}{{if .MapKey -}}
 	case map[{{ .MapKey }}]{{ .Elem }}:
 		fastpathTV.{{ .MethodNamePfx "Enc" false }}V(v, e)
 	case *map[{{ .MapKey }}]{{ .Elem }}:
-		fastpathTV.{{ .MethodNamePfx "Enc" false }}V(*v, e)
+		if *v == nil {
+			e.e.EncodeNil()
+		} else {
+			fastpathTV.{{ .MethodNamePfx "Enc" false }}V(*v, e)
+		}
 {{end}}{{end}}{{end -}}
 
 	default:
@@ -142,10 +154,7 @@ func (e *Encoder) {{ .MethodNamePfx "fastpathEnc" false }}R(f *codecFnInfo, rv r
 	}
 }
 func (fastpathT) {{ .MethodNamePfx "Enc" false }}V(v []{{ .Elem }}, e *Encoder) {
-	if v == nil {
-		e.e.EncodeNil()
-		return
-	}
+	{{/* if v == nil { e.e.EncodeNil(); return } */ -}}
 	e.arrayStart(len(v))
 	for j := range v {
 		e.arrayElem()
@@ -154,9 +163,10 @@ func (fastpathT) {{ .MethodNamePfx "Enc" false }}V(v []{{ .Elem }}, e *Encoder)
 	e.arrayEnd()
 }
 func (fastpathT) {{ .MethodNamePfx "EncAsMap" false }}V(v []{{ .Elem }}, e *Encoder) {
-	if v == nil {
+	{{/* if v == nil {
 		e.e.EncodeNil()
-    } else if len(v)%2 == 1 {
+    } else */ -}}
+	if len(v)%2 == 1 {
 		e.errorf(fastpathMapBySliceErrMsg, len(v))
 	} else {
 		e.mapStart(len(v) / 2)
@@ -179,7 +189,7 @@ func (e *Encoder) {{ .MethodNamePfx "fastpathEnc" false }}R(f *codecFnInfo, rv r
 	fastpathTV.{{ .MethodNamePfx "Enc" false }}V(rv2i(rv).(map[{{ .MapKey }}]{{ .Elem }}), e)
 }
 func (fastpathT) {{ .MethodNamePfx "Enc" false }}V(v map[{{ .MapKey }}]{{ .Elem }}, e *Encoder) {
-	if v == nil { e.e.EncodeNil(); return }
+	{{/* if v == nil { e.e.EncodeNil(); return } */ -}}
 	e.mapStart(len(v))
 	if e.h.Canonical { {{/* need to figure out .NoCanonical */}}
 		{{if eq .MapKey "interface{}"}}{{/* out of band */ -}}
@@ -299,9 +309,6 @@ func (f fastpathT) {{ .MethodNamePfx "Dec" false }}X(vp *[]{{ .Elem }}, d *Decod
 	if v, changed := f.{{ .MethodNamePfx "Dec" false }}Y(*vp, d); changed { *vp = v }
 }
 func (fastpathT) {{ .MethodNamePfx "Dec" false }}Y(v []{{ .Elem }}, d *Decoder) (_ []{{ .Elem }}, changed bool) {
-	{{/* dd := d.d
-		// if d.d.isContainerType(valueTypeNil) { d.d.TryDecodeAsNil() }
-	 */ -}}
 	slh, containerLenS := d.decSliceHelperStart()
 	if containerLenS == 0 {
 		if v == nil { v = []{{ .Elem }}{} } else if len(v) != 0 { v = v[:0] }
@@ -358,9 +365,6 @@ func (fastpathT) {{ .MethodNamePfx "Dec" false }}Y(v []{{ .Elem }}, d *Decoder)
 	return v, changed 
 }
 func (fastpathT) {{ .MethodNamePfx "Dec" false }}N(v []{{ .Elem }}, d *Decoder) {
-	{{/* dd := d.d
-		// if d.d.isContainerType(valueTypeNil) { d.d.TryDecodeAsNil() }
-	 */ -}}
 	slh, containerLenS := d.decSliceHelperStart()
 	if containerLenS == 0 {
 		slh.End()
@@ -397,7 +401,6 @@ Maps can change if they are
 - settable (e.g. contained in an interface{})
 */ -}}
 func (d *Decoder) {{ .MethodNamePfx "fastpathDec" false }}R(f *codecFnInfo, rv reflect.Value) {
-    {{/* // if d.d.isContainerType(valueTypeNil) {d.d.TryDecodeAsNil()	*/ -}}
 	containerLen := d.mapStart()
 	if rv.Kind() == reflect.Ptr {
 		vp := rv2i(rv).(*map[{{ .MapKey }}]{{ .Elem }})

codec/gen.go

@@ -697,8 +697,10 @@ func (x *genRunner) registerXtraT(t reflect.Type) {
 // The parameter, t, is the reflect.Type of the variable itself
 func (x *genRunner) encVar(varname string, t reflect.Type) {
 	var checkNil bool
+	// case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map, reflect.Chan:
+	// do not include checkNil for slice and maps, as we already checkNil below it
 	switch t.Kind() {
-	case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map, reflect.Chan:
+	case reflect.Ptr, reflect.Interface, reflect.Chan:
 		checkNil = true
 	}
 	x.encVarChkNil(varname, t, checkNil)
@@ -706,7 +708,7 @@ func (x *genRunner) encVar(varname string, t reflect.Type) {
 
 func (x *genRunner) encVarChkNil(varname string, t reflect.Type, checkNil bool) {
 	if checkNil {
-		x.linef("if %s == nil { r.EncodeNil() } else { ", varname)
+		x.linef("if %s == nil { r.EncodeNil() } else {", varname)
 	}
 
 	switch t.Kind() {
@@ -874,6 +876,7 @@ func (x *genRunner) enc(varname string, t reflect.Type) {
 		// else write encode function in-line.
 		// - if elements are primitives or Selfers, call dedicated function on each member.
 		// - else call Encoder.encode(XXX) on it.
+		x.linef("if %s == nil { r.EncodeNil() } else {", varname)
 		if rtid == uint8SliceTypId {
 			x.line("r.EncodeStringBytesRaw([]byte(" + varname + "))")
 		} else if fastpathAV.index(rtid) != -1 {
@@ -883,13 +886,14 @@ func (x *genRunner) enc(varname string, t reflect.Type) {
 			x.xtraSM(varname, t, true, false)
 			// x.encListFallback(varname, rtid, t)
 		}
+		x.linef("} // end block: if %s slice == nil", varname)
 	case reflect.Map:
 		// if nil, call dedicated function
 		// if a known fastpath map, call dedicated function
 		// else write encode function in-line.
 		// - if elements are primitives or Selfers, call dedicated function on each member.
 		// - else call Encoder.encode(XXX) on it.
-		// x.line("if " + varname + " == nil { \nr.EncodeNil()\n } else { ")
+		x.linef("if %s == nil { r.EncodeNil() } else {", varname)
 		if fastpathAV.index(rtid) != -1 {
 			g := x.newFastpathGenV(t)
 			x.line("z.F." + g.MethodNamePfx("Enc", false) + "V(" + varname + ", e)")
@@ -897,6 +901,7 @@ func (x *genRunner) enc(varname string, t reflect.Type) {
 			x.xtraSM(varname, t, true, false)
 			// x.encMapFallback(varname, rtid, t)
 		}
+		x.linef("} // end block: if %s map == nil", varname)
 	case reflect.Struct:
 		if !inlist {
 			delete(x.te, rtid)
@@ -1722,7 +1727,7 @@ func (x *genRunner) decStructMap(varname, lenvarname string, rtid uintptr, t ref
 	x.decStructMapSwitch(kName, varname, rtid, t)
 
 	x.line("} // end for " + tpfx + "j" + i)
-	x.line("z.DecReadMapEnd()")
+	// x.line("z.DecReadMapEnd()")
 }
 
 func (x *genRunner) decStructArray(varname, lenvarname, breakString string, rtid uintptr, t reflect.Type) {
@@ -1756,7 +1761,7 @@ func (x *genRunner) decStructArray(varname, lenvarname, breakString string, rtid
 	x.line("z.DecReadArrayElem()")
 	x.linef(`z.DecStructFieldNotFound(%sj%s - 1, "")`, tpfx, i)
 	x.line("}")
-	x.line("z.DecReadArrayEnd()")
+	// x.line("z.DecReadArrayEnd()")
 }
 
 func (x *genRunner) decStruct(varname string, rtid uintptr, t reflect.Type) {
@@ -1766,7 +1771,6 @@ func (x *genRunner) decStruct(varname string, rtid uintptr, t reflect.Type) {
 	x.linef("if %sct%s == codecSelferValueTypeMap%s {", genTempVarPfx, i, x.xs)
 	x.line(genTempVarPfx + "l" + i + " := z.DecReadMapStart()")
 	x.linef("if %sl%s == 0 {", genTempVarPfx, i)
-	x.line("z.DecReadMapEnd()")
 	if genUseOneFunctionForDecStructMap {
 		x.line("} else { ")
 		x.linef("%s.codecDecodeSelfFromMap(%sl%s, d)", varname, genTempVarPfx, i)
@@ -1777,15 +1781,15 @@ func (x *genRunner) decStruct(varname string, rtid uintptr, t reflect.Type) {
 		x.line(varname + ".codecDecodeSelfFromMapCheckBreak(" + genTempVarPfx + "l" + i + ", d)")
 	}
 	x.line("}")
+	x.line("z.DecReadMapEnd()")
 
 	// else if container is array
 	x.linef("} else if %sct%s == codecSelferValueTypeArray%s {", genTempVarPfx, i, x.xs)
 	x.line(genTempVarPfx + "l" + i + " := z.DecReadArrayStart()")
-	x.linef("if %sl%s == 0 {", genTempVarPfx, i)
-	x.line("z.DecReadArrayEnd()")
-	x.line("} else { ")
+	x.linef("if %sl%s != 0 {", genTempVarPfx, i)
 	x.linef("%s.codecDecodeSelfFromArray(%sl%s, d)", varname, genTempVarPfx, i)
 	x.line("}")
+	x.line("z.DecReadArrayEnd()")
 	// else panic
 	x.line("} else { ")
 	x.line("panic(errCodecSelferOnlyMapOrArrayEncodeToStruct" + x.xs + ")")

codec/helper.go

@@ -161,6 +161,7 @@ var zeroByteSlice = oneByteArr[:0:0]
 var codecgen bool
 
 var refBitset bitset256
+var isnilBitset bitset256
 var pool pooler
 var panicv panicHdl
 
@@ -172,6 +173,14 @@ func init() {
 	refBitset.set(byte(reflect.Func))
 	refBitset.set(byte(reflect.Chan))
 	refBitset.set(byte(reflect.UnsafePointer))
+
+	isnilBitset.set(byte(reflect.Map))
+	isnilBitset.set(byte(reflect.Ptr))
+	isnilBitset.set(byte(reflect.Func))
+	isnilBitset.set(byte(reflect.Chan))
+	isnilBitset.set(byte(reflect.UnsafePointer))
+	isnilBitset.set(byte(reflect.Interface))
+	isnilBitset.set(byte(reflect.Slice))
 }
 
 type handleFlag uint8

codec/helper_not_unsafe.go

@@ -33,10 +33,25 @@ func bytesView(v string) []byte {
 	return []byte(v)
 }
 
-func definitelyNil(v interface{}) bool {
-	// this is a best-effort option.
-	// We just return false, so we don't unnecessarily incur the cost of reflection this early.
-	return false
+// isNilRef says whether the interface is a nil reference or not.
+//
+// A reference here is a pointer-sized reference i.e. map, ptr, chan, func, unsafepointer.
+// It is optional to extend this to also check if slices or interfaces are nil also.
+func isNilRef(v interface{}) (rv reflect.Value, isnil bool) {
+	// this is a best-effort option - you should not pay any cost for this.
+	// It is ok to return false, so we don't unnecessarily pay for reflection this early.
+
+	rv = reflect.ValueOf(v)
+	if isnilBitset.isset(byte(rv.Kind())) {
+		isnil = rv.IsNil()
+	}
+	return
+
+	// switch rv.Kind() {
+	// case reflect.Invalid, reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map, reflect.Func, reflect.Chan:
+	// 	return rv.IsNil()
+	// }
+	// return false
 }
 
 func rv2i(rv reflect.Value) interface{} {
@@ -341,15 +356,3 @@ func mapDelete(m, k reflect.Value) {
 func mapAddressableRV(t reflect.Type) (r reflect.Value) {
 	return // reflect.New(t).Elem()
 }
-
-// func definitelyNil(v interface{}) bool {
-// 	rv := reflect.ValueOf(v)
-// 	switch rv.Kind() {
-// 	case reflect.Invalid:
-// 		return true
-// 	case reflect.Ptr, reflect.Interface, reflect.Chan, reflect.Slice, reflect.Map, reflect.Func:
-// 		return rv.IsNil()
-// 	default:
-// 		return false
-// 	}
-// }

codec/helper_unsafe.go

@@ -76,20 +76,19 @@ func bytesView(v string) []byte {
 	return *(*[]byte)(unsafe.Pointer(&unsafeSlice{sx.Data, sx.Len, sx.Len}))
 }
 
-func definitelyNil(v interface{}) bool {
+// isNilRef says whether the interface is a nil reference or not.
+//
+// A reference here is a pointer-sized reference i.e. map, ptr, chan, func, unsafepointer.
+// It is optional to extend this to also check if slices or interfaces are nil also.
+func isNilRef(v interface{}) (rv reflect.Value, isnil bool) {
 	// There is no global way of checking if an interface is nil.
 	// For true references (map, ptr, func, chan), you can just look
-	// at the word of the interface. However, for slices, you have to dereference
+	// at the word of the interface.
+	// However, for slices, you have to dereference
 	// the word, and get a pointer to the 3-word interface value.
-	//
-	// However, the following are cheap calls
-	// - TypeOf(interface): cheap 2-line call.
-	// - ValueOf(interface{}): expensive
-	// - type.Kind: cheap call through an interface
-	// - Value.Type(): cheap call
-	//                 except it's a method value (e.g. r.Read, which implies that it is a Func)
-
-	return ((*unsafeIntf)(unsafe.Pointer(&v))).word == nil
+
+	isnil = ((*unsafeIntf)(unsafe.Pointer(&v))).word == nil
+	return
 }
 
 func rv2ptr(urv *unsafeReflectValue) (ptr unsafe.Pointer) {