From f92a3aa0e5759ec939fbf6711eda02987483e579 Mon Sep 17 00:00:00 2001 From: Jeffrey Zhao Date: Thu, 17 Jan 2019 13:43:31 +0800 Subject: [PATCH] gzip: avoid unnecessary allocations when not compressing (#2396) --- caddyhttp/gzip/gzip.go | 38 ++++++++++++++++++++------- caddyhttp/gzip/responsefilter.go | 2 +- caddyhttp/gzip/responsefilter_test.go | 2 +- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/caddyhttp/gzip/gzip.go b/caddyhttp/gzip/gzip.go index 2da448f2..62e1d5ed 100644 --- a/caddyhttp/gzip/gzip.go +++ b/caddyhttp/gzip/gzip.go @@ -17,6 +17,7 @@ package gzip import ( + "compress/gzip" "io" "net/http" "strings" @@ -65,21 +66,31 @@ outer: } } - // gzipWriter modifies underlying writer at init, - // use a discard writer instead to leave ResponseWriter in - // original form. - gzipWriter := getWriter(c.Level) - defer putWriter(c.Level, gzipWriter) + // In order to avoid unused memory allocation, gzip.putWriter only be called when gzip compression happened. + // see https://github.com/mholt/caddy/issues/2395 gz := &gzipResponseWriter{ - Writer: gzipWriter, ResponseWriterWrapper: &httpserver.ResponseWriterWrapper{ResponseWriter: w}, + newWriter: func() io.Writer { + // gzipWriter modifies underlying writer at init, + // use a discard writer instead to leave ResponseWriter in + // original form. + return getWriter(c.Level) + }, } + defer func() { + if gzWriter, ok := gz.internalWriter.(*gzip.Writer); ok { + putWriter(c.Level, gzWriter) + } + }() + var rw http.ResponseWriter // if no response filter is used if len(c.ResponseFilters) == 0 { // replace discard writer with ResponseWriter - gzipWriter.Reset(w) + if gzWriter, ok := gz.Writer().(*gzip.Writer); ok { + gzWriter.Reset(w) + } rw = gz } else { // wrap gzip writer with ResponseFilterWriter @@ -106,9 +117,10 @@ outer: // gzipResponeWriter wraps the underlying Write method // with a gzip.Writer to compress the output. type gzipResponseWriter struct { - io.Writer + internalWriter io.Writer *httpserver.ResponseWriterWrapper statusCodeWritten bool + newWriter func() io.Writer } // WriteHeader wraps the underlying WriteHeader method to prevent @@ -135,9 +147,17 @@ func (w *gzipResponseWriter) Write(b []byte) (int, error) { if !w.statusCodeWritten { w.WriteHeader(http.StatusOK) } - n, err := w.Writer.Write(b) + n, err := w.Writer().Write(b) return n, err } +//Writer use a lazy way to initialize Writer +func (w *gzipResponseWriter) Writer() io.Writer { + if w.internalWriter == nil { + w.internalWriter = w.newWriter() + } + return w.internalWriter +} + // Interface guards var _ httpserver.HTTPInterfaces = (*gzipResponseWriter)(nil) diff --git a/caddyhttp/gzip/responsefilter.go b/caddyhttp/gzip/responsefilter.go index a78f7ea9..82e10e9c 100644 --- a/caddyhttp/gzip/responsefilter.go +++ b/caddyhttp/gzip/responsefilter.go @@ -82,7 +82,7 @@ func (r *ResponseFilterWriter) WriteHeader(code int) { if r.shouldCompress { // replace discard writer with ResponseWriter - if gzWriter, ok := r.gzipResponseWriter.Writer.(*gzip.Writer); ok { + if gzWriter, ok := r.gzipResponseWriter.Writer().(*gzip.Writer); ok { gzWriter.Reset(r.ResponseWriter) } // use gzip WriteHeader to include and delete diff --git a/caddyhttp/gzip/responsefilter_test.go b/caddyhttp/gzip/responsefilter_test.go index f77169fe..4db5468e 100644 --- a/caddyhttp/gzip/responsefilter_test.go +++ b/caddyhttp/gzip/responsefilter_test.go @@ -47,7 +47,7 @@ func TestLengthFilter(t *testing.T) { for j, filter := range filters { r := httptest.NewRecorder() r.Header().Set("Content-Length", fmt.Sprint(ts.length)) - wWriter := NewResponseFilterWriter([]ResponseFilter{filter}, &gzipResponseWriter{gzip.NewWriter(r), &httpserver.ResponseWriterWrapper{ResponseWriter: r}, false}) + wWriter := NewResponseFilterWriter([]ResponseFilter{filter}, &gzipResponseWriter{gzip.NewWriter(r), &httpserver.ResponseWriterWrapper{ResponseWriter: r}, false, nil}) if filter.ShouldCompress(wWriter) != ts.shouldCompress[j] { t.Errorf("Test %v: Expected %v found %v", i, ts.shouldCompress[j], filter.ShouldCompress(r)) }