]> Cypherpunks.ru repositories - gostls13.git/commitdiff
expvar: emit valid JSON strings
authorJoe Tsai <joetsai@digital-static.net>
Tue, 14 Mar 2023 22:35:36 +0000 (15:35 -0700)
committerJoseph Tsai <joetsai@digital-static.net>
Sat, 19 Aug 2023 23:26:49 +0000 (23:26 +0000)
Map.String and expvarHandler used the %q flag with fmt.Fprintf
to escape Go strings, which does so according to the Go grammar,
which is not always compatible with JSON strings.

Rather than calling json.Marshal for every string,
which will always allocate, declare a local appendJSONQuote
function that does basic string escaping.
Also, we declare an unexported appendJSON method on every
concrete Var type so that the final JSON output can be
constructed with far fewer allocations.

The resulting logic is both more correct and also much faster.
This does not alter the whitespace style of Map.String or expvarHandler,
but may alter the representation of JSON strings.

Performance:

name         old time/op    new time/op    delta
MapString    5.10µs ± 1%    1.56µs ± 1%  -69.33%  (p=0.000 n=10+9)

name         old alloc/op   new alloc/op   delta
MapString    1.21kB ± 0%    0.66kB ± 0%  -45.12%  (p=0.000 n=10+10)

name         old allocs/op  new allocs/op  delta
MapString      37.0 ± 0%       7.0 ± 0%  -81.08%  (p=0.000 n=10+10)

Fixes #59040

Change-Id: I46a2125f43550b91d52019e5edc003d9dd19590f
Reviewed-on: https://go-review.googlesource.com/c/go/+/476336
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/expvar/expvar.go
src/expvar/expvar_test.go

index 300d8c267643c245d33f66f18d2efb5e7553ed90..41ec437af06b5f385a529d7a5cf1ab82fe2ae78d 100644 (file)
@@ -23,7 +23,6 @@ package expvar
 
 import (
        "encoding/json"
-       "fmt"
        "log"
        "math"
        "net/http"
@@ -31,9 +30,9 @@ import (
        "runtime"
        "sort"
        "strconv"
-       "strings"
        "sync"
        "sync/atomic"
+       "unicode/utf8"
 )
 
 // Var is an abstract type for all exported variables.
@@ -44,6 +43,11 @@ type Var interface {
        String() string
 }
 
+type jsonVar interface {
+       // appendJSON appends the JSON representation of the receiver to b.
+       appendJSON(b []byte) []byte
+}
+
 // Int is a 64-bit integer variable that satisfies the Var interface.
 type Int struct {
        i int64
@@ -54,7 +58,11 @@ func (v *Int) Value() int64 {
 }
 
 func (v *Int) String() string {
-       return strconv.FormatInt(atomic.LoadInt64(&v.i), 10)
+       return string(v.appendJSON(nil))
+}
+
+func (v *Int) appendJSON(b []byte) []byte {
+       return strconv.AppendInt(b, atomic.LoadInt64(&v.i), 10)
 }
 
 func (v *Int) Add(delta int64) {
@@ -75,8 +83,11 @@ func (v *Float) Value() float64 {
 }
 
 func (v *Float) String() string {
-       return strconv.FormatFloat(
-               math.Float64frombits(v.f.Load()), 'g', -1, 64)
+       return string(v.appendJSON(nil))
+}
+
+func (v *Float) appendJSON(b []byte) []byte {
+       return strconv.AppendFloat(b, math.Float64frombits(v.f.Load()), 'g', -1, 64)
 }
 
 // Add adds delta to v.
@@ -111,23 +122,44 @@ type KeyValue struct {
 }
 
 func (v *Map) String() string {
-       var b strings.Builder
-       fmt.Fprintf(&b, "{")
+       return string(v.appendJSON(nil))
+}
+
+func (v *Map) appendJSON(b []byte) []byte {
+       return v.appendJSONMayExpand(b, false)
+}
+
+func (v *Map) appendJSONMayExpand(b []byte, expand bool) []byte {
+       afterCommaDelim := byte(' ')
+       mayAppendNewline := func(b []byte) []byte { return b }
+       if expand {
+               afterCommaDelim = '\n'
+               mayAppendNewline = func(b []byte) []byte { return append(b, '\n') }
+       }
+
+       b = append(b, '{')
+       b = mayAppendNewline(b)
        first := true
        v.Do(func(kv KeyValue) {
                if !first {
-                       fmt.Fprintf(&b, ", ")
-               }
-               fmt.Fprintf(&b, "%q: ", kv.Key)
-               if kv.Value != nil {
-                       fmt.Fprintf(&b, "%v", kv.Value)
-               } else {
-                       fmt.Fprint(&b, "null")
+                       b = append(b, ',', afterCommaDelim)
                }
                first = false
+               b = appendJSONQuote(b, kv.Key)
+               b = append(b, ':', ' ')
+               switch v := kv.Value.(type) {
+               case nil:
+                       b = append(b, "null"...)
+               case jsonVar:
+                       b = v.appendJSON(b)
+               default:
+                       b = append(b, v.String()...)
+               }
        })
-       fmt.Fprintf(&b, "}")
-       return b.String()
+       b = mayAppendNewline(b)
+       b = append(b, '}')
+       b = mayAppendNewline(b)
+       return b
 }
 
 // Init removes all keys from the map.
@@ -247,9 +279,11 @@ func (v *String) Value() string {
 // String implements the Var interface. To get the unquoted string
 // use Value.
 func (v *String) String() string {
-       s := v.Value()
-       b, _ := json.Marshal(s)
-       return string(b)
+       return string(v.appendJSON(nil))
+}
+
+func (v *String) appendJSON(b []byte) []byte {
+       return appendJSONQuote(b, v.Value())
 }
 
 func (v *String) Set(value string) {
@@ -270,31 +304,25 @@ func (f Func) String() string {
 }
 
 // All published variables.
-var (
-       vars      sync.Map // map[string]Var
-       varKeysMu sync.RWMutex
-       varKeys   []string // sorted
-)
+var vars Map
 
 // Publish declares a named exported variable. This should be called from a
 // package's init function when it creates its Vars. If the name is already
 // registered then this will log.Panic.
 func Publish(name string, v Var) {
-       if _, dup := vars.LoadOrStore(name, v); dup {
+       if _, dup := vars.m.LoadOrStore(name, v); dup {
                log.Panicln("Reuse of exported var name:", name)
        }
-       varKeysMu.Lock()
-       defer varKeysMu.Unlock()
-       varKeys = append(varKeys, name)
-       sort.Strings(varKeys)
+       vars.keysMu.Lock()
+       defer vars.keysMu.Unlock()
+       vars.keys = append(vars.keys, name)
+       sort.Strings(vars.keys)
 }
 
 // Get retrieves a named exported variable. It returns nil if the name has
 // not been registered.
 func Get(name string) Var {
-       i, _ := vars.Load(name)
-       v, _ := i.(Var)
-       return v
+       return vars.Get(name)
 }
 
 // Convenience functions for creating new exported variables.
@@ -327,26 +355,12 @@ func NewString(name string) *String {
 // The global variable map is locked during the iteration,
 // but existing entries may be concurrently updated.
 func Do(f func(KeyValue)) {
-       varKeysMu.RLock()
-       defer varKeysMu.RUnlock()
-       for _, k := range varKeys {
-               val, _ := vars.Load(k)
-               f(KeyValue{k, val.(Var)})
-       }
+       vars.Do(f)
 }
 
 func expvarHandler(w http.ResponseWriter, r *http.Request) {
        w.Header().Set("Content-Type", "application/json; charset=utf-8")
-       fmt.Fprintf(w, "{\n")
-       first := true
-       Do(func(kv KeyValue) {
-               if !first {
-                       fmt.Fprintf(w, ",\n")
-               }
-               first = false
-               fmt.Fprintf(w, "%q: %s", kv.Key, kv.Value)
-       })
-       fmt.Fprintf(w, "\n}\n")
+       w.Write(vars.appendJSONMayExpand(nil, true))
 }
 
 // Handler returns the expvar HTTP Handler.
@@ -371,3 +385,32 @@ func init() {
        Publish("cmdline", Func(cmdline))
        Publish("memstats", Func(memstats))
 }
+
+// TODO: Use json.appendString instead.
+func appendJSONQuote(b []byte, s string) []byte {
+       const hex = "0123456789abcdef"
+       b = append(b, '"')
+       for _, r := range s {
+               switch {
+               case r < ' ' || r == '\\' || r == '"' || r == '<' || r == '>' || r == '&' || r == '\u2028' || r == '\u2029':
+                       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:
+                               b = append(b, '\\', 'u', hex[(r>>12)&0xf], hex[(r>>8)&0xf], hex[(r>>4)&0xf], hex[(r>>0)&0xf])
+                       }
+               case r < utf8.RuneSelf:
+                       b = append(b, byte(r))
+               default:
+                       b = utf8.AppendRune(b, r)
+               }
+       }
+       b = append(b, '"')
+       return b
+}
index ee98b5ef19ab0212a0072c37d5c1f62adf3a993f..b827c4d621b93966b7a959bfa70f5307963d3480 100644 (file)
@@ -22,12 +22,12 @@ import (
 // RemoveAll removes all exported variables.
 // This is for tests only.
 func RemoveAll() {
-       varKeysMu.Lock()
-       defer varKeysMu.Unlock()
-       for _, k := range varKeys {
-               vars.Delete(k)
+       vars.keysMu.Lock()
+       defer vars.keysMu.Unlock()
+       for _, k := range vars.keys {
+               vars.m.Delete(k)
        }
-       varKeys = nil
+       vars.keys = nil
 }
 
 func TestNil(t *testing.T) {
@@ -487,6 +487,28 @@ func TestHandler(t *testing.T) {
        }
 }
 
+func BenchmarkMapString(b *testing.B) {
+       var m, m1, m2 Map
+       m.Set("map1", &m1)
+       m1.Add("a", 1)
+       m1.Add("z", 2)
+       m.Set("map2", &m2)
+       for i := 0; i < 9; i++ {
+               m2.Add(strconv.Itoa(i), int64(i))
+       }
+       var s1, s2 String
+       m.Set("str1", &s1)
+       s1.Set("hello, world!")
+       m.Set("str2", &s2)
+       s2.Set("fizz buzz")
+       b.ResetTimer()
+
+       b.ReportAllocs()
+       for i := 0; i < b.N; i++ {
+               _ = m.String()
+       }
+}
+
 func BenchmarkRealworldExpvarUsage(b *testing.B) {
        var (
                bytesSent Int
@@ -622,3 +644,20 @@ func BenchmarkRealworldExpvarUsage(b *testing.B) {
        }
        wg.Wait()
 }
+
+func TestAppendJSONQuote(t *testing.T) {
+       var b []byte
+       for i := 0; i < 128; i++ {
+               b = append(b, byte(i))
+       }
+       b = append(b, "\u2028\u2029"...)
+       got := string(appendJSONQuote(nil, string(b[:])))
+       want := `"` +
+               `\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\t\n\u000b\u000c\r\u000e\u000f` +
+               `\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f` +
+               ` !\"#$%\u0026'()*+,-./0123456789:;\u003c=\u003e?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_` +
+               "`" + `abcdefghijklmnopqrstuvwxyz{|}~` + "\x7f" + `\u2028\u2029"`
+       if got != want {
+               t.Errorf("appendJSONQuote mismatch:\ngot  %v\nwant %v", got, want)
+       }
+}