From 5e08337298b78f832ef87173a3b7203ecb61fc58 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Fri, 10 Sep 2021 14:13:03 -0700 Subject: [PATCH] JSON: Fix complex64 encoding precision (#1011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converting `complex64` to `complex128` meant that the real and imaginary components of the `complex64` value were encoded as `float64` values which, similar to #1002, encoded them with incorrect precision. Fix this by adding a precision parameter during encoding to specify the precision with which the components should be encoded. Guard against other such cases where there's loss of precision by adding tests for several such corner cases for complex, float, int, and uint. This has no significant performance impact on my laptop. (The drop is likely a false positive.) ``` name old time/op new time/op delta ZapJSONFloat32AndComplex64-4 491ns ±22% 442ns ± 6% -10.00% (p=0.000 n=9+9) ``` Fixes #1010 --- CHANGELOG.md | 8 +++-- zapcore/json_encoder.go | 58 +++++++++++++++++------------- zapcore/json_encoder_bench_test.go | 10 ++++++ zapcore/json_encoder_impl_test.go | 20 +++++++++++ 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b077ab08b..6e2b014ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,15 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## Unreleased -* No changes yet. + +Bugfixes: +* [#1011][]: JSON: Fix inaccurate precision when encoding float64. + +[#1011]: https://github.com/uber-go/zap/pull/1011 ## 1.19.1 (8 Sep 2021) -### Fixed +Bugfixes: * [#1001][]: JSON: Fix complex number encoding with negative imaginary part. Thanks to @hemantjadon. * [#1003][]: JSON: Fix inaccurate precision when encoding float32. diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index af220d9b4..7af00ddee 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -118,6 +118,11 @@ func (enc *jsonEncoder) AddComplex128(key string, val complex128) { enc.AppendComplex128(val) } +func (enc *jsonEncoder) AddComplex64(key string, val complex64) { + enc.addKey(key) + enc.AppendComplex64(val) +} + func (enc *jsonEncoder) AddDuration(key string, val time.Duration) { enc.addKey(key) enc.AppendDuration(val) @@ -225,20 +230,23 @@ func (enc *jsonEncoder) AppendByteString(val []byte) { enc.buf.AppendByte('"') } -func (enc *jsonEncoder) AppendComplex128(val complex128) { +// appendComplex appends the encoded form of the provided complex128 value. +// precision specifies the encoding precision for the real and imaginary +// components of the complex number. +func (enc *jsonEncoder) appendComplex(val complex128, precision int) { enc.addElementSeparator() // Cast to a platform-independent, fixed-size type. r, i := float64(real(val)), float64(imag(val)) enc.buf.AppendByte('"') // Because we're always in a quoted string, we can use strconv without // special-casing NaN and +/-Inf. - enc.buf.AppendFloat(r, 64) + enc.buf.AppendFloat(r, precision) // If imaginary part is less than 0, minus (-) sign is added by default // by AppendFloat. if i >= 0 { enc.buf.AppendByte('+') } - enc.buf.AppendFloat(i, 64) + enc.buf.AppendFloat(i, precision) enc.buf.AppendByte('i') enc.buf.AppendByte('"') } @@ -301,28 +309,28 @@ func (enc *jsonEncoder) AppendUint64(val uint64) { enc.buf.AppendUint(val) } -func (enc *jsonEncoder) AddComplex64(k string, v complex64) { enc.AddComplex128(k, complex128(v)) } -func (enc *jsonEncoder) AddInt(k string, v int) { enc.AddInt64(k, int64(v)) } -func (enc *jsonEncoder) AddInt32(k string, v int32) { enc.AddInt64(k, int64(v)) } -func (enc *jsonEncoder) AddInt16(k string, v int16) { enc.AddInt64(k, int64(v)) } -func (enc *jsonEncoder) AddInt8(k string, v int8) { enc.AddInt64(k, int64(v)) } -func (enc *jsonEncoder) AddUint(k string, v uint) { enc.AddUint64(k, uint64(v)) } -func (enc *jsonEncoder) AddUint32(k string, v uint32) { enc.AddUint64(k, uint64(v)) } -func (enc *jsonEncoder) AddUint16(k string, v uint16) { enc.AddUint64(k, uint64(v)) } -func (enc *jsonEncoder) AddUint8(k string, v uint8) { enc.AddUint64(k, uint64(v)) } -func (enc *jsonEncoder) AddUintptr(k string, v uintptr) { enc.AddUint64(k, uint64(v)) } -func (enc *jsonEncoder) AppendComplex64(v complex64) { enc.AppendComplex128(complex128(v)) } -func (enc *jsonEncoder) AppendFloat64(v float64) { enc.appendFloat(v, 64) } -func (enc *jsonEncoder) AppendFloat32(v float32) { enc.appendFloat(float64(v), 32) } -func (enc *jsonEncoder) AppendInt(v int) { enc.AppendInt64(int64(v)) } -func (enc *jsonEncoder) AppendInt32(v int32) { enc.AppendInt64(int64(v)) } -func (enc *jsonEncoder) AppendInt16(v int16) { enc.AppendInt64(int64(v)) } -func (enc *jsonEncoder) AppendInt8(v int8) { enc.AppendInt64(int64(v)) } -func (enc *jsonEncoder) AppendUint(v uint) { enc.AppendUint64(uint64(v)) } -func (enc *jsonEncoder) AppendUint32(v uint32) { enc.AppendUint64(uint64(v)) } -func (enc *jsonEncoder) AppendUint16(v uint16) { enc.AppendUint64(uint64(v)) } -func (enc *jsonEncoder) AppendUint8(v uint8) { enc.AppendUint64(uint64(v)) } -func (enc *jsonEncoder) AppendUintptr(v uintptr) { enc.AppendUint64(uint64(v)) } +func (enc *jsonEncoder) AddInt(k string, v int) { enc.AddInt64(k, int64(v)) } +func (enc *jsonEncoder) AddInt32(k string, v int32) { enc.AddInt64(k, int64(v)) } +func (enc *jsonEncoder) AddInt16(k string, v int16) { enc.AddInt64(k, int64(v)) } +func (enc *jsonEncoder) AddInt8(k string, v int8) { enc.AddInt64(k, int64(v)) } +func (enc *jsonEncoder) AddUint(k string, v uint) { enc.AddUint64(k, uint64(v)) } +func (enc *jsonEncoder) AddUint32(k string, v uint32) { enc.AddUint64(k, uint64(v)) } +func (enc *jsonEncoder) AddUint16(k string, v uint16) { enc.AddUint64(k, uint64(v)) } +func (enc *jsonEncoder) AddUint8(k string, v uint8) { enc.AddUint64(k, uint64(v)) } +func (enc *jsonEncoder) AddUintptr(k string, v uintptr) { enc.AddUint64(k, uint64(v)) } +func (enc *jsonEncoder) AppendComplex64(v complex64) { enc.appendComplex(complex128(v), 32) } +func (enc *jsonEncoder) AppendComplex128(v complex128) { enc.appendComplex(complex128(v), 64) } +func (enc *jsonEncoder) AppendFloat64(v float64) { enc.appendFloat(v, 64) } +func (enc *jsonEncoder) AppendFloat32(v float32) { enc.appendFloat(float64(v), 32) } +func (enc *jsonEncoder) AppendInt(v int) { enc.AppendInt64(int64(v)) } +func (enc *jsonEncoder) AppendInt32(v int32) { enc.AppendInt64(int64(v)) } +func (enc *jsonEncoder) AppendInt16(v int16) { enc.AppendInt64(int64(v)) } +func (enc *jsonEncoder) AppendInt8(v int8) { enc.AppendInt64(int64(v)) } +func (enc *jsonEncoder) AppendUint(v uint) { enc.AppendUint64(uint64(v)) } +func (enc *jsonEncoder) AppendUint32(v uint32) { enc.AppendUint64(uint64(v)) } +func (enc *jsonEncoder) AppendUint16(v uint16) { enc.AppendUint64(uint64(v)) } +func (enc *jsonEncoder) AppendUint8(v uint8) { enc.AppendUint64(uint64(v)) } +func (enc *jsonEncoder) AppendUintptr(v uintptr) { enc.AppendUint64(uint64(v)) } func (enc *jsonEncoder) Clone() Encoder { clone := enc.clone() diff --git a/zapcore/json_encoder_bench_test.go b/zapcore/json_encoder_bench_test.go index 4bd5033b4..bcb5a0133 100644 --- a/zapcore/json_encoder_bench_test.go +++ b/zapcore/json_encoder_bench_test.go @@ -38,6 +38,16 @@ func BenchmarkJSONLogMarshalerFunc(b *testing.B) { } } +func BenchmarkZapJSONFloat32AndComplex64(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + enc := NewJSONEncoder(testEncoderConfig()) + enc.AddFloat32("float32", 3.14) + enc.AddComplex64("complex64", 2.71+3.14i) + } + }) +} + func BenchmarkZapJSON(b *testing.B) { b.RunParallel(func(pb *testing.PB) { for pb.Next() { diff --git a/zapcore/json_encoder_impl_test.go b/zapcore/json_encoder_impl_test.go index 3f11a8cca..8db12b673 100644 --- a/zapcore/json_encoder_impl_test.go +++ b/zapcore/json_encoder_impl_test.go @@ -121,32 +121,52 @@ func TestJSONEncoderObjectFields(t *testing.T) { {"byteString", `"k":""`, func(e Encoder) { e.AddByteString("k", []byte{}) }}, {"byteString", `"k":""`, func(e Encoder) { e.AddByteString("k", nil) }}, {"complex128", `"k":"1+2i"`, func(e Encoder) { e.AddComplex128("k", 1+2i) }}, + {"complex128/negative_i", `"k":"1-2i"`, func(e Encoder) { e.AddComplex128("k", 1-2i) }}, {"complex64", `"k":"1+2i"`, func(e Encoder) { e.AddComplex64("k", 1+2i) }}, + {"complex64/negative_i", `"k":"1-2i"`, func(e Encoder) { e.AddComplex64("k", 1-2i) }}, + {"complex64", `"k":"2.71+3.14i"`, func(e Encoder) { e.AddComplex64("k", 2.71+3.14i) }}, {"duration", `"k":0.000000001`, func(e Encoder) { e.AddDuration("k", 1) }}, + {"duration/negative", `"k":-0.000000001`, func(e Encoder) { e.AddDuration("k", -1) }}, {"float64", `"k":1`, func(e Encoder) { e.AddFloat64("k", 1.0) }}, {"float64", `"k":10000000000`, func(e Encoder) { e.AddFloat64("k", 1e10) }}, {"float64", `"k":"NaN"`, func(e Encoder) { e.AddFloat64("k", math.NaN()) }}, {"float64", `"k":"+Inf"`, func(e Encoder) { e.AddFloat64("k", math.Inf(1)) }}, {"float64", `"k":"-Inf"`, func(e Encoder) { e.AddFloat64("k", math.Inf(-1)) }}, + {"float64/pi", `"k":3.141592653589793`, func(e Encoder) { e.AddFloat64("k", math.Pi) }}, {"float32", `"k":1`, func(e Encoder) { e.AddFloat32("k", 1.0) }}, + {"float32", `"k":2.71`, func(e Encoder) { e.AddFloat32("k", 2.71) }}, + {"float32", `"k":0.1`, func(e Encoder) { e.AddFloat32("k", 0.1) }}, {"float32", `"k":10000000000`, func(e Encoder) { e.AddFloat32("k", 1e10) }}, {"float32", `"k":"NaN"`, func(e Encoder) { e.AddFloat32("k", float32(math.NaN())) }}, {"float32", `"k":"+Inf"`, func(e Encoder) { e.AddFloat32("k", float32(math.Inf(1))) }}, {"float32", `"k":"-Inf"`, func(e Encoder) { e.AddFloat32("k", float32(math.Inf(-1))) }}, + {"float32/pi", `"k":3.1415927`, func(e Encoder) { e.AddFloat32("k", math.Pi) }}, {"int", `"k":42`, func(e Encoder) { e.AddInt("k", 42) }}, {"int64", `"k":42`, func(e Encoder) { e.AddInt64("k", 42) }}, + {"int64/min", `"k":-9223372036854775808`, func(e Encoder) { e.AddInt64("k", math.MinInt64) }}, + {"int64/max", `"k":9223372036854775807`, func(e Encoder) { e.AddInt64("k", math.MaxInt64) }}, {"int32", `"k":42`, func(e Encoder) { e.AddInt32("k", 42) }}, + {"int32/min", `"k":-2147483648`, func(e Encoder) { e.AddInt32("k", math.MinInt32) }}, + {"int32/max", `"k":2147483647`, func(e Encoder) { e.AddInt32("k", math.MaxInt32) }}, {"int16", `"k":42`, func(e Encoder) { e.AddInt16("k", 42) }}, + {"int16/min", `"k":-32768`, func(e Encoder) { e.AddInt16("k", math.MinInt16) }}, + {"int16/max", `"k":32767`, func(e Encoder) { e.AddInt16("k", math.MaxInt16) }}, {"int8", `"k":42`, func(e Encoder) { e.AddInt8("k", 42) }}, + {"int8/min", `"k":-128`, func(e Encoder) { e.AddInt8("k", math.MinInt8) }}, + {"int8/max", `"k":127`, func(e Encoder) { e.AddInt8("k", math.MaxInt8) }}, {"string", `"k":"v\\"`, func(e Encoder) { e.AddString(`k`, `v\`) }}, {"string", `"k":"v"`, func(e Encoder) { e.AddString("k", "v") }}, {"string", `"k":""`, func(e Encoder) { e.AddString("k", "") }}, {"time", `"k":1`, func(e Encoder) { e.AddTime("k", time.Unix(1, 0)) }}, {"uint", `"k":42`, func(e Encoder) { e.AddUint("k", 42) }}, {"uint64", `"k":42`, func(e Encoder) { e.AddUint64("k", 42) }}, + {"uint64/max", `"k":18446744073709551615`, func(e Encoder) { e.AddUint64("k", math.MaxUint64) }}, {"uint32", `"k":42`, func(e Encoder) { e.AddUint32("k", 42) }}, + {"uint32/max", `"k":4294967295`, func(e Encoder) { e.AddUint32("k", math.MaxUint32) }}, {"uint16", `"k":42`, func(e Encoder) { e.AddUint16("k", 42) }}, + {"uint16/max", `"k":65535`, func(e Encoder) { e.AddUint16("k", math.MaxUint16) }}, {"uint8", `"k":42`, func(e Encoder) { e.AddUint8("k", 42) }}, + {"uint8/max", `"k":255`, func(e Encoder) { e.AddUint8("k", math.MaxUint8) }}, {"uintptr", `"k":42`, func(e Encoder) { e.AddUintptr("k", 42) }}, { desc: "object (success)",