net/http: don't allocate 0-byte io.LimitedReaders for GET requests

Save an allocation per GET request and don't call io.LimitedReader(r, 0)
just to read 0 bytes.  There's already an eofReader global variable
for when we just want a non-nil io.Reader to immediately EOF.

(Sorry, I know Rob told me to stop, but I was bored on the plane and
wrote this before I received the recent "please, really stop" email.)

benchmark                         old ns/op    new ns/op    delta
BenchmarkServerHandlerTypeLen         13888        13279   -4.39%
BenchmarkServerHandlerNoLen           12912        12229   -5.29%
BenchmarkServerHandlerNoType          13348        12632   -5.36%
BenchmarkServerHandlerNoHeader        10911        10261   -5.96%

benchmark                        old allocs   new allocs    delta
BenchmarkServerHandlerTypeLen            20           19   -5.00%
BenchmarkServerHandlerNoLen              18           17   -5.56%
BenchmarkServerHandlerNoType             18           17   -5.56%
BenchmarkServerHandlerNoHeader           13           12   -7.69%

benchmark                         old bytes    new bytes    delta
BenchmarkServerHandlerTypeLen          1913         1878   -1.83%
BenchmarkServerHandlerNoLen            1878         1843   -1.86%
BenchmarkServerHandlerNoType           1878         1844   -1.81%
BenchmarkServerHandlerNoHeader         1085         1051   -3.13%

Fixes #5188

R=golang-dev, adg, r
CC=golang-dev
https://golang.org/cl/8297044

diff --git a/src/pkg/net/http/transfer.go b/src/pkg/net/http/transfer.go
index 43c6023a3a18fece9a6f8336faa50232edf966a7..53569bcc2fcfd1fc6a0db70d7d659696ecdc3dd4 100644 (file)
--- a/src/pkg/net/http/transfer.go
+++ b/src/pkg/net/http/transfer.go
@@ -328,12 +328,13 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
        switch {
        case chunked(t.TransferEncoding):
                if noBodyExpected(t.RequestMethod) {
-                       t.Body = &body{Reader: io.LimitReader(r, 0), closing: t.Close}
+                       t.Body = &body{Reader: eofReader, closing: t.Close}
                } else {
                        t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing: t.Close}
                }
-       case realLength >= 0:
-               // TODO: limit the Content-Length. This is an easy DoS vector.
+       case realLength == 0:
+               t.Body = &body{Reader: eofReader, closing: t.Close}
+       case realLength > 0:
                t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close}
        default:
                // realLength < 0, i.e. "Content-Length" not mentioned in header
@@ -342,7 +343,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) {
                        t.Body = &body{Reader: r, closing: t.Close}
                } else {
                        // Persistent connection (i.e. HTTP/1.1)
-                       t.Body = &body{Reader: io.LimitReader(r, 0), closing: t.Close}
+                       t.Body = &body{Reader: eofReader, closing: t.Close}
                }
        }
 
@@ -612,30 +613,26 @@ func (b *body) Close() error {
        if b.closed {
                return nil
        }
-       defer func() {
-               b.closed = true
-       }()
-       if b.hdr == nil && b.closing {
+       var err error
+       switch {
+       case b.hdr == nil && b.closing:
                // no trailer and closing the connection next.
                // no point in reading to EOF.
-               return nil
-       }
-
-       // In a server request, don't continue reading from the client
-       // if we've already hit the maximum body size set by the
-       // handler. If this is set, that also means the TCP connection
-       // is about to be closed, so getting to the next HTTP request
-       // in the stream is not necessary.
-       if b.res != nil && b.res.requestBodyLimitHit {
-               return nil
-       }
-
-       // Fully consume the body, which will also lead to us reading
-       // the trailer headers after the body, if present.
-       if _, err := io.Copy(ioutil.Discard, b); err != nil {
-               return err
+       case b.res != nil && b.res.requestBodyLimitHit:
+               // In a server request, don't continue reading from the client
+               // if we've already hit the maximum body size set by the
+               // handler. If this is set, that also means the TCP connection
+               // is about to be closed, so getting to the next HTTP request
+               // in the stream is not necessary.
+       case b.Reader == eofReader:
+               // Nothing to read. No need to io.Copy from it.
+       default:
+               // Fully consume the body, which will also lead to us reading
+               // the trailer headers after the body, if present.
+               _, err = io.Copy(ioutil.Discard, b)
        }
-       return nil
+       b.closed = true
+       return err
 }
 
 // parseContentLength trims whitespace from s and returns -1 if no value