code review 8094046: net/http: reuse textproto.Readers; remove 2 more allocations

net/http: reuse textproto.Readers; remove 2 more allocations

Saves both the textproto.Reader allocation, and its internal
scratch buffer growing.

benchmark                                   old ns/op    new ns/op    delta
BenchmarkServerFakeConnWithKeepAliveLite        10324        10149   -1.70%

benchmark                                  old allocs   new allocs    delta
BenchmarkServerFakeConnWithKeepAliveLite           19           17  -10.53%

benchmark                                   old bytes    new bytes    delta
BenchmarkServerFakeConnWithKeepAliveLite         1559         1492   -4.30%

[bradfitz, 2013-03-28 21:42:06]

Hello golang-dev@googlegroups.com,

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

[r, 2013-03-28 21:43:28]

LGTM

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go
File src/pkg/net/http/request.go (right):

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go#...
src/pkg/net/http/request.go:482: var textprotoReaderCache = make(chan
*textproto.Reader, 4)
how did you pick 4? odd choice. (ha)

[gri, 2013-03-28 21:46:08]

LGTM

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go
File src/pkg/net/http/request.go (right):

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go#...
src/pkg/net/http/request.go:481: // TODO: use a sync.Cache when available
// TODO(bradfitz)

otherwise it is not found by go/doc

[bradfitz, 2013-03-28 21:47:16]

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go
File src/pkg/net/http/request.go (right):

https://codereview.appspot.com/8094046/diff/5001/src/pkg/net/http/request.go#...
src/pkg/net/http/request.go:482: var textprotoReaderCache = make(chan
*textproto.Reader, 4)
On 2013/03/28 21:43:29, r wrote:
> how did you pick 4? odd choice. (ha)

Quite arbitrary. It's more than 0 and 1.

If I picked a big number, that's more memory wasted. Hence the TODO to use a
per-M/P sync.Cache.

[bradfitz, 2013-03-28 21:51:23]

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

net/http: reuse textproto.Readers; remove 2 more allocations

Saves both the textproto.Reader allocation, and its internal
scratch buffer growing.

benchmark                                   old ns/op    new ns/op    delta
BenchmarkServerFakeConnWithKeepAliveLite        10324        10149   -1.70%

benchmark                                  old allocs   new allocs    delta
BenchmarkServerFakeConnWithKeepAliveLite           19           17  -10.53%

benchmark                                   old bytes    new bytes    delta
BenchmarkServerFakeConnWithKeepAliveLite         1559         1492   -4.30%

R=golang-dev, r, gri
CC=golang-dev
https://codereview.appspot.com/8094046