Rebel Go: Forking encoding/json
What happens when I'm left alone for too long
So in my mind the conversation went a little like this.
Phil: I’d like to make encoding/json better. I’d like to save allocations when marshaling by adding a new interface for custom marshalers.
World: Given that this can be explored in 3rd-party packages easily, this seems like a likely decline. Leaving open for a week for final comments.
Phil: Oh, interesting, what would a successful exploration look like?
World: One consequence of writing your own package is that you could use it and would not need the change in the standard library.
Phil: Did you just tell me to go fork myself?
World: I believe I did Bob.
I’m really not sure I want to fork encoding/json. I’m not even sure how I would fork part of a repo, or at least not in a way where I could easily merge upstream fixes. And I’d be shocked if anyone would use it. It would likely be a sad and lonely orphan. Why create something with such a miserable fate?
Why? Well, basically because it’s Saturday, @lizrice has left for a conference and I don’t have anything much else to do.
I’m not going to start with a fork. I’ll just create a branch and show what the changes would involve, and what the improvements would be. As ever, I’ll start with a benchmark.
At Ravelin we pass around a lot of JSON, and that JSON includes fields that are time.Time. We also made a decision many years ago to use https://github.com/guregu/null to express null values. Now, at this point none of those decisions look the best. If we started from scratch I’m not sure I’d use JSON, or ever use time.Time as the primary method of dealing with time, or use https://github.com/guregu/null, but at this point these decisions are very hard to reverse.
So let’s have a benchmark that reflects these decisions. We’ll marshal a struct that has a few of these elements in. In reality at Ravelin our structs have many more fields than this, but a great many of the fields are null.* and time.Time values.
func BenchmarkEncodeMarshaler(b *testing.B) {
b.ReportAllocs()
m := struct {
A null.Int
B time.Time
C time.Time
D null.String
}{
A: null.IntFrom(42),
B: time.Now(),
C: time.Now().Add(-time.Hour),
D: null.StringFrom(`hello`),
}
b.RunParallel(func(pb *testing.PB) {
enc := json.NewEncoder(ioutil.Discard)
for pb.Next() {
if err := enc.Encode(&m); err != nil {
b.Fatal("Encode:", err)
}
}
})
}
If we run this benchmark with Go 1.13 and run the output through benchstat we get the following. Yes, there are 13 allocations needed to marshal this little struct to JSON.
name time/op
EncodeMarshaler-8 753ns ± 2%
name alloc/op
EncodeMarshaler-8 496B ± 0%
name allocs/op
EncodeMarshaler-8 13.0 ± 0%
There are already some improvements scheduled for Go 1.14 that get this down to 5 allocations (some from me! post, post, contribution, contribution). But with a rough version of the proposed change we can get rid of the allocations altogether. Here’s a comparison of the same benchmark at Go tip versus my branch with the changes.
name old time/op new time/op delta
EncodeMarshaler-8 626ns ± 3% 315ns ± 1% -49.65% (p=0.000 n=8+7)
name old alloc/op new alloc/op delta
EncodeMarshaler-8 128B ± 0% 0B -100.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
EncodeMarshaler-8 5.00 ± 0% 0.00 -100.00% (p=0.000 n=8+8)
This is why I think this is a good idea. Running time is halved and all the allocations have gone. The changes are entirely back-compatible and have taken less than 3 hours so far (including writing this much of the blog post).
So what are the changes? Well, we start by adding a new marshaler interface. The idea here is that the MarshalAppendJSON implementation doesn’t need to allocate the []byte it returns that contains the marshaled JSON. It can build its response by appending to the in slice that encoding/json passes to it. With the current MarshalJSON function that’s not true so pretty much every call to a MarshalJSON method results in an allocation.
type MarshalAppender interface {
MarshalAppendJSON(in []byte) ([]byte, error)
}
Once we have the interface we add some code to identify when it is present for a type and assign the appropriate marshaling function. We need to make sure we use the new MarshalAppender interface in preference to Marshaler whenever it is present.
var (
marshalerType = reflect.TypeOf((*Marshaler)(nil)).Elem()
marshalAppenderType = reflect.TypeOf((*MarshalAppender)(nil)).Elem()
textMarshalerType = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem()
)
// newTypeEncoder constructs an encoderFunc for a type.
// The returned encoder only checks CanAddr when allowAddr is true.
func newTypeEncoder(t reflect.Type, allowAddr bool) encoderFunc {
// If we have a non-pointer value whose type implements
// Marshaler with a value receiver, then we're better off taking
// the address of the value - otherwise we end up with an
// allocation as we cast the value to an interface.
if t.Kind() != reflect.Ptr && allowAddr && reflect.PtrTo(t).Implements(marshalAppenderType) {
return newCondAddrEncoder(addrMarshalAppenderEncoder, newTypeEncoder(t, false))
}
if t.Implements(marshalAppenderType) {
return marshalAppenderEncoder
}
if t.Kind() != reflect.Ptr && allowAddr && reflect.PtrTo(t).Implements(marshalerType) {
return newCondAddrEncoder(addrMarshalerEncoder, newTypeEncoder(t, false))
}
The final change to make the new interface work is to add some code to call the new method. This follows the pattern set for the existing Marshaler, except we assume MarshalAppender implementations produce valid, compact JSON. The current Marshaler does not assume this, so it calls the compact function on the output. compact validates and compacts the JSON returned from the Marshaler.
I think the better choice is to trust library writers to create proper implementations. We do not need to check the output millions upon billions of times in live running applications. So my implementation below simply copies the result of MarshalAppendJSON into the encoder’s buffer.
func marshalAppenderEncoder(e *encodeState, v reflect.Value, opts encOpts) {
if v.Kind() == reflect.Ptr && v.IsNil() {
e.WriteString("null")
return
}
m, ok := v.Interface().(MarshalAppender)
if !ok {
e.WriteString("null")
return
}
b, err := m.MarshalAppendJSON(e.scratch[:0])
if err != nil {
e.error(&MarshalerError{v.Type(), err, "MarshalAppendJSON"})
return
}
// We trust implementers of MarshalAppender to generate valid, maximally compact JSON
e.Write(b)
}
func addrMarshalAppenderEncoder(e *encodeState, v reflect.Value, opts encOpts) {
va := v.Addr()
if va.IsNil() {
e.WriteString("null")
return
}
m := va.Interface().(MarshalAppender)
b, err := m.MarshalAppendJSON(e.scratch[:0])
if err != nil {
e.error(&MarshalerError{v.Type(), err, "MarshalAppendJSON"})
return
}
// We trust implementers of MarshalAppender to generate valid, maximally compact JSON
e.Write(b)
}
The next step to make this useful is to implement some custom marshalers. First we’ll do time.Time. To me this one is very important - there must be a very large number of folk with time.Time in structs that get marshaled to JSON. The time.AppendFormat function makes this easy to implement.
// MarshalAppendJSON implements the json.Marshaler interface.
// The time is a quoted string in RFC 3339 format, with sub-second precision added if present.
func (t Time) MarshalAppendJSON(b []byte) ([]byte, error) {
if y := t.Year(); y < 0 || y >= 10000 {
// RFC 3339 is clear that years are 4 digits exactly.
// See golang.org/issue/4556#c15 for more discussion.
return nil, errors.New("Time.MarshalAppendJSON: year outside of range [0,9999]")
}
b = append(b, '"')
b = t.AppendFormat(b, RFC3339Nano)
b = append(b, '"')
return b, nil
}
Finally for my benchmark I need to create MarshalAppendJSON methods for Int and String from the null package. The Int one is easy because of the strconv.AppendInt function.
func (i Int) MarshalAppendJSON(b []byte) ([]byte, error) {
if !i.Valid {
return nullLiteral, nil
}
return strconv.AppendInt(b, i.Int64, 10), nil
}
The String version is much more problematic. I need to JSON encode a string value. But if I just call json.Marshal to do this I don’t get any performance improvements. Really I’d need to add an efficient EncodeString method to encoding/json. But even without that at least the new interface makes it possible to put in the work and improve performance. Here I’ve just followed the definition of a JSON string from json.org and implemented the encoding myself inline. And it might even be partially correct (it definitely isn’t fully correct yet)! And at least it shows that it would be possible to easily implement an efficient EncodeString method.
func (s String) MarshalAppendJSON(b []byte) ([]byte, error) {
if !s.Valid {
return append(b, nullLiteral...), nil
}
b = append(b, '"')
for i, r := range s.String {
switch r {
case '\\', '"':
b = append(b, '\\', byte(r))
case '\n':
b = append(b, '\\', 'n')
case '\r':
b = append(b, '\\', 'r')
case '\t':
b = append(b, '\\', 't')
default:
if r < 32 {
b = append(b, '\\', 'u', '0', '0', hex[r>>4], hex[r&0xF])
} else if r < utf8.RuneSelf {
b = append(b, byte(r))
} else {
// append in its natural form
b = append(b, s.String[i:i+utf8.RuneLen(r)]...)
}
}
}
b = append(b, '"')
return b, nil
}
That was all that was necessary to remove all allocations from this particular JSON marshaling benchmark. Hopefully I’ve explained why this is a useful & positive change to make. I think I’ve also highlighted why it should be done in the standard library. It is only really useful if library writers implement MarshalAppender. And I think they’ll only do that if MarshalAppender is part of the defacto JSON library. Certainly I can’t imagine the time standard library package will be modified to implement a third-party JSON encoding method!
Dear reader, you’ve made it this far, but I have another task to ask of you. I want to know whether I should fork encoding/json to add MarshalAppender, and potentially make other changes that aren’t going to be acceptable to the Go team. I don’t really want to do it just for my own amusement. I only want to do it if people will actually use it. I don’t have a voting system, but if this post becomes “popular” (and my standards for popularity are laughably low) then I might just do it. So if you’d like me to make a fork mash that like/retweet/applaud/up-vote button on whatever thing you discovered this post on.