Code review - Issue 12708046: code review 12708046: net/http: simplify server, use bufio Reader.Reset and W...https://codereview.appspot.com/2013-08-11T05:29:55+00:00rietveld
Message from unknown
2013-08-10T23:20:38+00:00bradfitzurn:md5:18089d2a8665cd7bd4c1b10081626c00
Message from unknown
2013-08-10T23:20:41+00:00bradfitzurn:md5:bf799c4b57178e91bf79b953e8cd08f2
Message from unknown
2013-08-10T23:21:11+00:00bradfitzurn:md5:b56ab344555a8710a9b461128340b34d
Message from bradfitz@golang.org
2013-08-10T23:21:16+00:00bradfitzurn:md5:2368a4b553644c11072fe10e1ceb8156
Hello golang-dev@googlegroups.com,
I'd like you to review this change to
https://go.googlecode.com/hg/
Message from r@golang.org
2013-08-10T23:23:34+00:00rurn:md5:ecc2ac8e6909d4a30c9be6a0123171f4
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.
Message from r@golang.org
2013-08-10T23:23:58+00:00rurn:md5:b289e36386eace20719d74c2ac58d183
LGTM
Message from unknown
2013-08-11T02:22:28+00:00bradfitzurn:md5:24d08c9300700dfda7fcbbda1934bb47
Message from bradfitz@golang.org
2013-08-11T02:22:39+00:00bradfitzurn:md5:49ee938dc34779a3d5d751fc0238b632
*** 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
Message from r@golang.org
2013-08-11T03:14:55+00:00rurn:md5:d35d86404ddacc71917fe7cd7407834a
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
Message from dsymonds@golang.org
2013-08-11T05:29:55+00:00dsymondsurn:md5:9aa517ef5c014ebb1336ef7d53baea36
There are docs describing the syntax:
https://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_control