In order to fix bugs like #22967, #27722, or #33993, where the MarshalJSON method was not called on a pointer receiver, the v2 implementation consistently always ensures that a reflect.Value is addressable at the entry point (shallow copying a value on the heap if necessary). This makes the logic easier to reason about, since we can assume that a value is always addressable.
However, this leads to a performance regression relative to v1 when marshaling a non-pointer values, which will always allocate.
What did you do?
var src bool
b.ReportAllocs()
for range b.N {
jsonv1.Marshal(src)
}
where src is a bool (rather than a *bool).
What did you see happen?
BenchmarkV1-32 23085357 52.64 ns/op 8 B/op 1 allocs/op
BenchmarkV1in2-32 13171356 81.62 ns/op 16 B/op 2 allocs/op
What did you expect to see?
BenchmarkV1-32 23085357 52.64 ns/op 8 B/op 1 allocs/op
BenchmarkV1in2-32 23085357 52.64 ns/op 8 B/op 1 allocs/op
I expect to see a single allocation for v1in2.
We could fix this by deferring the shallow copy until we actually need to call a method on a pointer receiver. This is functionally what v1 did, but the code was harder to reason about whether we will accidentally have a panic due to trying to call Addr on a non-addressable value.
\cc @neild @mvdan @johanbrandhorst
Comment From: gabyhelp
Related Issues
- testing: Problems with memory allocation calculation for "json.Marshal" in Benchmarkc tests #68381 (closed)
- encoding/json: Marshal silently ignores custom implementations on pass by value #30351 (closed)
- encoding/json: marshaling RawMessage has poor performance #33422
- encoding/json: bad encoding of field with MarshalJSON method #22967
- encoding/json: fix and optimize marshal for quoted string #34127 (closed)
Related Code Changes
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)