From f157f6aab59d57d47efc02568be6b0b83adcfeda Mon Sep 17 00:00:00 2001 From: Dominique Lefevre Date: Sun, 26 Oct 2025 12:22:07 +0200 Subject: [PATCH 1/4] Have fewer indentations in Decoder.setDefaults(). Less nesting an a switch instead of if ... else if ... else if are easier to read. --- decoder.go | 87 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/decoder.go b/decoder.go index 54c88ec..1f6a722 100644 --- a/decoder.go +++ b/decoder.go @@ -128,53 +128,64 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { for _, f := range struc.fields { vCurrent := v.FieldByName(f.name) - if vCurrent.Type().Kind() == reflect.Struct && f.defaultValue == "" { - errs.merge(d.setDefaults(vCurrent.Type(), vCurrent)) - } else if isPointerToStruct(vCurrent) && f.defaultValue == "" { - errs.merge(d.setDefaults(vCurrent.Elem().Type(), vCurrent.Elem())) + if f.defaultValue == "" { + if vCurrent.Type().Kind() == reflect.Struct { + errs.merge(d.setDefaults(vCurrent.Type(), vCurrent)) + } else if isPointerToStruct(vCurrent) { + errs.merge(d.setDefaults(vCurrent.Elem().Type(), vCurrent.Elem())) + } + continue } - if f.defaultValue != "" && f.isRequired { + if f.isRequired { errs.merge(MultiError{"default-" + f.name: errors.New("required fields cannot have a default value")}) - } else if f.defaultValue != "" && vCurrent.IsZero() && !f.isRequired { - if f.typ.Kind() == reflect.Struct { - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) - } else if f.typ.Kind() == reflect.Slice { - vals := strings.Split(f.defaultValue, "|") + continue + } + if !vCurrent.IsZero() { + continue + } - // check if slice has one of the supported types for defaults - if _, ok := builtinConverters[f.typ.Elem().Kind()]; !ok { - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) - continue - } + switch f.typ.Kind() { + case reflect.Struct: + errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) - defaultSlice := reflect.MakeSlice(f.typ, 0, cap(vals)) - for _, val := range vals { - // this check is to handle if the wrong value is provided - convertedVal := builtinConverters[f.typ.Elem().Kind()](val) - if !convertedVal.IsValid() { - errs.merge(MultiError{"default-" + f.name: fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name)}) - break - } - defaultSlice = reflect.Append(defaultSlice, convertedVal) - } - vCurrent.Set(defaultSlice) - } else if f.typ.Kind() == reflect.Ptr { - t1 := f.typ.Elem() + case reflect.Slice: + vals := strings.Split(f.defaultValue, "|") - if t1.Kind() == reflect.Struct || t1.Kind() == reflect.Slice { - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) - } + // check if slice has one of the supported types for defaults + if _, ok := builtinConverters[f.typ.Elem().Kind()]; !ok { + errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) + continue + } + defaultSlice := reflect.MakeSlice(f.typ, 0, cap(vals)) + for _, val := range vals { // this check is to handle if the wrong value is provided - if convertedVal := convertPointer(t1.Kind(), f.defaultValue); convertedVal.IsValid() { - vCurrent.Set(convertedVal) - } - } else { - // this check is to handle if the wrong value is provided - if convertedVal := builtinConverters[f.typ.Kind()](f.defaultValue); convertedVal.IsValid() { - vCurrent.Set(builtinConverters[f.typ.Kind()](f.defaultValue)) + convertedVal := builtinConverters[f.typ.Elem().Kind()](val) + if !convertedVal.IsValid() { + errs.merge(MultiError{"default-" + f.name: fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name)}) + break } + defaultSlice = reflect.Append(defaultSlice, convertedVal) + } + vCurrent.Set(defaultSlice) + + case reflect.Ptr: + t1 := f.typ.Elem() + + if t1.Kind() == reflect.Struct || t1.Kind() == reflect.Slice { + errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) + } + + // this check is to handle if the wrong value is provided + if convertedVal := convertPointer(t1.Kind(), f.defaultValue); convertedVal.IsValid() { + vCurrent.Set(convertedVal) + } + + default: + // this check is to handle if the wrong value is provided + if convertedVal := builtinConverters[f.typ.Kind()](f.defaultValue); convertedVal.IsValid() { + vCurrent.Set(builtinConverters[f.typ.Kind()](f.defaultValue)) } } } From cc6a7ee2319ae9a00abfc3e97aeff63f3553876c Mon Sep 17 00:00:00 2001 From: Dominique Lefevre Date: Sun, 26 Oct 2025 13:00:58 +0200 Subject: [PATCH 2/4] Replace MultiError.merge() with MultiError.add() in Decoder.setDefaults(). Using MultiError.merge() forces the caller to allocate an instance of MultiError just to add one error. Let us have a method add() that does this a temporary map. Also, add constants for errors that are copied- and-pasted. This may seem a minor performance improvement to skip an allocation, but in fact it is not. The go compiler detects that a temporary MultiError instance does not escape to the heap, and allocates it on the stack. Go 1.25 on amd64 would use 2408 bytes for the local variables of setDefaults(). This change reduces the local variables frame to 840 bytes. Large stack usage by setDefaults() is in fact very costly. Goroutines start with a small stack. A big frame for local variables makes it very probable that the go runtime will need to grow the stack upon a call to setDefaults(). In a service where I have observed this behaviour, Decoder.Decode() spends 3x more time in runtime.newstack() than in the rest of Decoder.Decode(). --- decoder.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/decoder.go b/decoder.go index 1f6a722..3983669 100644 --- a/decoder.go +++ b/decoder.go @@ -138,7 +138,7 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { } if f.isRequired { - errs.merge(MultiError{"default-" + f.name: errors.New("required fields cannot have a default value")}) + errs.add("default-"+f.name, errRequiredFieldsCantHaveDefaults) continue } if !vCurrent.IsZero() { @@ -147,14 +147,14 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { switch f.typ.Kind() { case reflect.Struct: - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) + errs.add("default-"+f.name, errUnsupportedDefaultFieldType) case reflect.Slice: vals := strings.Split(f.defaultValue, "|") // check if slice has one of the supported types for defaults if _, ok := builtinConverters[f.typ.Elem().Kind()]; !ok { - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) + errs.add("default-"+f.name, errUnsupportedDefaultFieldType) continue } @@ -163,7 +163,7 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { // this check is to handle if the wrong value is provided convertedVal := builtinConverters[f.typ.Elem().Kind()](val) if !convertedVal.IsValid() { - errs.merge(MultiError{"default-" + f.name: fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name)}) + errs.add("default-"+f.name, fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name)) break } defaultSlice = reflect.Append(defaultSlice, convertedVal) @@ -174,7 +174,7 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { t1 := f.typ.Elem() if t1.Kind() == reflect.Struct || t1.Kind() == reflect.Slice { - errs.merge(MultiError{"default-" + f.name: errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices")}) + errs.add("default-"+f.name, errUnsupportedDefaultFieldType) } // this check is to handle if the wrong value is provided @@ -557,6 +557,11 @@ type unmarshaler struct { // Errors --------------------------------------------------------------------- +var ( + errRequiredFieldsCantHaveDefaults = errors.New("required fields cannot have a default value") + errUnsupportedDefaultFieldType = errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices") +) + // ConversionError stores information about a failed conversion. type ConversionError struct { Key string // key from the source map. @@ -622,6 +627,12 @@ func (e MultiError) Error() string { return fmt.Sprintf("%s (and %d other errors)", s, len(e)-1) } +func (e MultiError) add(key string, err error) { + if e[key] == nil { + e[key] = err + } +} + func (e MultiError) merge(errors MultiError) { for key, err := range errors { if e[key] == nil { From 2be8d481db64c4f01607c2459e66866d4fc075dd Mon Sep 17 00:00:00 2001 From: Dominique Lefevre Date: Sun, 26 Oct 2025 20:54:50 +0200 Subject: [PATCH 3/4] Make MultiError.merge() and MultiError.add() non-inlineable. Inlining these calls gains nothing performance-wise, but uses a lot of stack. With these two calls de-inlined, setDefaults() uses 528 bytes for the local variables instead of 840. --- decoder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/decoder.go b/decoder.go index 3983669..64c2add 100644 --- a/decoder.go +++ b/decoder.go @@ -627,12 +627,14 @@ func (e MultiError) Error() string { return fmt.Sprintf("%s (and %d other errors)", s, len(e)-1) } +//go:noinline func (e MultiError) add(key string, err error) { if e[key] == nil { e[key] = err } } +//go:noinline func (e MultiError) merge(errors MultiError) { for key, err := range errors { if e[key] == nil { From e62223fd9671b959fb33bf8fc104566abd7f7dcc Mon Sep 17 00:00:00 2001 From: Dominique Lefevre Date: Sun, 26 Oct 2025 21:02:01 +0200 Subject: [PATCH 4/4] Uninline some more functions from Decoder.setDefaults(). Now setDefaults() needs 312 bytes for local variables instead of 528. --- decoder.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/decoder.go b/decoder.go index 64c2add..9cf46dd 100644 --- a/decoder.go +++ b/decoder.go @@ -119,7 +119,7 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { if v.Type().Kind() == reflect.Struct { for i := 0; i < v.NumField(); i++ { field := v.Field(i) - if field.Type().Kind() == reflect.Ptr && field.IsNil() && v.Type().Field(i).Anonymous { + if field.Kind() == reflect.Ptr && field.IsNil() && isAnonymousField(t, i) { field.Set(reflect.New(field.Type().Elem())) } } @@ -163,7 +163,7 @@ func (d *Decoder) setDefaults(t reflect.Type, v reflect.Value) MultiError { // this check is to handle if the wrong value is provided convertedVal := builtinConverters[f.typ.Elem().Kind()](val) if !convertedVal.IsValid() { - errs.add("default-"+f.name, fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name)) + errs.add("default-"+f.name, errIncompatibleValue(val, f)) break } defaultSlice = reflect.Append(defaultSlice, convertedVal) @@ -197,6 +197,11 @@ func isPointerToStruct(v reflect.Value) bool { return !v.IsZero() && v.Type().Kind() == reflect.Ptr && v.Elem().Type().Kind() == reflect.Struct } +//go:noinline +func isAnonymousField(t reflect.Type, nr int) bool { + return t.Field(nr).Anonymous +} + // checkRequired checks whether required fields are empty // // check type t recursively if t has struct fields. @@ -562,6 +567,11 @@ var ( errUnsupportedDefaultFieldType = errors.New("default option is supported only on: bool, float variants, string, unit variants types or their corresponding pointers or slices") ) +//go:noinline +func errIncompatibleValue(val string, f *fieldInfo) error { + return fmt.Errorf("failed setting default: %s is not compatible with field %s type", val, f.name) +} + // ConversionError stores information about a failed conversion. type ConversionError struct { Key string // key from the source map.