From 3e38b7f2465a6ab476cb0e184f2b2abee1a6e76f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 10 Aug 2013 19:22:44 -0700 Subject: [PATCH] net/http: simplify server, use bufio Reader.Reset and Writer.Reset Update #5100 Update #6086 Remove switchReader, switchWriter, switchReaderPair, switchWriterPair, etc. Now it only maintains pools of bufio Readers and Writers, but uses Reset instead of working around all their previously-associated state. Compared to before the bufio Reset change, it's the same number of allocations, and also faster: benchmark old ns/op new ns/op delta BenchmarkClientServer 111218 109828 -1.25% BenchmarkClientServerParallel4 70580 70013 -0.80% BenchmarkClientServerParallel64 72636 68919 -5.12% BenchmarkServer 139858 137068 -1.99% BenchmarkServerFakeConnNoKeepAlive 14619 14314 -2.09% BenchmarkServerFakeConnWithKeepAlive 12390 11361 -8.31% BenchmarkServerFakeConnWithKeepAliveLite 7630 7306 -4.25% BenchmarkServerHandlerTypeLen 9688 9342 -3.57% BenchmarkServerHandlerNoLen 8700 8470 -2.64% BenchmarkServerHandlerNoType 9255 8949 -3.31% BenchmarkServerHandlerNoHeader 7058 6806 -3.57% benchmark old allocs new allocs delta BenchmarkClientServer 61 61 0.00% BenchmarkClientServerParallel4 61 61 0.00% BenchmarkClientServerParallel64 61 61 0.00% BenchmarkServer 16 16 0.00% BenchmarkServerFakeConnNoKeepAlive 24 24 0.00% BenchmarkServerFakeConnWithKeepAlive 19 19 0.00% BenchmarkServerFakeConnWithKeepAliveLite 9 9 0.00% BenchmarkServerHandlerTypeLen 17 17 0.00% BenchmarkServerHandlerNoLen 14 14 0.00% BenchmarkServerHandlerNoType 15 15 0.00% BenchmarkServerHandlerNoHeader 9 9 0.00% benchmark old bytes new bytes delta BenchmarkClientServer 6988 6985 -0.04% BenchmarkClientServerParallel4 6979 6985 0.09% BenchmarkClientServerParallel64 7002 7019 0.24% BenchmarkServer 1846 1848 0.11% BenchmarkServerFakeConnNoKeepAlive 2420 2412 -0.33% BenchmarkServerFakeConnWithKeepAlive 2126 2129 0.14% BenchmarkServerFakeConnWithKeepAliveLite 989 990 0.10% BenchmarkServerHandlerTypeLen 1818 1819 0.06% BenchmarkServerHandlerNoLen 1775 1777 0.11% BenchmarkServerHandlerNoType 1783 1785 0.11% BenchmarkServerHandlerNoHeader 989 990 0.10% R=golang-dev, r CC=golang-dev https://golang.org/cl/12708046 --- src/pkg/net/http/server.go | 81 +++++++++++--------------------------- 1 file changed, 24 insertions(+), 57 deletions(-) diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go index 56b8f4a58a8..b58364c7673 100644 --- a/src/pkg/net/http/server.go +++ b/src/pkg/net/http/server.go @@ -110,8 +110,6 @@ type conn struct { sr liveSwitchReader // where the LimitReader reads from; usually the rwc lr *io.LimitedReader // io.LimitReader(sr) buf *bufio.ReadWriter // buffered(lr,rwc), reading from bufio->limitReader->sr->rwc - bufswr *switchReader // the *switchReader io.Reader source of buf - bufsww *switchWriter // the *switchWriter io.Writer dest of buf tlsState *tls.ConnectionState // or nil when not using TLS mu sync.Mutex // guards the following @@ -430,34 +428,20 @@ func (srv *Server) newConn(rwc net.Conn) (c *conn, err error) { } c.sr = liveSwitchReader{r: c.rwc} c.lr = io.LimitReader(&c.sr, noLimit).(*io.LimitedReader) - br, sr := newBufioReader(c.lr) - bw, sw := newBufioWriterSize(c.rwc, 4<<10) + br := newBufioReader(c.lr) + bw := newBufioWriterSize(c.rwc, 4<<10) c.buf = bufio.NewReadWriter(br, bw) - c.bufswr = sr - c.bufsww = sw return c, nil } -// TODO: remove this, if issue 5100 is fixed -type bufioReaderPair struct { - br *bufio.Reader - sr *switchReader // from which the bufio.Reader is reading -} - -// TODO: remove this, if issue 5100 is fixed -type bufioWriterPair struct { - bw *bufio.Writer - sw *switchWriter // to which the bufio.Writer is writing -} - // TODO: use a sync.Cache instead var ( - bufioReaderCache = make(chan bufioReaderPair, 4) - bufioWriterCache2k = make(chan bufioWriterPair, 4) - bufioWriterCache4k = make(chan bufioWriterPair, 4) + bufioReaderCache = make(chan *bufio.Reader, 4) + bufioWriterCache2k = make(chan *bufio.Writer, 4) + bufioWriterCache4k = make(chan *bufio.Writer, 4) ) -func bufioWriterCache(size int) chan bufioWriterPair { +func bufioWriterCache(size int) chan *bufio.Writer { switch size { case 2 << 10: return bufioWriterCache2k @@ -467,55 +451,38 @@ func bufioWriterCache(size int) chan bufioWriterPair { return nil } -func newBufioReader(r io.Reader) (*bufio.Reader, *switchReader) { +func newBufioReader(r io.Reader) *bufio.Reader { select { case p := <-bufioReaderCache: - p.sr.Reader = r - return p.br, p.sr + p.Reset(r) + return p default: - sr := &switchReader{r} - return bufio.NewReader(sr), sr + return bufio.NewReader(r) } } -func putBufioReader(br *bufio.Reader, sr *switchReader) { - if n := br.Buffered(); n > 0 { - io.CopyN(ioutil.Discard, br, int64(n)) - } - br.Read(nil) // clears br.err - sr.Reader = nil +func putBufioReader(br *bufio.Reader) { + br.Reset(nil) select { - case bufioReaderCache <- bufioReaderPair{br, sr}: + case bufioReaderCache <- br: default: } } -func newBufioWriterSize(w io.Writer, size int) (*bufio.Writer, *switchWriter) { +func newBufioWriterSize(w io.Writer, size int) *bufio.Writer { select { case p := <-bufioWriterCache(size): - p.sw.Writer = w - return p.bw, p.sw + p.Reset(w) + return p default: - sw := &switchWriter{w} - return bufio.NewWriterSize(sw, size), sw + return bufio.NewWriterSize(w, size) } } -func putBufioWriter(bw *bufio.Writer, sw *switchWriter) { - if bw.Buffered() > 0 { - // It must have failed to flush to its target - // earlier. We can't reuse this bufio.Writer. - return - } - if err := bw.Flush(); err != nil { - // Its sticky error field is set, which is returned by - // Flush even when there's no data buffered. This - // bufio Writer is dead to us. Don't reuse it. - return - } - sw.Writer = nil +func putBufioWriter(bw *bufio.Writer) { + bw.Reset(nil) select { - case bufioWriterCache(bw.Available()) <- bufioWriterPair{bw, sw}: + case bufioWriterCache(bw.Available()) <- bw: default: } } @@ -621,7 +588,7 @@ func (c *conn) readRequest() (w *response, err error) { contentLength: -1, } w.cw.res = w - w.w, w.sw = newBufioWriterSize(&w.cw, bufferBeforeChunkingSize) + w.w = newBufioWriterSize(&w.cw, bufferBeforeChunkingSize) return w, nil } @@ -1007,7 +974,7 @@ func (w *response) finishRequest() { } w.w.Flush() - putBufioWriter(w.w, w.sw) + putBufioWriter(w.w) w.cw.close() w.conn.buf.Flush() @@ -1040,11 +1007,11 @@ func (c *conn) finalFlush() { // Steal the bufio.Reader (~4KB worth of memory) and its associated // reader for a future connection. - putBufioReader(c.buf.Reader, c.bufswr) + putBufioReader(c.buf.Reader) // Steal the bufio.Writer (~4KB worth of memory) and its associated // writer for a future connection. - putBufioWriter(c.buf.Writer, c.bufsww) + putBufioWriter(c.buf.Writer) c.buf = nil }