From f4243df2af592a2e0ccdccb77e1ad44f78d735e5 Mon Sep 17 00:00:00 2001 From: Joshua T Corbin Date: Tue, 26 Jun 2018 20:28:19 -0700 Subject: [PATCH] Pool the buffer and encoder used for generic JSON reflection (#602) Pool buffers and encoders to make JSON reflection less expensive. --- buffer/buffer.go | 9 +++ zapcore/json_encoder.go | 30 +++++++-- zapcore/json_encoder_test.go | 122 +++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 zapcore/json_encoder_test.go diff --git a/buffer/buffer.go b/buffer/buffer.go index d15f7fdb3..7592e8c63 100644 --- a/buffer/buffer.go +++ b/buffer/buffer.go @@ -98,6 +98,15 @@ func (b *Buffer) Write(bs []byte) (int, error) { return len(bs), nil } +// TrimNewline trims any final "\n" byte from the end of the buffer. +func (b *Buffer) TrimNewline() { + if i := len(b.bs) - 1; i >= 0 { + if b.bs[i] == '\n' { + b.bs = b.bs[:i] + } + } +} + // Free returns the Buffer to its Pool. // // Callers must not retain references to the Buffer after calling Free. diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index 1006ba2b1..2dc67d81e 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -44,10 +44,15 @@ func getJSONEncoder() *jsonEncoder { } func putJSONEncoder(enc *jsonEncoder) { + if enc.reflectBuf != nil { + enc.reflectBuf.Free() + } enc.EncoderConfig = nil enc.buf = nil enc.spaced = false enc.openNamespaces = 0 + enc.reflectBuf = nil + enc.reflectEnc = nil _jsonPool.Put(enc) } @@ -56,6 +61,10 @@ type jsonEncoder struct { buf *buffer.Buffer spaced bool // include spaces after colons and commas openNamespaces int + + // for encoding generic values by reflection + reflectBuf *buffer.Buffer + reflectEnc *json.Encoder } // NewJSONEncoder creates a fast, low-allocation JSON encoder. The encoder @@ -124,13 +133,24 @@ func (enc *jsonEncoder) AddInt64(key string, val int64) { enc.AppendInt64(val) } +func (enc *jsonEncoder) resetReflectBuf() { + if enc.reflectBuf == nil { + enc.reflectBuf = bufferpool.Get() + enc.reflectEnc = json.NewEncoder(enc.reflectBuf) + } else { + enc.reflectBuf.Reset() + } +} + func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error { - marshaled, err := json.Marshal(obj) + enc.resetReflectBuf() + err := enc.reflectEnc.Encode(obj) if err != nil { return err } + enc.reflectBuf.TrimNewline() enc.addKey(key) - _, err = enc.buf.Write(marshaled) + _, err = enc.buf.Write(enc.reflectBuf.Bytes()) return err } @@ -213,12 +233,14 @@ func (enc *jsonEncoder) AppendInt64(val int64) { } func (enc *jsonEncoder) AppendReflected(val interface{}) error { - marshaled, err := json.Marshal(val) + enc.resetReflectBuf() + err := enc.reflectEnc.Encode(val) if err != nil { return err } + enc.reflectBuf.TrimNewline() enc.addElementSeparator() - _, err = enc.buf.Write(marshaled) + _, err = enc.buf.Write(enc.reflectBuf.Bytes()) return err } diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go new file mode 100644 index 000000000..31e0852df --- /dev/null +++ b/zapcore/json_encoder_test.go @@ -0,0 +1,122 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zapcore_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// TestJSONEncodeEntry is an more "integrated" test that makes it easier to get +// coverage on the json encoder (e.g. putJSONEncoder, let alone EncodeEntry +// itself) than the tests in json_encoder_impl_test.go; it needs to be in the +// zapcore_test package, so that it can import the toplevel zap package for +// field constructor convenience. +func TestJSONEncodeEntry(t *testing.T) { + type bar struct { + Key string `json:"key"` + Val float64 `json:"val"` + } + + type foo struct { + A string `json:"aee"` + B int `json:"bee"` + C float64 `json:"cee"` + D []bar `json:"dee"` + } + + tests := []struct { + desc string + expected string + ent zapcore.Entry + fields []zapcore.Field + }{ + { + desc: "info entry with some fields", + expected: `{ + "L": "info", + "T": "2018-06-19T16:33:42.000Z", + "N": "bob", + "M": "lob law", + "so": "passes", + "answer": 42, + "common_pie": 3.14, + "such": { + "aee": "lol", + "bee": 123, + "cee": 0.9999, + "dee": [ + {"key": "pi", "val": 3.141592653589793}, + {"key": "tau", "val": 6.283185307179586} + ] + } + }`, + ent: zapcore.Entry{ + Level: zapcore.InfoLevel, + Time: time.Date(2018, 6, 19, 16, 33, 42, 99, time.UTC), + LoggerName: "bob", + Message: "lob law", + }, + fields: []zapcore.Field{ + zap.String("so", "passes"), + zap.Int("answer", 42), + zap.Float64("common_pie", 3.14), + zap.Reflect("such", foo{ + A: "lol", + B: 123, + C: 0.9999, + D: []bar{ + {"pi", 3.141592653589793}, + {"tau", 6.283185307179586}, + }, + }), + }, + }, + } + + enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{ + MessageKey: "M", + LevelKey: "L", + TimeKey: "T", + NameKey: "N", + CallerKey: "C", + StacktraceKey: "S", + EncodeLevel: zapcore.LowercaseLevelEncoder, + EncodeTime: zapcore.ISO8601TimeEncoder, + EncodeDuration: zapcore.SecondsDurationEncoder, + EncodeCaller: zapcore.ShortCallerEncoder, + }) + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + buf, err := enc.EncodeEntry(tt.ent, tt.fields) + if assert.NoError(t, err, "Unexpected JSON encoding error.") { + assert.JSONEq(t, tt.expected, buf.String(), "Incorrect encoded JSON entry.") + } + buf.Free() + }) + } +}