Skip to content

Commit

Permalink
Pool the buffer and encoder used for generic JSON reflection (#602)
Browse files Browse the repository at this point in the history
Pool buffers and encoders to make JSON reflection less expensive.
  • Loading branch information
jcorbin authored and akshayjshah committed Jun 27, 2018
1 parent 88c71ae commit f4243df
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 4 deletions.
9 changes: 9 additions & 0 deletions buffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
30 changes: 26 additions & 4 deletions zapcore/json_encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
122 changes: 122 additions & 0 deletions zapcore/json_encoder_test.go
Original file line number Diff line number Diff line change
@@ -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()
})
}
}

0 comments on commit f4243df

Please sign in to comment.