Description
Proposal Details
The jsontext.Encoder
and jsontext.Decoder
APIs both include a Reset(..., opts ...Options)
method for reconfiguring it "such that it is [reading/writing] afresh from [r/w] and configured with the provided options." At present these are both implemented in such a way that calling them forces a rather large allocation. I'd like to propose an optimization to improve the reuse potential of both types by avoiding that allocation when possible.
My use case involves pooling short-lived Encoder
and Decoder
values with sync.Pool
and reusing them via their Reset
method. Two important notes specific to my usage:
pprof
showed that that the forced allocations account for about 50% of total allocations (alloc_space
) in my use case.- I never actually need to change the options—I configure them once up front and then call
enc.Reset(newW, enc.Options())
on each reuse.
I think it's possible and worthwhile for both Encoder.Reset
and Decoder.Reset
to be allocation-free in this scenario.
cc @dsnet
Possible solution
The source of the allocation is the handling the provided opts
. Specifically, each call to Reset
currently allocates a new 120-byte jsonopts.Struct
struct and merges the new opts
into it.
Of the approaches I considered, the simplest is to recognize when the given opts
are exactly identical to the existing jsonopts.Struct
and forego the needless reconstruction:
diff --git jsontext/encode.go jsontext/encode.go
index 997820f..d123354 100644
--- jsontext/encode.go
+++ jsontext/encode.go
@@ -114,9 +114,11 @@ func (e *encoderState) reset(b []byte, w io.Writer, opts ...Options) {
if bb, ok := w.(*bytes.Buffer); ok && bb != nil {
e.Buf = bb.Bytes()[bb.Len():] // alias the unused buffer of bb
}
+ if unchanged := len(opts) == 1 && opts[0] == &e.Struct; !unchanged {
opts2 := jsonopts.Struct{} // avoid mutating e.Struct in case it is part of opts
opts2.Join(opts...)
e.Struct = opts2
+ }
if e.Flags.Get(jsonflags.Multiline) {
if !e.Flags.Has(jsonflags.SpaceAfterColon) {
e.Flags.Set(jsonflags.SpaceAfterColon | 1)
This seems safe to me but I'm interested in the assessment of someone familiar with the inner workings of jsontext
and jsonopts
.
I verified via pprof
that this change indeed eliminates any allocations if the caller uses the enc.Reset(newW, enc.Options())
pattern. If accepted, it may additionally be worth considering documenting Reset
to be allocation-free if used in this way.