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: 9b0ac1d4c5318b6bf9ed7930320f2bd755f9939c
GitHub-Pull-Request: golang/go#34127
Reviewed-on: https://go-review.googlesource.com/c/go/+/193604
Reviewed-by: Daniel Martí <[email protected]>
Run-TryBot: Daniel Martí <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
breml authored and mvdan committed Sep 11, 2019
1 parent 56765a6 commit c3bfbe7
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
16 changes: 16 additions & 0 deletions 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 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 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 c3bfbe7

Please sign in to comment.