Skip to content

Commit

Permalink
JSON: Fix complex64 encoding precision (#1011)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@abhinav
abhinav authored Sep 10, 2021
1 parent a0e2380 commit 5e08337
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 27 deletions.
8 changes: 6 additions & 2 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
58 changes: 33 additions & 25 deletions zapcore/json_encoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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('"')
}
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 10 additions & 0 deletions zapcore/json_encoder_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
20 changes: 20 additions & 0 deletions zapcore/json_encoder_impl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down

0 comments on commit 5e08337

Please sign in to comment.