Skip to content

Commit

Permalink
Use simpler *gzip.Writer pool design
Browse files Browse the repository at this point in the history
This is slightly more efficient as it avoids calling Reset on a newly
created writer, it's also much simpler code.
  • Loading branch information
tmthrgd committed Jan 6, 2019
1 parent d5e0219 commit c8683fd
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 29 deletions.
39 changes: 15 additions & 24 deletions gzip.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package gziphandler
import (
"bufio"
"compress/gzip"
"io"
"net"
"net/http"
"sync"
Expand All @@ -30,29 +31,24 @@ var bufferPool = &sync.Pool{
},
}

var gzipPool [BestCompression - HuffmanOnly + 1]*sync.Pool
var gzipWriterPools [gzip.BestCompression - gzip.HuffmanOnly + 1]sync.Pool

func gzipPoolIndex(level int) int {
return level - HuffmanOnly
func gzipWriterPool(level int) *sync.Pool {
return &gzipWriterPools[level-gzip.HuffmanOnly]
}

func addGzipPool(level int) {
gzipPool[gzipPoolIndex(level)] = &sync.Pool{
New: func() interface{} {
w, err := gzip.NewWriterLevel(nil, level)
if err != nil {
panic(err)
}

return w
},
func gzipWriterGet(w io.Writer, level int) *gzip.Writer {
if gw, ok := gzipWriterPool(level).Get().(*gzip.Writer); ok {
gw.Reset(w)
return gw
}

gw, _ := gzip.NewWriterLevel(w, level)
return gw
}

func init() {
for level := HuffmanOnly; level <= BestCompression; level++ {
addGzipPool(level)
}
func gzipWriterPut(gw *gzip.Writer, level int) {
gzipWriterPool(level).Put(gw)
}

// These constants are copied from the gzip package, so
Expand Down Expand Up @@ -166,8 +162,7 @@ func (w *responseWriter) startGzip() (err error) {
// Bytes written during ServeHTTP are redirected to
// this gzip writer before being written to the
// underlying response.
w.gw = w.h.pool().Get().(*gzip.Writer)
w.gw.Reset(w.ResponseWriter)
w.gw = gzipWriterGet(w.ResponseWriter, w.h.level)

if buf := *w.buf; len(buf) != 0 {
// Flush the buffer into the gzip response.
Expand Down Expand Up @@ -287,7 +282,7 @@ func (w *responseWriter) Close() error {
func (w *responseWriter) closeGzipped() error {
err := w.gw.Close()

w.h.pool().Put(w.gw)
gzipWriterPut(w.gw, w.h.level)
w.gw = nil

return err
Expand Down Expand Up @@ -330,10 +325,6 @@ type handler struct {
config
}

func (h *handler) pool() *sync.Pool {
return gzipPool[gzipPoolIndex(h.level)]
}

func (h *handler) shouldGzip(r *http.Request) bool {
if h.config.shouldGzip != nil {
switch h.config.shouldGzip(r) {
Expand Down
7 changes: 2 additions & 5 deletions gzip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,6 @@ func TestMinSizePanicsForInvalid(t *testing.T) {
}

func TestGzipDoubleClose(t *testing.T) {
addGzipPool(DefaultCompression)
pool := gzipPool[gzipPoolIndex(DefaultCompression)]

h := Gzip(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// call close here and it'll get called again interally by
// NewGzipLevelHandler's handler defer
Expand All @@ -212,8 +209,8 @@ func TestGzipDoubleClose(t *testing.T) {

// the second close shouldn't have added the same writer
// so we pull out 2 writers from the pool and make sure they're different
w1 := pool.Get()
w2 := pool.Get()
w1 := gzipWriterGet(nil, DefaultCompression)
w2 := gzipWriterGet(nil, DefaultCompression)
// assert.NotEqual looks at the value and not the address, so we use regular ==
assert.False(t, w1 == w2)
}
Expand Down

0 comments on commit c8683fd

Please sign in to comment.