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.

Comment From: dsnet

Hi, thanks for the proposal, but I don't believe this needs to be a proposal, since we should optimize Reset to reduce allocations, regardless. I'll switch this to a normal bug report.

There are some recent and outstanding optimizations that might actually already fix this: * [CL/685135] encoding/json/v2: avoid escaping jsonopts.Struct * [CL/681177] encoding/json/jsontext: preserve buffer capacity in Decoder.Reset

Comment From: tylerchr

Ok, thanks for the guidance. I wondered.

Great! Admittedly I'm actually using github.com/go-json-experiment/json—are you planning to continue merging changes back to there? If not, once they land here I can re-test with a tip compiler.

Comment From: dsnet

I just merged https://github.com/go-json-experiment/json/pull/178, which should pull in the optimization.

Comment From: tylerchr

It works! Preventing join from escaping its receiver seems to be the change that does it. Thanks for going out of your way to backport that so promptly.

I looked at the CL out of curiosity. For my own education: I see why escaping the receiver in join would heap-allocate opts2 in reset, but I don't know if I quite understand why join escaped its receiver in the first place. Your commit message says it's

because it is passed to JoinUnknownOption, which is a dynamically implemented function

I would think that the compiler can tell that jsonopts.JoinUnknownOption, at least, doesn't escape its argument. Am I understanding correctly that because JoinUnknownOption is an exported dynamic function that can be (and is) overridden from another package at runtime, the compiler can't know when compiling jsonopts that the runtime implementation of JoinUnknownOption doesn't escape it and therefore has to assume that it might? And that assumption then percolates back up through Join into reset where opts2 gets needlessly heap-allocated?

Comment From: dsnet

Yep, you are correct in your assessment.

A smarter compiler might be able to figure out when compiling an entire program that there is practically only one implementation of JoinUnknownOption and analyze that single implementation for escape analysis. However, this analysis is impossible to do when compiling just the encoding/json/internal/jsonopts package in isolation, which has no knowledge of external circumstances.

Comment From: prattmic

It sounds like this is fixed with https://go.dev/cl/685135 and https://go.dev/cl/681177?

I'm going to optimistically close this. Please let me know if I got it wrong.