code review 8297044: net/http: don't allocate 0-byte io.LimitedReaders for G...

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 Issue 5188

[bradfitz, 2013-04-03 07:29:45]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[adg, 2013-04-03 09:22:21]

LGTM

but yeah, stop now :)

https://codereview.appspot.com/8297044/diff/5001/src/pkg/net/http/transfer.go
File src/pkg/net/http/transfer.go (left):

https://codereview.appspot.com/8297044/diff/5001/src/pkg/net/http/transfer.go...
src/pkg/net/http/transfer.go:336: // TODO: limit the Content-Length. This is an
easy DoS vector.
where'd the todo go?

[bradfitz, 2013-04-03 15:24:04]

https://codereview.appspot.com/8297044/diff/5001/src/pkg/net/http/transfer.go
File src/pkg/net/http/transfer.go (left):

https://codereview.appspot.com/8297044/diff/5001/src/pkg/net/http/transfer.go...
src/pkg/net/http/transfer.go:336: // TODO: limit the Content-Length. This is an
easy DoS vector.
On 2013/04/03 09:22:21, adg wrote:
> where'd the todo go?

It's just old. It's been handled already a dozen places since then.

[r, 2013-04-03 17:16:50]

LGTM
agree about time.Format but not now

[bradfitz, 2013-04-03 17:31:17]

*** Submitted as https://code.google.com/p/go/source/detail?r=753f7b5d8636 ***

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 Issue 5188

R=golang-dev, adg, r
CC=golang-dev
https://codereview.appspot.com/8297044