From 62145fad3e204312f818d9772d1405372ff21e9c Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 14 Jan 2019 21:12:22 +0300 Subject: [PATCH] [extend #1191] Unnecessary alloc for XML, JSON, JSONP (#1199) * [extend #1191] Unnecessary alloc for XML, JSON, JSONP * add legacy (JSON/JSONP/XML)Blob tests * fix namings * fix `jsonPBlob` allocs * fix review comments (thx @alexaandru) * fix review comments (thx @alexaandru) add benchmarks --- context.go | 79 ++++++++++++++------- context_test.go | 184 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 207 insertions(+), 56 deletions(-) diff --git a/context.go b/context.go index 9a9d133e..cec6ab39 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) == "" { @@ -403,24 +405,46 @@ 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) { + enc := json.NewEncoder(c.response) _, 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 = c.response.Write([]byte(callback + "(")); err != nil { return } - return c.JSONBlob(code, b) + if err = enc.Encode(i); err != nil { + return + } + if _, err = c.response.Write([]byte(");")); err != nil { + return + } + return +} + +func (c *context) jsonBlob(code int, i interface{}, indent *string) error { + enc := json.NewEncoder(c.response) + if indent != nil { + enc.SetIndent("", *indent) + } + c.writeContentType(MIMEApplicationJSONCharsetUTF8) + c.response.WriteHeader(code) + return enc.Encode(i) +} + +func (c *context) JSON(code int, i interface{}) (err error) { + 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) { - b, err := json.MarshalIndent(i, "", indent) - if err != nil { - return - } - return c.JSONBlob(code, b) + return c.jsonBlob(code, i, &indent) } func (c *context) JSONBlob(code int, b []byte) (err error) { @@ -428,11 +452,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 +468,29 @@ 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{}, indent *string) (err error) { + c.writeContentType(MIMEApplicationXMLCharsetUTF8) + c.response.WriteHeader(code) + enc := xml.NewEncoder(c.response) + if indent != nil { + enc.Indent("", *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) XML(code int, i interface{}) (err error) { + 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) { - b, err := xml.MarshalIndent(i, "", indent) - if err != nil { - return - } - return c.XMLBlob(code, b) + 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 a50b4c9f..2da37db7 100644 --- a/context_test.go +++ b/context_test.go @@ -2,6 +2,7 @@ package echo import ( "bytes" + "encoding/json" "encoding/xml" "errors" "io" @@ -14,7 +15,7 @@ import ( "text/template" "time" - "github.com/stretchr/testify/assert" + testify "github.com/stretchr/testify/assert" ) type ( @@ -23,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) } @@ -33,7 +78,7 @@ func TestContext(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec).(*context) - assert := assert.New(t) + assert := testify.New(t) // Echo assert.Equal(e, c.Echo()) @@ -69,7 +114,7 @@ func TestContext(t *testing.T) { if assert.NoError(err) { assert.Equal(http.StatusOK, rec.Code) assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(userJSON, rec.Body.String()) + assert.Equal(userJSON+"\n", rec.Body.String()) } // JSON with "?pretty" @@ -80,7 +125,7 @@ func TestContext(t *testing.T) { if assert.NoError(err) { assert.Equal(http.StatusOK, rec.Code) assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(userJSONPretty, rec.Body.String()) + assert.Equal(userJSONPretty+"\n", rec.Body.String()) } req = httptest.NewRequest(http.MethodGet, "/", nil) // reset @@ -91,7 +136,7 @@ func TestContext(t *testing.T) { if assert.NoError(err) { assert.Equal(http.StatusOK, rec.Code) assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(userJSONPretty, rec.Body.String()) + assert.Equal(userJSONPretty+"\n", rec.Body.String()) } // JSON (error) @@ -108,7 +153,7 @@ func TestContext(t *testing.T) { if assert.NoError(err) { assert.Equal(http.StatusOK, rec.Code) assert.Equal(MIMEApplicationJavaScriptCharsetUTF8, rec.Header().Get(HeaderContentType)) - assert.Equal(callback+"("+userJSON+");", rec.Body.String()) + assert.Equal(callback+"("+userJSON+"\n);", rec.Body.String()) } // XML @@ -149,6 +194,87 @@ func TestContext(t *testing.T) { assert.Equal(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() + assert := testify.New(t) + + // 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(err) { + assert.Equal(http.StatusOK, rec.Code) + assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(buf.String(), rec.Body.String()) + } + }) + + t.Run("xml", func(t *testing.T) { + buf.Reset() + assert := testify.New(t) + + // 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(err) { + assert.Equal(http.StatusOK, rec.Code) + assert.Equal(MIMEApplicationXMLCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(xml.Header+buf.String(), rec.Body.String()) + } + }) + }) + + // Legacy JSONBlob + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + data, err := json.Marshal(user{1, "Jon Snow"}) + assert.NoError(err) + err = c.JSONBlob(http.StatusOK, data) + if assert.NoError(err) { + assert.Equal(http.StatusOK, rec.Code) + assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(userJSON, rec.Body.String()) + } + + // Legacy JSONPBlob + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + callback = "callback" + data, err = json.Marshal(user{1, "Jon Snow"}) + assert.NoError(err) + err = c.JSONPBlob(http.StatusOK, callback, data) + if assert.NoError(err) { + assert.Equal(http.StatusOK, rec.Code) + assert.Equal(MIMEApplicationJavaScriptCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(callback+"("+userJSON+");", rec.Body.String()) + } + + // Legacy XMLBlob + rec = httptest.NewRecorder() + c = e.NewContext(req, rec).(*context) + data, err = xml.Marshal(user{1, "Jon Snow"}) + assert.NoError(err) + err = c.XMLBlob(http.StatusOK, data) + if assert.NoError(err) { + assert.Equal(http.StatusOK, rec.Code) + assert.Equal(MIMEApplicationXMLCharsetUTF8, rec.Header().Get(HeaderContentType)) + assert.Equal(xml.Header+userXML, rec.Body.String()) + } + // String rec = httptest.NewRecorder() c = e.NewContext(req, rec).(*context) @@ -235,7 +361,7 @@ func TestContextCookie(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec).(*context) - assert := assert.New(t) + assert := testify.New(t) // Read single cookie, err := c.Cookie("theme") @@ -280,7 +406,7 @@ func TestContextPath(t *testing.T) { c := e.NewContext(nil, nil) r.Find(http.MethodGet, "/users/1", c) - assert := assert.New(t) + assert := testify.New(t) assert.Equal("/users/:id", c.Path()) @@ -297,14 +423,14 @@ func TestContextPathParam(t *testing.T) { // ParamNames c.SetParamNames("uid", "fid") - assert.EqualValues(t, []string{"uid", "fid"}, c.ParamNames()) + testify.EqualValues(t, []string{"uid", "fid"}, c.ParamNames()) // ParamValues c.SetParamValues("101", "501") - assert.EqualValues(t, []string{"101", "501"}, c.ParamValues()) + testify.EqualValues(t, []string{"101", "501"}, c.ParamValues()) // Param - assert.Equal(t, "501", c.Param("fid")) + testify.Equal(t, "501", c.Param("fid")) } func TestContextFormValue(t *testing.T) { @@ -318,13 +444,13 @@ func TestContextFormValue(t *testing.T) { c := e.NewContext(req, nil) // FormValue - assert.Equal(t, "Jon Snow", c.FormValue("name")) - assert.Equal(t, "jon@labstack.com", c.FormValue("email")) + testify.Equal(t, "Jon Snow", c.FormValue("name")) + testify.Equal(t, "jon@labstack.com", c.FormValue("email")) // FormParams params, err := c.FormParams() - if assert.NoError(t, err) { - assert.Equal(t, url.Values{ + if testify.NoError(t, err) { + testify.Equal(t, url.Values{ "name": []string{"Jon Snow"}, "email": []string{"jon@labstack.com"}, }, params) @@ -340,11 +466,11 @@ func TestContextQueryParam(t *testing.T) { c := e.NewContext(req, nil) // QueryParam - assert.Equal(t, "Jon Snow", c.QueryParam("name")) - assert.Equal(t, "jon@labstack.com", c.QueryParam("email")) + testify.Equal(t, "Jon Snow", c.QueryParam("name")) + testify.Equal(t, "jon@labstack.com", c.QueryParam("email")) // QueryParams - assert.Equal(t, url.Values{ + testify.Equal(t, url.Values{ "name": []string{"Jon Snow"}, "email": []string{"jon@labstack.com"}, }, c.QueryParams()) @@ -355,7 +481,7 @@ func TestContextFormFile(t *testing.T) { buf := new(bytes.Buffer) mr := multipart.NewWriter(buf) w, err := mr.CreateFormFile("file", "test") - if assert.NoError(t, err) { + if testify.NoError(t, err) { w.Write([]byte("test")) } mr.Close() @@ -364,8 +490,8 @@ func TestContextFormFile(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) f, err := c.FormFile("file") - if assert.NoError(t, err) { - assert.Equal(t, "test", f.Filename) + if testify.NoError(t, err) { + testify.Equal(t, "test", f.Filename) } } @@ -380,8 +506,8 @@ func TestContextMultipartForm(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) f, err := c.MultipartForm() - if assert.NoError(t, err) { - assert.NotNil(t, f) + if testify.NoError(t, err) { + testify.NotNil(t, f) } } @@ -390,17 +516,17 @@ func TestContextRedirect(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) rec := httptest.NewRecorder() c := e.NewContext(req, rec) - assert.Equal(t, nil, c.Redirect(http.StatusMovedPermanently, "http://labstack.github.io/echo")) - assert.Equal(t, http.StatusMovedPermanently, rec.Code) - assert.Equal(t, "http://labstack.github.io/echo", rec.Header().Get(HeaderLocation)) - assert.Error(t, c.Redirect(310, "http://labstack.github.io/echo")) + testify.Equal(t, nil, c.Redirect(http.StatusMovedPermanently, "http://labstack.github.io/echo")) + testify.Equal(t, http.StatusMovedPermanently, rec.Code) + testify.Equal(t, "http://labstack.github.io/echo", rec.Header().Get(HeaderLocation)) + testify.Error(t, c.Redirect(310, "http://labstack.github.io/echo")) } func TestContextStore(t *testing.T) { var c Context c = new(context) c.Set("name", "Jon Snow") - assert.Equal(t, "Jon Snow", c.Get("name")) + testify.Equal(t, "Jon Snow", c.Get("name")) } func TestContextHandler(t *testing.T) { @@ -415,5 +541,5 @@ func TestContextHandler(t *testing.T) { c := e.NewContext(nil, nil) r.Find(http.MethodGet, "/handler", c) c.Handler()(c) - assert.Equal(t, "handler", b.String()) + testify.Equal(t, "handler", b.String()) }