From d0b6d07a1b4bbf60a54521b9e6b837d52299a1a5 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 1 Oct 2018 14:40:56 +0300 Subject: [PATCH 1/6] [extend #1191] Unnecessary alloc for XML, JSON, JSONP --- context.go | 87 ++++++++++++++++++++++++++++++++++--------------- context_test.go | 8 ++--- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/context.go b/context.go index 9a9d133ed..0754dae2f 100644 --- a/context.go +++ b/context.go @@ -403,24 +403,52 @@ func (c *context) String(code int, s string) (err error) { return c.Blob(code, MIMETextPlainCharsetUTF8, []byte(s)) } -func (c *context) JSON(code int, i interface{}) (err error) { +func (c *context) jsonPBlob(code int, callback string, i interface{}) (err error) { + buf := new(bytes.Buffer) + enc := json.NewEncoder(buf) _, pretty := c.QueryParams()["pretty"] if c.echo.Debug || pretty { - return c.JSONPretty(code, i, " ") + enc.SetIndent("", " ") } - b, err := json.Marshal(i) - if err != nil { + c.writeContentType(MIMEApplicationJavaScriptCharsetUTF8) + c.response.WriteHeader(code) + if _, err = buf.WriteString(callback + "("); err != nil { + + } + if err = enc.Encode(i); err != nil { return } - return c.JSONBlob(code, b) + if _, err = buf.WriteString(");"); err != nil { + return + } + _, err = buf.WriteTo(c.response) + return } -func (c *context) JSONPretty(code int, i interface{}, indent string) (err error) { - b, err := json.MarshalIndent(i, "", indent) - if err != nil { - return +func (c *context) jsonBlob(code int, i interface{}, pretty bool, indent string) error { + enc := json.NewEncoder(c.response) + if pretty { + enc.SetIndent("", indent) } - return c.JSONBlob(code, b) + c.writeContentType(MIMEApplicationJSONCharsetUTF8) + c.response.WriteHeader(code) + return enc.Encode(i) +} + +func (c *context) JSON(code int, i interface{}) (err error) { + var ( + pretty bool + indent string + ) + if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { + pretty = true + indent = " " + } + return c.jsonBlob(code, i, pretty, indent) +} + +func (c *context) JSONPretty(code int, i interface{}, indent string) (err error) { + return c.jsonBlob(code, i, true, indent) } func (c *context) JSONBlob(code int, b []byte) (err error) { @@ -428,11 +456,7 @@ func (c *context) JSONBlob(code int, b []byte) (err error) { } func (c *context) JSONP(code int, callback string, i interface{}) (err error) { - b, err := json.Marshal(i) - if err != nil { - return - } - return c.JSONPBlob(code, callback, b) + return c.jsonPBlob(code, callback, i) } func (c *context) JSONPBlob(code int, callback string, b []byte) (err error) { @@ -448,24 +472,33 @@ func (c *context) JSONPBlob(code int, callback string, b []byte) (err error) { return } -func (c *context) XML(code int, i interface{}) (err error) { - _, pretty := c.QueryParams()["pretty"] - if c.echo.Debug || pretty { - return c.XMLPretty(code, i, " ") +func (c *context) xmlBlob(code int, i interface{}, pretty bool, indent string) (err error) { + c.writeContentType(MIMEApplicationXMLCharsetUTF8) + c.response.WriteHeader(code) + enc := xml.NewEncoder(c.response) + if pretty { + enc.Indent("", " ") } - b, err := xml.Marshal(i) - if err != nil { + if _, err = c.response.Write([]byte(xml.Header)); err != nil { return } - return c.XMLBlob(code, b) + return enc.Encode(i) } -func (c *context) XMLPretty(code int, i interface{}, indent string) (err error) { - b, err := xml.MarshalIndent(i, "", indent) - if err != nil { - return +func (c *context) XML(code int, i interface{}) (err error) { + var ( + pretty bool + indent string + ) + if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { + pretty = true + indent = " " } - return c.XMLBlob(code, b) + return c.xmlBlob(code, i, pretty, indent) +} + +func (c *context) XMLPretty(code int, i interface{}, indent string) (err error) { + return c.xmlBlob(code, i, true, indent) } func (c *context) XMLBlob(code int, b []byte) (err error) { diff --git a/context_test.go b/context_test.go index bcbf9751a..3449e7494 100644 --- a/context_test.go +++ b/context_test.go @@ -67,7 +67,7 @@ func TestContext(t *testing.T) { if assert.NoError(t, err) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(t, userJSON, rec.Body.String()) + assert.Equal(t, userJSON+"\n", rec.Body.String()) } // JSON with "?pretty" @@ -78,7 +78,7 @@ func TestContext(t *testing.T) { if assert.NoError(t, err) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(t, userJSONPretty, rec.Body.String()) + assert.Equal(t, userJSONPretty+"\n", rec.Body.String()) } req = httptest.NewRequest(GET, "/", nil) // reset @@ -89,7 +89,7 @@ func TestContext(t *testing.T) { if assert.NoError(t, err) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(t, userJSONPretty, rec.Body.String()) + assert.Equal(t, userJSONPretty+"\n", rec.Body.String()) } // JSON (error) @@ -106,7 +106,7 @@ func TestContext(t *testing.T) { if assert.NoError(t, err) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, MIMEApplicationJavaScriptCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(t, callback+"("+userJSON+");", rec.Body.String()) + assert.Equal(t, callback+"("+userJSON+"\n);", rec.Body.String()) } // XML From 3dc6c4f1605561ee404258a3b0675036e13e6fad Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 1 Oct 2018 15:00:21 +0300 Subject: [PATCH 2/6] add legacy (JSON/JSONP/XML)Blob tests --- context_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/context_test.go b/context_test.go index 3449e7494..f1ad30ceb 100644 --- a/context_test.go +++ b/context_test.go @@ -2,6 +2,7 @@ package echo import ( "bytes" + "encoding/json" "encoding/xml" "errors" "io" @@ -147,6 +148,43 @@ func TestContext(t *testing.T) { assert.Equal(t, xml.Header+userXMLPretty, rec.Body.String()) } + // Legacy JSONBlob + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + data, err := json.Marshal(user{1, "Jon Snow"}) + assert.NoError(t, err) + err = c.JSONBlob(http.StatusOK, data) + if assert.NoError(t, err) { + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(t, userJSON, rec.Body.String()) + } + + // Legacy JSONP + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + callback = "callback" + data, err = json.Marshal(user{1, "Jon Snow"}) + assert.NoError(t, err) + err = c.JSONPBlob(http.StatusOK, callback, data) + if assert.NoError(t, err) { + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, MIMEApplicationJavaScriptCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(t, callback+"("+userJSON+");", rec.Body.String()) + } + + // Legacy XML + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + data, err = xml.Marshal(user{1, "Jon Snow"}) + assert.NoError(t, err) + err = c.XMLBlob(http.StatusOK, data) + if assert.NoError(t, err) { + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, MIMEApplicationXMLCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(t, xml.Header+userXML, rec.Body.String()) + } + // String rec = httptest.NewRecorder() c = e.NewContext(req, rec).(*context) From bb09993d30b76ea8b13b393f9c50eb02b9d3f19d Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 1 Oct 2018 15:08:57 +0300 Subject: [PATCH 3/6] fix namings --- context_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/context_test.go b/context_test.go index f1ad30ceb..736f5c453 100644 --- a/context_test.go +++ b/context_test.go @@ -160,7 +160,7 @@ func TestContext(t *testing.T) { assert.Equal(t, userJSON, rec.Body.String()) } - // Legacy JSONP + // Legacy JSONPBlob rec = httptest.NewRecorder() c = e.NewContext(req, rec).(*context) callback = "callback" @@ -173,7 +173,7 @@ func TestContext(t *testing.T) { assert.Equal(t, callback+"("+userJSON+");", rec.Body.String()) } - // Legacy XML + // Legacy XMLBlob rec = httptest.NewRecorder() c = e.NewContext(req, rec).(*context) data, err = xml.Marshal(user{1, "Jon Snow"}) From e33262441cf190134bbceb76038dce563c1ca0d0 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 1 Oct 2018 16:28:49 +0300 Subject: [PATCH 4/6] fix `jsonPBlob` allocs --- context.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/context.go b/context.go index 0754dae2f..0c8f19ef4 100644 --- a/context.go +++ b/context.go @@ -404,24 +404,22 @@ func (c *context) String(code int, s string) (err error) { } func (c *context) jsonPBlob(code int, callback string, i interface{}) (err error) { - buf := new(bytes.Buffer) - enc := json.NewEncoder(buf) + enc := json.NewEncoder(c.response) _, pretty := c.QueryParams()["pretty"] if c.echo.Debug || pretty { enc.SetIndent("", " ") } c.writeContentType(MIMEApplicationJavaScriptCharsetUTF8) c.response.WriteHeader(code) - if _, err = buf.WriteString(callback + "("); err != nil { - + if _, err = c.response.Write([]byte(callback + "(")); err != nil { + return } if err = enc.Encode(i); err != nil { return } - if _, err = buf.WriteString(");"); err != nil { + if _, err = c.response.Write([]byte(");")); err != nil { return } - _, err = buf.WriteTo(c.response) return } From 2cb6d3a0d2b8df6599159be713aa9bacc5e79c96 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Wed, 10 Oct 2018 12:40:12 +0300 Subject: [PATCH 5/6] fix review comments (thx @alexaandru) --- context.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/context.go b/context.go index 0c8f19ef4..c1592efa8 100644 --- a/context.go +++ b/context.go @@ -423,9 +423,9 @@ func (c *context) jsonPBlob(code int, callback string, i interface{}) (err error return } -func (c *context) jsonBlob(code int, i interface{}, pretty bool, indent string) error { +func (c *context) jsonBlob(code int, i interface{}, indent string) error { enc := json.NewEncoder(c.response) - if pretty { + if indent != "" { enc.SetIndent("", indent) } c.writeContentType(MIMEApplicationJSONCharsetUTF8) @@ -439,14 +439,13 @@ func (c *context) JSON(code int, i interface{}) (err error) { indent string ) if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { - pretty = true indent = " " } - return c.jsonBlob(code, i, pretty, indent) + return c.jsonBlob(code, i, indent) } func (c *context) JSONPretty(code int, i interface{}, indent string) (err error) { - return c.jsonBlob(code, i, true, indent) + return c.jsonBlob(code, i, indent) } func (c *context) JSONBlob(code int, b []byte) (err error) { @@ -470,11 +469,11 @@ func (c *context) JSONPBlob(code int, callback string, b []byte) (err error) { return } -func (c *context) xmlBlob(code int, i interface{}, pretty bool, indent string) (err error) { +func (c *context) xmlBlob(code int, i interface{}, indent string) (err error) { c.writeContentType(MIMEApplicationXMLCharsetUTF8) c.response.WriteHeader(code) enc := xml.NewEncoder(c.response) - if pretty { + if indent != "" { enc.Indent("", " ") } if _, err = c.response.Write([]byte(xml.Header)); err != nil { @@ -489,14 +488,13 @@ func (c *context) XML(code int, i interface{}) (err error) { indent string ) if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { - pretty = true indent = " " } - return c.xmlBlob(code, i, pretty, indent) + return c.xmlBlob(code, i, indent) } func (c *context) XMLPretty(code int, i interface{}, indent string) (err error) { - return c.xmlBlob(code, i, true, indent) + return c.xmlBlob(code, i, indent) } func (c *context) XMLBlob(code int, b []byte) (err error) { From 521e4e0237b339509b295b62e76244f60c86b1b3 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Wed, 10 Oct 2018 16:04:52 +0300 Subject: [PATCH 6/6] fix review comments (thx @alexaandru) add benchmarks --- context.go | 36 +++++++++----------- context_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 22 deletions(-) diff --git a/context.go b/context.go index c1592efa8..cec6ab394 100644 --- a/context.go +++ b/context.go @@ -206,6 +206,8 @@ const ( indexPage = "index.html" ) +var defaultIndent = " " + func (c *context) writeContentType(value string) { header := c.Response().Header() if header.Get(HeaderContentType) == "" { @@ -423,10 +425,10 @@ func (c *context) jsonPBlob(code int, callback string, i interface{}) (err error return } -func (c *context) jsonBlob(code int, i interface{}, indent string) error { +func (c *context) jsonBlob(code int, i interface{}, indent *string) error { enc := json.NewEncoder(c.response) - if indent != "" { - enc.SetIndent("", indent) + if indent != nil { + enc.SetIndent("", *indent) } c.writeContentType(MIMEApplicationJSONCharsetUTF8) c.response.WriteHeader(code) @@ -434,18 +436,15 @@ func (c *context) jsonBlob(code int, i interface{}, indent string) error { } func (c *context) JSON(code int, i interface{}) (err error) { - var ( - pretty bool - indent string - ) - if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { - indent = " " + var indent *string + if _, pretty := c.QueryParams()["pretty"]; c.echo.Debug || pretty { + indent = &defaultIndent } return c.jsonBlob(code, i, indent) } func (c *context) JSONPretty(code int, i interface{}, indent string) (err error) { - return c.jsonBlob(code, i, indent) + return c.jsonBlob(code, i, &indent) } func (c *context) JSONBlob(code int, b []byte) (err error) { @@ -469,12 +468,12 @@ func (c *context) JSONPBlob(code int, callback string, b []byte) (err error) { return } -func (c *context) xmlBlob(code int, i interface{}, indent string) (err error) { +func (c *context) xmlBlob(code int, i interface{}, indent *string) (err error) { c.writeContentType(MIMEApplicationXMLCharsetUTF8) c.response.WriteHeader(code) enc := xml.NewEncoder(c.response) - if indent != "" { - enc.Indent("", " ") + if indent != nil { + enc.Indent("", *indent) } if _, err = c.response.Write([]byte(xml.Header)); err != nil { return @@ -483,18 +482,15 @@ func (c *context) xmlBlob(code int, i interface{}, indent string) (err error) { } func (c *context) XML(code int, i interface{}) (err error) { - var ( - pretty bool - indent string - ) - if _, pretty = c.QueryParams()["pretty"]; c.echo.Debug || pretty { - indent = " " + var indent *string + if _, pretty := c.QueryParams()["pretty"]; c.echo.Debug || pretty { + indent = &defaultIndent } return c.xmlBlob(code, i, indent) } func (c *context) XMLPretty(code int, i interface{}, indent string) (err error) { - return c.xmlBlob(code, i, indent) + return c.xmlBlob(code, i, &indent) } func (c *context) XMLBlob(code int, b []byte) (err error) { diff --git a/context_test.go b/context_test.go index 736f5c453..c7dd242d4 100644 --- a/context_test.go +++ b/context_test.go @@ -24,6 +24,50 @@ type ( } ) +var testUser = user{1, "Jon Snow"} + +func BenchmarkAllocJSONP(b *testing.B) { + e := New() + req := httptest.NewRequest(POST, "/", strings.NewReader(userJSON)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec).(*context) + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + c.JSONP(http.StatusOK, "callback", testUser) + } +} + +func BenchmarkAllocJSON(b *testing.B) { + e := New() + req := httptest.NewRequest(POST, "/", strings.NewReader(userJSON)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec).(*context) + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + c.JSON(http.StatusOK, testUser) + } +} + +func BenchmarkAllocXML(b *testing.B) { + e := New() + req := httptest.NewRequest(POST, "/", strings.NewReader(userJSON)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec).(*context) + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + c.XML(http.StatusOK, testUser) + } +} + func (t *Template) Render(w io.Writer, name string, data interface{}, c Context) error { return t.templates.ExecuteTemplate(w, name, data) } @@ -43,9 +87,9 @@ func TestContext(t *testing.T) { // Response assert.NotNil(t, c.Response()) - //-------- + // -------- // Render - //-------- + // -------- tmpl := &Template{ templates: template.Must(template.New("hello").Parse("Hello, {{.}}!")), @@ -148,6 +192,48 @@ func TestContext(t *testing.T) { assert.Equal(t, xml.Header+userXMLPretty, rec.Body.String()) } + t.Run("empty indent", func(t *testing.T) { + var ( + u = user{1, "Jon Snow"} + buf = new(bytes.Buffer) + emptyIndent = "" + ) + + t.Run("json", func(t *testing.T) { + buf.Reset() + + // New JSONBlob with empty indent + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + enc := json.NewEncoder(buf) + enc.SetIndent(emptyIndent, emptyIndent) + err = enc.Encode(u) + err = c.jsonBlob(http.StatusOK, user{1, "Jon Snow"}, &emptyIndent) + if assert.NoError(t, err) { + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(t, buf.String(), rec.Body.String()) + } + }) + + t.Run("xml", func(t *testing.T) { + buf.Reset() + + // New XMLBlob with empty indent + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + enc := xml.NewEncoder(buf) + enc.Indent(emptyIndent, emptyIndent) + err = enc.Encode(u) + err = c.xmlBlob(http.StatusOK, user{1, "Jon Snow"}, &emptyIndent) + if assert.NoError(t, err) { + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, MIMEApplicationXMLCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(t, xml.Header+buf.String(), rec.Body.String()) + } + }) + }) + // Legacy JSONBlob rec = httptest.NewRecorder() c = e.NewContext(req, rec).(*context)