JSON Encoder Buffer Reuse

[Dave Grijalva]

In skimming through the JSON updates in 1.2, I noticed something curious WRT buffer reuse:

encode.go creates a leaky bucket pool (encodeStatePool).  It's used by json.Encoder but ignored by json.Marshal.  Is this intentional or an oversight?

-dave

[minux]

On Mon, Dec 2, 2013 at 3:28 PM, Dave Grijalva <grij...@gmail.com> wrote:

In skimming through the JSON updates in 1.2, I noticed something curious WRT buffer reuse:

encode.go creates a leaky bucket pool (encodeStatePool).  It's used by json.Encoder but ignored by json.Marshal.  Is this intentional or an oversight?

IMHO there is little benefit in reusing encodeState{} for json.Marshal because Marshal will

allocate the byte slice every time anyway, so little point in saving memory for encodeState.

in other words, if you care about allocation, you should definitely use the streaming interface.

[Kevin Gillette]

On Monday, December 2, 2013 1:40:50 PM UTC-7, minux wrote:

IMHO there is little benefit in reusing encodeState{} for json.Marshal because Marshal will

allocate the byte slice every time anyway, so little point in saving memory for encodeState.

in other words, if you care about allocation, you should definitely use the streaming interface.

Unfortunately the streaming interface (json.NewEncoder) dispatches to encodeState as well, meaning that the encoded data is fully buffered before being sent to the specified writer; e.g. if you have 10gb worth of data that you can guarantee is json-encodable, it'll still allocate an additional 10gb of memory just for buffering. At the moment, there's no benefit to using the streaming interface, and there seems to be little or no interest in making json.NewEncoder act as anything more than a convenience for json.Marshal -> bytes.NewReader -> io.Copy.

[Kevin Gillette]

On Monday, December 2, 2013 4:22:29 PM UTC-7, Kevin Gillette wrote:

At the moment, there's no benefit to using the streaming interface

Correction: no meaningful performance or overhead-reduction benefit. 

[minux]

Of course there are people interested in getting Encoder allocate less (e.g., https://codereview.appspot.com/9365044)

IMO, on the contrary, Marshal is for convenience.

Also, future optimization could certainly reduce the memory allocated by reduced buffering exploiting

the nature of the streaming interface. The same kind of optimization couldn't apply the Marshal case.

With this in mind, it's still recommended to use the Encoder/Decoder interface rather than using

Marshal/Unmarshal directly if you're dealing with Writer/Reader.

[Dave Grijalva]

I'm actually more interested in the amount of garbage produced.  Encoder will return its buffers to a shared pool and they will be re-used (thus reducing garbage).  Marshal always creates and discards its context.  Upon re-reading the code, it occurred to me this lack of reuse is because Marshal returns the bytes underlying the buffer.  It would be unsafe to reuse that buffer.

-dave

[Kevin Gillette]

On Monday, December 2, 2013 4:45:39 PM UTC-7, minux wrote:

Also, future optimization could certainly reduce the memory allocated by reduced buffering exploiting
the nature of the streaming interface. The same kind of optimization couldn't apply the Marshal case.

That's what I was describing, though I don't consider it an optimization, but rather, patching an existing bug. 

 

Of course there are people interested in getting Encoder allocate less (e.g., https://codereview.appspot.com/9365044)

That removed a redundant allocation, but didn't attempt to deal with the "why are we buffering Encode at all?" problem.