code review 5416046: json: speed up encoding, caching reflect calls

src/pkg/encoding/json/encode.go

 // Copyright 2010 The Go Authors.  All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 // Package json implements encoding and decoding of JSON objects as defined in
 // RFC 4627.
 //
 // See "JSON and Go" for an introduction to this package:
 // http://blog.golang.org/2011/01/json-and-go.html
 package json
 
 import (
         "bytes"
         "encoding/base64"
         "reflect"
         "runtime"
         "sort"
         "strconv"
+        "sync"
         "unicode"
         "unicode/utf8"
 )
 
 // Marshal returns the JSON encoding of v.
 //
 // Marshal traverses the value v recursively.
 // If an encountered value implements the Marshaler interface
 // and is not a nil pointer, Marshal calls its MarshalJSON method
 // to produce JSON.  The nil pointer exception is not strictly necessary
@@ skipping 160 matching lines @@
 type interfaceOrPtrValue interface {
         IsNil() bool
         Elem() reflect.Value
 }
 
 var hex = "0123456789abcdef"
 
 // An encodeState encodes JSON into a bytes.Buffer.
 type encodeState struct {
         bytes.Buffer // accumulated output
+
+        // tagCache is a cache of seen StructTag's "json" fields.

[rsc, 2011/11/21 15:06:51]

Unused, I believe.

[bradfitz, 2011/11/21 15:25:54]

Indeed.

+        // the map starts out nil.
+        tagCache map[reflect.StructTag]string
 }
 
 func (e *encodeState) marshal(v interface{}) (err error) {
         defer func() {
                 if r := recover(); r != nil {
                         if _, ok := r.(runtime.Error); ok {
                                 panic(r)
                         }
                         err = r.(error)
                 }
@@ skipping 79 matching lines @@
                         if err != nil {
                                 e.error(err)
                         }
                         e.string(string(sb))
                 } else {
                         e.string(v.String())
                 }
 
         case reflect.Struct:
                 e.WriteByte('{')
-                t := v.Type()
-                n := v.NumField()
                 first := true
-                for i := 0; i < n; i++ {
-                        f := t.Field(i)
-                        if f.PkgPath != "" {
-                                continue
-                        }
-                        tag, omitEmpty, quoted := f.Name, false, false
-                        if tv := f.Tag.Get("json"); tv != "" {
-                                if tv == "-" {
-                                        continue
-                                }
-                                name, opts := parseTag(tv)
-                                if isValidTag(name) {
-                                        tag = name
-                                }
-                                omitEmpty = opts.Contains("omitempty")
-                                quoted = opts.Contains("string")
-                        }
-                        fieldValue := v.Field(i)
-                        if omitEmpty && isEmptyValue(fieldValue) {
+                for _, ef := range encodeFields(v.Type()) {
+                        fieldValue := v.Field(ef.i)
+                        if ef.omitEmpty && isEmptyValue(fieldValue) {
                                 continue
                         }
                         if first {
                                 first = false
                         } else {
                                 e.WriteByte(',')
                         }
-                        e.string(tag)
+                        e.string(ef.tag)
                         e.WriteByte(':')
-                        e.reflectValueQuoted(fieldValue, quoted)
+                        e.reflectValueQuoted(fieldValue, ef.quoted)
                 }
                 e.WriteByte('}')
 
         case reflect.Map:
                 if v.Type().Key().Kind() != reflect.String {
                         e.error(&UnsupportedTypeError{v.Type()})
                 }
                 if v.IsNil() {
                         e.WriteString("null")
                         break
@@ skipping 123 matching lines @@
                         e.error(&InvalidUTF8Error{s})
                 }
                 i += size
         }
         if start < len(s) {
                 e.WriteString(s[start:])
         }
         e.WriteByte('"')
         return e.Len() - len0, nil
 }
+
+// encodeField contains information about how to encode a field of a
+// struct.
+type encodeField struct {
+        i         int // field index in struct
+        tag       string
+        quoted    bool
+        omitEmpty bool
+}
+
+var (
+        typeCacheLock     sync.RWMutex
+        encodeFieldsCache = make(map[reflect.Type][]encodeField)
+)
+
+// encodeFields returns a slice of encodeField for a given
+// struct type.
+func encodeFields(t reflect.Type) []encodeField {
+        typeCacheLock.RLock()
+        defer typeCacheLock.RUnlock()

[rsc, 2011/11/21 15:06:51]

The locking here is suspect.  I think you need:

typeCacheLock.RLock()
fs, ok := encodeFieldsCache[t]
typeCacheLock.RUnlock()
if ok {
    return fs
}

typeCacheLock.Lock()
defer typeCacheLock.Unlock()
fs, ok := encodeFieldsCache[t]
if ok {
    return fs
}

[bradfitz, 2011/11/21 15:25:54]

uh, whoops.

+        fs, ok := encodeFieldsCache[t]
+        if ok {
+                return fs
+        }
+        v := reflect.Zero(t)
+        n := v.NumField()
+        for i := 0; i < n; i++ {
+                f := t.Field(i)
+                if f.PkgPath != "" {
+                        continue
+                }
+                var ef encodeField
+                ef.i = i
+                ef.tag = f.Name
+
+                tv := f.Tag.Get("json")
+                if tv != "" {
+                        if tv == "-" {
+                                continue
+                        }
+                        name, opts := parseTag(tv)
+                        if isValidTag(name) {
+                                ef.tag = name
+                        }
+                        ef.omitEmpty = opts.Contains("omitempty")
+                        ef.quoted = opts.Contains("string")
+                }
+                fs = append(fs, ef)
+        }
+        encodeFieldsCache[t] = fs
+        return fs
+}