Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zap.Any: Reduce stack size with generics #1310

Merged
merged 1 commit into from
Aug 1, 2023
Merged

zap.Any: Reduce stack size with generics #1310

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 105 additions & 64 deletions field.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,43 @@ func Inline(val zapcore.ObjectMarshaler) Field {
}
}

// We discovered an issue where zap.Any can cause a performance degradation
// when used in new goroutines.
//
// This happens because the compiler assigns 4.8kb (one zap.Field per arm of
// switch statement) of stack space for zap.Any when it takes the form:
//
// switch v := v.(type) {
// case string:
// return String(key, v)
// case int:
// return Int(key, v)
// // ...
// default:
// return Reflect(key, v)
// }
//
// To avoid this, we use the type switch to assign a value to a single local variable
// and then call a function on it.
// The local variable is just a function reference so it doesn't allocate
// when converted to an interface{}.
//
// A fair bit of experimentation went into this.
// See also:
//
// - https://github.com/uber-go/zap/pull/1301
// - https://github.com/uber-go/zap/pull/1303
// - https://github.com/uber-go/zap/pull/1304
// - https://github.com/uber-go/zap/pull/1305
// - https://github.com/uber-go/zap/pull/1308
type anyFieldC[T any] func(string, T) Field
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment about why we're doing all of this

ideally we'd have an upstream issue linked which describes the compiler issue

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more details. I don't think there's an upstream compiler issue yet. I believe there are plans to write one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue now exists, adding a link in #1387


func (f anyFieldC[T]) Any(key string, val any) Field {
rabbbit marked this conversation as resolved.
Show resolved Hide resolved
v, _ := val.(T)
// val is guaranteed to be a T, except when it's nil.
return f(key, v)
}

// Any takes a key and an arbitrary value and chooses the best way to represent
// them as a field, falling back to a reflection-based approach only if
// necessary.
Expand All @@ -418,132 +455,136 @@ func Inline(val zapcore.ObjectMarshaler) Field {
// them. To minimize surprises, []byte values are treated as binary blobs, byte
// values are treated as uint8, and runes are always treated as integers.
func Any(key string, value interface{}) Field {
switch val := value.(type) {
var c interface{ Any(string, any) Field }
rabbbit marked this conversation as resolved.
Show resolved Hide resolved

switch value.(type) {
case zapcore.ObjectMarshaler:
return Object(key, val)
c = anyFieldC[zapcore.ObjectMarshaler](Object)
case zapcore.ArrayMarshaler:
return Array(key, val)
c = anyFieldC[zapcore.ArrayMarshaler](Array)
case bool:
return Bool(key, val)
c = anyFieldC[bool](Bool)
case *bool:
return Boolp(key, val)
c = anyFieldC[*bool](Boolp)
case []bool:
return Bools(key, val)
c = anyFieldC[[]bool](Bools)
case complex128:
return Complex128(key, val)
c = anyFieldC[complex128](Complex128)
case *complex128:
return Complex128p(key, val)
c = anyFieldC[*complex128](Complex128p)
case []complex128:
return Complex128s(key, val)
c = anyFieldC[[]complex128](Complex128s)
case complex64:
return Complex64(key, val)
c = anyFieldC[complex64](Complex64)
case *complex64:
return Complex64p(key, val)
c = anyFieldC[*complex64](Complex64p)
case []complex64:
return Complex64s(key, val)
c = anyFieldC[[]complex64](Complex64s)
case float64:
return Float64(key, val)
c = anyFieldC[float64](Float64)
case *float64:
return Float64p(key, val)
c = anyFieldC[*float64](Float64p)
case []float64:
return Float64s(key, val)
c = anyFieldC[[]float64](Float64s)
case float32:
return Float32(key, val)
c = anyFieldC[float32](Float32)
case *float32:
return Float32p(key, val)
c = anyFieldC[*float32](Float32p)
case []float32:
return Float32s(key, val)
c = anyFieldC[[]float32](Float32s)
case int:
return Int(key, val)
c = anyFieldC[int](Int)
case *int:
return Intp(key, val)
c = anyFieldC[*int](Intp)
case []int:
return Ints(key, val)
c = anyFieldC[[]int](Ints)
case int64:
return Int64(key, val)
c = anyFieldC[int64](Int64)
case *int64:
return Int64p(key, val)
c = anyFieldC[*int64](Int64p)
case []int64:
return Int64s(key, val)
c = anyFieldC[[]int64](Int64s)
case int32:
return Int32(key, val)
c = anyFieldC[int32](Int32)
case *int32:
return Int32p(key, val)
c = anyFieldC[*int32](Int32p)
case []int32:
return Int32s(key, val)
c = anyFieldC[[]int32](Int32s)
case int16:
return Int16(key, val)
c = anyFieldC[int16](Int16)
case *int16:
return Int16p(key, val)
c = anyFieldC[*int16](Int16p)
case []int16:
return Int16s(key, val)
c = anyFieldC[[]int16](Int16s)
case int8:
return Int8(key, val)
c = anyFieldC[int8](Int8)
case *int8:
return Int8p(key, val)
c = anyFieldC[*int8](Int8p)
case []int8:
return Int8s(key, val)
c = anyFieldC[[]int8](Int8s)
case string:
return String(key, val)
c = anyFieldC[string](String)
case *string:
return Stringp(key, val)
c = anyFieldC[*string](Stringp)
case []string:
return Strings(key, val)
c = anyFieldC[[]string](Strings)
case uint:
return Uint(key, val)
c = anyFieldC[uint](Uint)
case *uint:
return Uintp(key, val)
c = anyFieldC[*uint](Uintp)
case []uint:
return Uints(key, val)
c = anyFieldC[[]uint](Uints)
case uint64:
return Uint64(key, val)
c = anyFieldC[uint64](Uint64)
case *uint64:
return Uint64p(key, val)
c = anyFieldC[*uint64](Uint64p)
case []uint64:
return Uint64s(key, val)
c = anyFieldC[[]uint64](Uint64s)
case uint32:
return Uint32(key, val)
c = anyFieldC[uint32](Uint32)
case *uint32:
return Uint32p(key, val)
c = anyFieldC[*uint32](Uint32p)
case []uint32:
return Uint32s(key, val)
c = anyFieldC[[]uint32](Uint32s)
case uint16:
return Uint16(key, val)
c = anyFieldC[uint16](Uint16)
case *uint16:
return Uint16p(key, val)
c = anyFieldC[*uint16](Uint16p)
case []uint16:
return Uint16s(key, val)
c = anyFieldC[[]uint16](Uint16s)
case uint8:
return Uint8(key, val)
c = anyFieldC[uint8](Uint8)
case *uint8:
return Uint8p(key, val)
c = anyFieldC[*uint8](Uint8p)
case []byte:
return Binary(key, val)
c = anyFieldC[[]byte](Binary)
case uintptr:
return Uintptr(key, val)
c = anyFieldC[uintptr](Uintptr)
case *uintptr:
return Uintptrp(key, val)
c = anyFieldC[*uintptr](Uintptrp)
case []uintptr:
return Uintptrs(key, val)
c = anyFieldC[[]uintptr](Uintptrs)
case time.Time:
return Time(key, val)
c = anyFieldC[time.Time](Time)
case *time.Time:
return Timep(key, val)
c = anyFieldC[*time.Time](Timep)
case []time.Time:
return Times(key, val)
c = anyFieldC[[]time.Time](Times)
case time.Duration:
return Duration(key, val)
c = anyFieldC[time.Duration](Duration)
case *time.Duration:
return Durationp(key, val)
c = anyFieldC[*time.Duration](Durationp)
case []time.Duration:
return Durations(key, val)
c = anyFieldC[[]time.Duration](Durations)
case error:
return NamedError(key, val)
c = anyFieldC[error](NamedError)
case []error:
return Errors(key, val)
c = anyFieldC[[]error](Errors)
case fmt.Stringer:
return Stringer(key, val)
c = anyFieldC[fmt.Stringer](Stringer)
default:
return Reflect(key, val)
c = anyFieldC[any](Reflect)
}

return c.Any(key, value)
}