Skip to content

encoding/json/v2: enable allocation-free Encoder and Decoder reuse #74538

Closed
@tylerchr

Description

@tylerchr

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.Performance

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions