Skip to content

Commit

Permalink
encoding/json: fix and optimize marshal for quoted string
Browse files Browse the repository at this point in the history
Since Go 1.2 every string can be marshaled to JSON without error even if it
contains invalid UTF-8 byte sequences. Therefore there is no need to use
Marshal again for the only reason of enclosing the string in double quotes.
Not using Marshal here also removes the error check as there has not been a
way for Marshal to fail anyway.

name          old time/op    new time/op    delta
Issue34127-4     360ns ± 3%     200ns ± 3%  -44.56%  (p=0.008 n=5+5)

name          old alloc/op   new alloc/op   delta
Issue34127-4     56.0B ± 0%     40.0B ± 0%  -28.57%  (p=0.008 n=5+5)

name          old allocs/op  new allocs/op  delta
Issue34127-4      3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)

Fixes #34154

Change-Id: Ib60dc11980f9b20d8bef2982de7168943d632263
GitHub-Last-Rev: 9b0ac1d
GitHub-Pull-Request: #34127
Reviewed-on: https://go-review.googlesource.com/c/go/+/193604
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
breml authored and mvdan committed Sep 11, 2019
1 parent 11c2411 commit 0e015e2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
16 changes: 16 additions & 0 deletions src/encoding/json/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,22 @@ func BenchmarkIssue10335(b *testing.B) {
})
}

func BenchmarkIssue34127(b *testing.B) {
b.ReportAllocs()
j := struct {
Bar string `json:"bar,string"`
}{
Bar: `foobar`,
}
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
if _, err := Marshal(&j); err != nil {
b.Fatal(err)
}
}
})
}

func BenchmarkUnmapped(b *testing.B) {
b.ReportAllocs()
j := []byte(`{"s": "hello", "y": 2, "o": {"x": 0}, "a": [1, 99, {"x": 1}]}`)
Expand Down
10 changes: 5 additions & 5 deletions src/encoding/json/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,11 +600,11 @@ func stringEncoder(e *encodeState, v reflect.Value, opts encOpts) {
return
}
if opts.quoted {
sb, err := Marshal(v.String())
if err != nil {
e.error(err)
}
e.string(string(sb), opts.escapeHTML)
b := make([]byte, 0, v.Len()+2)
b = append(b, '"')
b = append(b, []byte(v.String())...)
b = append(b, '"')
e.stringBytes(b, opts.escapeHTML)
} else {
e.string(v.String(), opts.escapeHTML)
}
Expand Down
10 changes: 10 additions & 0 deletions src/encoding/json/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ func TestEncoderSetEscapeHTML(t *testing.T) {
Ptr strPtrMarshaler
}{`"<str>"`, `"<str>"`}

// https://golang.org/issue/34154
stringOption := struct {
Bar string `json:"bar,string"`
}{`<html>foobar</html>`}

for _, tt := range []struct {
name string
v interface{}
Expand All @@ -137,6 +142,11 @@ func TestEncoderSetEscapeHTML(t *testing.T) {
`{"NonPtr":"\u003cstr\u003e","Ptr":"\u003cstr\u003e"}`,
`{"NonPtr":"<str>","Ptr":"<str>"}`,
},
{
"stringOption", stringOption,
`{"bar":"\"\u003chtml\u003efoobar\u003c/html\u003e\""}`,
`{"bar":"\"<html>foobar</html>\""}`,
},
} {
var buf bytes.Buffer
enc := NewEncoder(&buf)
Expand Down

0 comments on commit 0e015e2

Please sign in to comment.