code review 12708046: net/http: simplify server, use bufio Reader.Reset and W...

net/http: simplify server, use bufio Reader.Reset and Writer.Reset

Update issue 5100
Update issue 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%

[bradfitz, 2013-08-10 23:21:16]

Hello golang-dev@googlegroups.com,

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

[r, 2013-08-10 23:23:34]

Another "issue" tip: As I understand it, the way Update works is that it
adds the text that appears after the Update line, so you probably want the
Update line first. I don't know what happens with multiple Update lines,
but we'll find out.

A few thousand more CLs and we'll figure it out, I'm sure.

[r, 2013-08-10 23:23:58]

LGTM

[bradfitz, 2013-08-11 02:22:39]

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

net/http: simplify server, use bufio Reader.Reset and Writer.Reset

Update issue 5100
Update issue 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://codereview.appspot.com/12708046

[r, 2013-08-11 03:14:55]

More detail to help me remember. Your CL said

Update issue 5100
Update issue 6086

<text>


Issue 5100 was updated with the text, including the "Update issue
6086" part, but issue 6086 was not.

-rob

[dsymonds, 2013-08-11 05:29:55]

There are docs describing the syntax:
https://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_...