Issue 52840043: code review 52840043: net/http: cache transport environment lookup (Closed)
Descriptionnet/http: cache transport environment lookup
Apparently this is expensive on Windows.
Fixes Issue 7020
Patch Set 1 #Patch Set 2 : diff -r 889628c63997 https://go.googlecode.com/hg/ #Patch Set 3 : diff -r 889628c63997 https://go.googlecode.com/hg/ #Patch Set 4 : diff -r 911be0abd79f https://go.googlecode.com/hg/ #
MessagesTotal messages: 12
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
LGTM. Alex
Sign in to reply to this message.
Do you want to measure this on Windows before I submit it? On Wed, Jan 15, 2014 at 3:23 PM, <alex.brainman@gmail.com> wrote: > LGTM. > > Alex > > https://codereview.appspot.com/52840043/ >
Sign in to reply to this message.
On 2014/01/15 23:24:03, bradfitz wrote: > Do you want to measure this on Windows before I submit it? > I did test it calls windows Getenv twice (lower and uppercase) only. What do you mean "measure"? Feel free to wait for others. Alex
Sign in to reply to this message.
On Wed, Jan 15, 2014 at 3:29 PM, <alex.brainman@gmail.com> wrote: > On 2014/01/15 23:24:03, bradfitz wrote: > >> Do you want to measure this on Windows before I submit it? >> > > > I did test it calls windows Getenv twice (lower and uppercase) only. > What do you mean "measure"? Feel free to wait for others. > I thought Dmitry said it affected the benchmarks on Windows notably. Just running the net/http benchmarks before & after should be sufficient.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
Don't you add new function to reset them?
Sign in to reply to this message.
ResetCachedEnvironment is in export_test.go, because it's only needed by tests. We're not modifying the public API here. On Wed, Jan 15, 2014 at 3:59 PM, <mattn.jp@gmail.com> wrote: > Don't you add new function to reset them? > > https://codereview.appspot.com/52840043/ > > -- > You received this message because you are subscribed to the Google Groups > "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. >
Sign in to reply to this message.
On 2014/01/15 23:54:34, bradfitz wrote:
>
> I thought Dmitry said it affected the benchmarks on Windows notably. Just
> running the net/http benchmarks before & after should be sufficient.
I run this command:
go test -v -run=NONE -bench=.*
Here are benchmarks before this CL applied:
PASS
BenchmarkHeaderWriteSubset 2000000 1476 ns/op 0 B/op 0
allocs/op
BenchmarkReadRequestChrome 200000 11718 ns/op 52.14 MB/s 1000 B/op
14 allocs/op
BenchmarkReadRequestCurl 500000 5125 ns/op 15.22 MB/s 477 B/op
9 allocs/op
BenchmarkReadRequestApachebench 500000 5093 ns/op 16.10 MB/s 477
B/op 9 allocs/op
BenchmarkReadRequestSiege 500000 7125 ns/op 21.19 MB/s 550 B/op
11 allocs/op
BenchmarkReadRequestWrk 500000 3562 ns/op 11.23 MB/s 429 B/op
7 allocs/op
BenchmarkClientServer 20000 129689 ns/op 5798 B/op 70 allocs/op
BenchmarkClientServerParallel4 20000 122658 ns/op 5799 B/op 70
allocs/op
BenchmarkClientServerParallel64 20000 122658 ns/op 5776 B/op 70
allocs/op
BenchmarkServer 20000 130471 ns/op 1056 B/op 16 allocs/op
BenchmarkServerFakeConnNoKeepAlive 100000 18125 ns/op 1485 B/op
24 allocs/op
BenchmarkServerFakeConnWithKeepAlive 200000 14687 ns/op 1302 B/op
19 allocs/op
BenchmarkServerFakeConnWithKeepAliveLite 200000 8359 ns/op 588
B/op 10 allocs/op
BenchmarkServerHandlerTypeLen 200000 11172 ns/op 1014 B/op 17
allocs/op
BenchmarkServerHandlerNoLen 200000 10000 ns/op 987 B/op 14
allocs/op
BenchmarkServerHandlerNoType 200000 10547 ns/op 1006 B/op 16
allocs/op
BenchmarkServerHandlerNoHeader 200000 8047 ns/op 588 B/op 10
allocs/op
BenchmarkServerHijack 100000 16875 ns/op 9065 B/op 19 allocs/op
ok net/http 48.657s
Here are benchmarks after this CL applied:
PASS
BenchmarkHeaderWriteSubset 1000000 1468 ns/op 0 B/op 0
allocs/op
BenchmarkReadRequestChrome 200000 11640 ns/op 52.49 MB/s 1000 B/op
14 allocs/op
BenchmarkReadRequestCurl 500000 4968 ns/op 15.70 MB/s 478 B/op
9 allocs/op
BenchmarkReadRequestApachebench 500000 4937 ns/op 16.61 MB/s 477
B/op 9 allocs/op
BenchmarkReadRequestSiege 500000 7000 ns/op 21.57 MB/s 550 B/op
11 allocs/op
BenchmarkReadRequestWrk 500000 3406 ns/op 11.74 MB/s 428 B/op
7 allocs/op
BenchmarkClientServer 20000 116408 ns/op 5143 B/op 60 allocs/op
BenchmarkClientServerParallel4 20000 111720 ns/op 5121 B/op 60
allocs/op
BenchmarkClientServerParallel64 20000 111720 ns/op 5144 B/op 60
allocs/op
BenchmarkServer 10000 118752 ns/op 1059 B/op 16 allocs/op
BenchmarkServerFakeConnNoKeepAlive 100000 18125 ns/op 1485 B/op
24 allocs/op
BenchmarkServerFakeConnWithKeepAlive 200000 14687 ns/op 1303 B/op
19 allocs/op
BenchmarkServerFakeConnWithKeepAliveLite 200000 8437 ns/op 588
B/op 10 allocs/op
BenchmarkServerHandlerTypeLen 200000 11172 ns/op 1014 B/op 17
allocs/op
BenchmarkServerHandlerNoLen 200000 10000 ns/op 986 B/op 14
allocs/op
BenchmarkServerHandlerNoType 200000 10547 ns/op 1005 B/op 16
allocs/op
BenchmarkServerHandlerNoHeader 200000 7968 ns/op 587 B/op 10
allocs/op
BenchmarkServerHijack 100000 16719 ns/op 9064 B/op 19 allocs/op
ok net/http 44.079s
Here are the diffs:
benchmark old ns/op new ns/op delta
BenchmarkHeaderWriteSubset 1476 1468 -0.54%
BenchmarkReadRequestChrome 11718 11640 -0.67%
BenchmarkReadRequestCurl 5125 4968 -3.06%
BenchmarkReadRequestApachebench 5093 4937 -3.06%
BenchmarkReadRequestSiege 7125 7000 -1.75%
BenchmarkReadRequestWrk 3562 3406 -4.38%
BenchmarkClientServer 129689 116408 -10.24%
BenchmarkClientServerParallel4 122658 111720 -8.92%
BenchmarkClientServerParallel64 122658 111720 -8.92%
BenchmarkServer 130471 118752 -8.98%
BenchmarkServerFakeConnNoKeepAlive 18125 18125 +0.00%
BenchmarkServerFakeConnWithKeepAlive 14687 14687 +0.00%
BenchmarkServerFakeConnWithKeepAliveLite 8359 8437 +0.93%
BenchmarkServerHandlerTypeLen 11172 11172 +0.00%
BenchmarkServerHandlerNoLen 10000 10000 +0.00%
BenchmarkServerHandlerNoType 10547 10547 +0.00%
BenchmarkServerHandlerNoHeader 8047 7968 -0.98%
BenchmarkServerHijack 16875 16719 -0.92%
benchmark old MB/s new MB/s speedup
BenchmarkReadRequestChrome 52.14 52.49 1.01x
BenchmarkReadRequestCurl 15.22 15.70 1.03x
BenchmarkReadRequestApachebench 16.10 16.61 1.03x
BenchmarkReadRequestSiege 21.19 21.57 1.02x
BenchmarkReadRequestWrk 11.23 11.74 1.05x
benchmark old allocs new allocs delta
BenchmarkHeaderWriteSubset 0 0 n/a%
BenchmarkReadRequestChrome 14 14 0.00%
BenchmarkReadRequestCurl 9 9 0.00%
BenchmarkReadRequestApachebench 9 9 0.00%
BenchmarkReadRequestSiege 11 11 0.00%
BenchmarkReadRequestWrk 7 7 0.00%
BenchmarkClientServer 70 60 -14.29%
BenchmarkClientServerParallel4 70 60 -14.29%
BenchmarkClientServerParallel64 70 60 -14.29%
BenchmarkServer 16 16 0.00%
BenchmarkServerFakeConnNoKeepAlive 24 24 0.00%
BenchmarkServerFakeConnWithKeepAlive 19 19 0.00%
BenchmarkServerFakeConnWithKeepAliveLite 10 10 0.00%
BenchmarkServerHandlerTypeLen 17 17 0.00%
BenchmarkServerHandlerNoLen 14 14 0.00%
BenchmarkServerHandlerNoType 16 16 0.00%
BenchmarkServerHandlerNoHeader 10 10 0.00%
BenchmarkServerHijack 19 19 0.00%
benchmark old bytes new bytes delta
BenchmarkHeaderWriteSubset 0 0 n/a%
BenchmarkReadRequestChrome 1000 1000 0.00%
BenchmarkReadRequestCurl 477 478 0.21%
BenchmarkReadRequestApachebench 477 477 0.00%
BenchmarkReadRequestSiege 550 550 0.00%
BenchmarkReadRequestWrk 429 428 -0.23%
BenchmarkClientServer 5798 5143 -11.30%
BenchmarkClientServerParallel4 5799 5121 -11.69%
BenchmarkClientServerParallel64 5776 5144 -10.94%
BenchmarkServer 1056 1059 0.28%
BenchmarkServerFakeConnNoKeepAlive 1485 1485 0.00%
BenchmarkServerFakeConnWithKeepAlive 1302 1303 0.08%
BenchmarkServerFakeConnWithKeepAliveLite 588 588 0.00%
BenchmarkServerHandlerTypeLen 1014 1014 0.00%
BenchmarkServerHandlerNoLen 987 986 -0.10%
BenchmarkServerHandlerNoType 1006 1005 -0.10%
BenchmarkServerHandlerNoHeader 588 587 -0.17%
BenchmarkServerHijack 9065 9064 -0.01%
Like Dmitry said, these particular windows syscalls allocate memory (to
translate utf8 to utf16 and back), so we should avoid it, if we can.
I hope it helps.
Alex
Sign in to reply to this message.
On Thu, Jan 16, 2014 at 4:26 AM, <alex.brainman@gmail.com> wrote: > BenchmarkClientServer 5798 5143 > -11.30% this is client/server benchmark, so if you use only client, then it can be whole 20-25% or garbage
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=f16865e8a2c8 *** net/http: cache transport environment lookup Apparently this is expensive on Windows. Fixes Issue 7020 R=golang-codereviews, alex.brainman, mattn.jp, dvyukov CC=golang-codereviews https://codereview.appspot.com/52840043
Sign in to reply to this message.
Message was sent while issue was closed.
FYI ---------- Forwarded message ---------- Subject: Perf changes on windows-amd64 by net/http: cache transport environment lookup Change f16865e8a2c8 caused perf changes on windows-amd64: net/http: cache transport environment lookup Apparently this is expensive on Windows. Fixes Issue 7020 R=golang-codereviews, alex.brainman, mattn.jp, dvyukov CC=golang-codereviews https://codereview.appspot.com/52840043 http://code.google.com/p/go/source/detail?r=f16865e8a2c8 http-1 old new delta allocated 7660 7006 -8.54 allocs 70 60 -14.29 time 110329 99829 -9.52 http-8 old new delta allocated 12090 10970 -9.26 allocs 91 79 -13.19 time 44775 41061 -8.29 http://goperfd.appspot.com/perfdetail?commit=f16865e8a2c8349ce6a231f0629f1b98...
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
