go: net/http/httputil: ReverseProxy no longer uses io.ReaderFrom/WriterTo

Before go 1.8 httputil.ReverseProxy would use io.CopyBuffer which would use io.ReaderFrom or io.WriterTo if possible to avoid allocation. In fixing #16659 it removed using io.CopyBuffer and implements its own without the optimisations of io.CopyBuffer to add an extra log.

Please answer these questions before submitting your issue. Thanks!

What did you do?

https://play.golang.org/p/4GjT-hwI3t

What did you expect to see?

WriteTo

What did you see instead?

Read

System details

go version go1.9 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/chris/dev/go"
GORACE=""
GOROOT="/usr/local/opt/go/libexec"
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9
uname -v: Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.11.3
BuildVersion:	15D21
lldb --version: lldb-340.4.110.1
gdb --version: GNU gdb (GDB) 8.0

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 18 (14 by maintainers)

Most upvoted comments

Actually, net/http’s ResponseWriter implements ReaderFrom, so I think option 3 would break #16659.

To clarify: the WaitinfForInfo state is waiting for some justification that WriteTo/ReadFrom provide an actual (measurable) performance gain.

tombergan on Sep 27, 2017

The former makes more sense to me as would also log write errors which adds more symmetry.

Looking at the CL that introduced this, it seems that the intent was to log only read errors: https://go-review.googlesource.com/c/go/+/30692/5/src/net/http/httputil/reverseproxy.go#264. That is why io.CopyBuffer was copied in. Whatever solution we use here should probably retain that property.

tomwans on Sep 19, 2017