mirror of
https://github.com/golang/go
synced 2024-09-06 00:19:45 +00:00
net/http: let Transport request body writes use sendfile
net.TCPConn has the ability to send data out using system calls such as sendfile when the source data comes from an *os.File. However, the way that I/O has been laid out in the transport means that the File is actually wrapped behind two outer io.Readers, and as such the TCP stack cannot properly type-assert the reader, ensuring that it falls back to genericReadFrom. This commit does the following: * Removes transferBodyReader and moves its functionality to a new doBodyCopy helper. This is not an io.Reader implementation, but no functionality is lost this way, and it allows us to unwrap one layer from the body. * The second layer of the body is unwrapped if the original reader was wrapped with ioutil.NopCloser, which is what NewRequest wraps the body in if it's not a ReadCloser on its own. The unwrap operation passes through the existing body if there's no nopCloser. Note that this depends on change https://golang.org/cl/163737 to properly function, as the lack of ReaderFrom implementation otherwise means that this functionality is essentially walled off. Benchmarks between this commit and https://golang.org/cl/163862, incorporating https://golang.org/cl/163737: linux/amd64: name old time/op new time/op delta FileAndServer_1KB/NoTLS-4 53.2µs ± 0% 53.3µs ± 0% ~ (p=0.075 n=10+9) FileAndServer_1KB/TLS-4 61.2µs ± 0% 60.7µs ± 0% -0.77% (p=0.000 n=10+9) FileAndServer_16MB/NoTLS-4 25.3ms ± 5% 3.8ms ± 6% -84.95% (p=0.000 n=10+10) FileAndServer_16MB/TLS-4 33.2ms ± 2% 13.4ms ± 2% -59.57% (p=0.000 n=10+10) FileAndServer_64MB/NoTLS-4 106ms ± 4% 16ms ± 2% -84.45% (p=0.000 n=10+10) FileAndServer_64MB/TLS-4 129ms ± 1% 54ms ± 3% -58.32% (p=0.000 n=8+10) name old speed new speed delta FileAndServer_1KB/NoTLS-4 19.2MB/s ± 0% 19.2MB/s ± 0% ~ (p=0.095 n=10+9) FileAndServer_1KB/TLS-4 16.7MB/s ± 0% 16.9MB/s ± 0% +0.78% (p=0.000 n=10+9) FileAndServer_16MB/NoTLS-4 664MB/s ± 5% 4415MB/s ± 6% +565.27% (p=0.000 n=10+10) FileAndServer_16MB/TLS-4 505MB/s ± 2% 1250MB/s ± 2% +147.32% (p=0.000 n=10+10) FileAndServer_64MB/NoTLS-4 636MB/s ± 4% 4090MB/s ± 2% +542.81% (p=0.000 n=10+10) FileAndServer_64MB/TLS-4 522MB/s ± 1% 1251MB/s ± 3% +139.95% (p=0.000 n=8+10) darwin/amd64: name old time/op new time/op delta FileAndServer_1KB/NoTLS-8 93.0µs ± 5% 96.6µs ±11% ~ (p=0.190 n=10+10) FileAndServer_1KB/TLS-8 105µs ± 7% 100µs ± 5% -5.14% (p=0.002 n=10+9) FileAndServer_16MB/NoTLS-8 87.5ms ±19% 10.0ms ± 6% -88.57% (p=0.000 n=10+10) FileAndServer_16MB/TLS-8 52.7ms ±11% 17.4ms ± 5% -66.92% (p=0.000 n=10+10) FileAndServer_64MB/NoTLS-8 363ms ±54% 39ms ± 7% -89.24% (p=0.000 n=10+10) FileAndServer_64MB/TLS-8 209ms ±13% 73ms ± 5% -65.37% (p=0.000 n=9+10) name old speed new speed delta FileAndServer_1KB/NoTLS-8 11.0MB/s ± 5% 10.6MB/s ±10% ~ (p=0.184 n=10+10) FileAndServer_1KB/TLS-8 9.75MB/s ± 7% 10.27MB/s ± 5% +5.26% (p=0.003 n=10+9) FileAndServer_16MB/NoTLS-8 194MB/s ±16% 1680MB/s ± 6% +767.83% (p=0.000 n=10+10) FileAndServer_16MB/TLS-8 319MB/s ±10% 963MB/s ± 4% +201.36% (p=0.000 n=10+10) FileAndServer_64MB/NoTLS-8 180MB/s ±31% 1719MB/s ± 7% +853.61% (p=0.000 n=9+10) FileAndServer_64MB/TLS-8 321MB/s ±12% 926MB/s ± 5% +188.24% (p=0.000 n=9+10) Updates #30377.
This commit is contained in:
Chris Marchesi
parent
9a71015860
commit
4a77dd1b80
|
|
@ -53,19 +53,6 @@ func (br *byteReader) Read(p []byte) (n int, err error) {
|
|||
return 1, io.EOF
|
||||
}
|
||||
|
||||
// transferBodyReader is an io.Reader that reads from tw.Body
|
||||
// and records any non-EOF error in tw.bodyReadError.
|
||||
// It is exactly 1 pointer wide to avoid allocations into interfaces.
|
||||
type transferBodyReader struct{ tw *transferWriter }
|
||||
|
||||
func (br transferBodyReader) Read(p []byte) (n int, err error) {
|
||||
n, err = br.tw.Body.Read(p)
|
||||
if err != nil && err != io.EOF {
|
||||
br.tw.bodyReadError = err
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// transferWriter inspects the fields of a user-supplied Request or Response,
|
||||
// sanitizes them without changing the user object and provides methods for
|
||||
// writing the respective header, body and trailer in wire format.
|
||||
|
|
@ -347,15 +334,18 @@ func (t *transferWriter) writeBody(w io.Writer) error {
|
|||
var err error
|
||||
var ncopy int64
|
||||
|
||||
// Write body
|
||||
// Write body. We "unwrap" the body first if it was wrapped in a
|
||||
// nopCloser. This is to ensure that we can take advantage of
|
||||
// OS-level optimizations in the event that the body is an
|
||||
// *os.File.
|
||||
if t.Body != nil {
|
||||
var body = transferBodyReader{t}
|
||||
var body = t.unwrapBody()
|
||||
if chunked(t.TransferEncoding) {
|
||||
if bw, ok := w.(*bufio.Writer); ok && !t.IsResponse {
|
||||
w = &internal.FlushAfterChunkWriter{Writer: bw}
|
||||
}
|
||||
cw := internal.NewChunkedWriter(w)
|
||||
_, err = io.Copy(cw, body)
|
||||
_, err = t.doBodyCopy(cw, body)
|
||||
if err == nil {
|
||||
err = cw.Close()
|
||||
}
|
||||
|
|
@ -364,14 +354,14 @@ func (t *transferWriter) writeBody(w io.Writer) error {
|
|||
if t.Method == "CONNECT" {
|
||||
dst = bufioFlushWriter{dst}
|
||||
}
|
||||
ncopy, err = io.Copy(dst, body)
|
||||
ncopy, err = t.doBodyCopy(dst, body)
|
||||
} else {
|
||||
ncopy, err = io.Copy(w, io.LimitReader(body, t.ContentLength))
|
||||
ncopy, err = t.doBodyCopy(w, io.LimitReader(body, t.ContentLength))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var nextra int64
|
||||
nextra, err = io.Copy(ioutil.Discard, body)
|
||||
nextra, err = t.doBodyCopy(ioutil.Discard, body)
|
||||
ncopy += nextra
|
||||
}
|
||||
if err != nil {
|
||||
|
|
@ -402,6 +392,31 @@ func (t *transferWriter) writeBody(w io.Writer) error {
|
|||
return err
|
||||
}
|
||||
|
||||
// doBodyCopy wraps a copy operation, with any resulting error also
|
||||
// being saved in bodyReadError.
|
||||
//
|
||||
// This function is only intended for use in writeBody.
|
||||
func (t *transferWriter) doBodyCopy(dst io.Writer, src io.Reader) (n int64, err error) {
|
||||
n, err = io.Copy(dst, src)
|
||||
if err != nil && err != io.EOF {
|
||||
t.bodyReadError = err
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// unwrapBodyReader unwraps the body's inner reader if it's a
|
||||
// nopCloser. This is to ensure that body writes sourced from local
|
||||
// files (*os.File types) are properly optimized.
|
||||
//
|
||||
// This function is only intended for use in writeBody.
|
||||
func (t *transferWriter) unwrapBody() io.Reader {
|
||||
if reflect.TypeOf(t.Body) == nopCloserType {
|
||||
return reflect.ValueOf(t.Body).Field(0).Interface().(io.Reader)
|
||||
}
|
||||
|
||||
return t.Body
|
||||
}
|
||||
|
||||
type transferReader struct {
|
||||
// Input
|
||||
Header Header
|
||||
|
|
|
|||
|
|
@ -7,8 +7,12 @@ package http
|
|||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"crypto/rand"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
|
@ -90,3 +94,187 @@ func TestDetectInMemoryReaders(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
type mockTransferWriter struct {
|
||||
CalledReader io.Reader
|
||||
WriteCalled bool
|
||||
}
|
||||
|
||||
var _ io.ReaderFrom = (*mockTransferWriter)(nil)
|
||||
|
||||
func (w *mockTransferWriter) ReadFrom(r io.Reader) (int64, error) {
|
||||
w.CalledReader = r
|
||||
return io.Copy(ioutil.Discard, r)
|
||||
}
|
||||
|
||||
func (w *mockTransferWriter) Write(p []byte) (int, error) {
|
||||
w.WriteCalled = true
|
||||
return ioutil.Discard.Write(p)
|
||||
}
|
||||
|
||||
func TestTransferWriterWriteBodyReaderTypes(t *testing.T) {
|
||||
fileType := reflect.TypeOf(&os.File{})
|
||||
bufferType := reflect.TypeOf(&bytes.Buffer{})
|
||||
|
||||
nBytes := int64(1 << 10)
|
||||
newFileFunc := func() (r io.Reader, done func(), err error) {
|
||||
f, err := ioutil.TempFile("", "net-http-newfilefunc")
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
// Write some bytes to the file to enable reading.
|
||||
if _, err := io.CopyN(f, rand.Reader, nBytes); err != nil {
|
||||
return nil, nil, fmt.Errorf("failed to write data to file: %v", err)
|
||||
}
|
||||
if _, err := f.Seek(0, 0); err != nil {
|
||||
return nil, nil, fmt.Errorf("failed to seek to front: %v", err)
|
||||
}
|
||||
|
||||
done = func() {
|
||||
f.Close()
|
||||
os.Remove(f.Name())
|
||||
}
|
||||
|
||||
return f, done, nil
|
||||
}
|
||||
|
||||
newBufferFunc := func() (io.Reader, func(), error) {
|
||||
return bytes.NewBuffer(make([]byte, nBytes)), func() {}, nil
|
||||
}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
bodyFunc func() (io.Reader, func(), error)
|
||||
method string
|
||||
contentLength int64
|
||||
transferEncoding []string
|
||||
limitedReader bool
|
||||
expectedReader reflect.Type
|
||||
expectedWrite bool
|
||||
}{
|
||||
{
|
||||
name: "file, non-chunked, size set",
|
||||
bodyFunc: newFileFunc,
|
||||
method: "PUT",
|
||||
contentLength: nBytes,
|
||||
limitedReader: true,
|
||||
expectedReader: fileType,
|
||||
},
|
||||
{
|
||||
name: "file, non-chunked, size set, nopCloser wrapped",
|
||||
method: "PUT",
|
||||
bodyFunc: func() (io.Reader, func(), error) {
|
||||
r, cleanup, err := newFileFunc()
|
||||
return ioutil.NopCloser(r), cleanup, err
|
||||
},
|
||||
contentLength: nBytes,
|
||||
limitedReader: true,
|
||||
expectedReader: fileType,
|
||||
},
|
||||
{
|
||||
name: "file, non-chunked, negative size",
|
||||
method: "PUT",
|
||||
bodyFunc: newFileFunc,
|
||||
contentLength: -1,
|
||||
expectedReader: fileType,
|
||||
},
|
||||
{
|
||||
name: "file, non-chunked, CONNECT, negative size",
|
||||
method: "CONNECT",
|
||||
bodyFunc: newFileFunc,
|
||||
contentLength: -1,
|
||||
expectedReader: fileType,
|
||||
},
|
||||
{
|
||||
name: "file, chunked",
|
||||
method: "PUT",
|
||||
bodyFunc: newFileFunc,
|
||||
transferEncoding: []string{"chunked"},
|
||||
expectedWrite: true,
|
||||
},
|
||||
{
|
||||
name: "buffer, non-chunked, size set",
|
||||
bodyFunc: newBufferFunc,
|
||||
method: "PUT",
|
||||
contentLength: nBytes,
|
||||
limitedReader: true,
|
||||
expectedReader: bufferType,
|
||||
},
|
||||
{
|
||||
name: "buffer, non-chunked, size set, nopCloser wrapped",
|
||||
method: "PUT",
|
||||
bodyFunc: func() (io.Reader, func(), error) {
|
||||
r, cleanup, err := newBufferFunc()
|
||||
return ioutil.NopCloser(r), cleanup, err
|
||||
},
|
||||
contentLength: nBytes,
|
||||
limitedReader: true,
|
||||
expectedReader: bufferType,
|
||||
},
|
||||
{
|
||||
name: "buffer, non-chunked, negative size",
|
||||
method: "PUT",
|
||||
bodyFunc: newBufferFunc,
|
||||
contentLength: -1,
|
||||
expectedWrite: true,
|
||||
},
|
||||
{
|
||||
name: "buffer, non-chunked, CONNECT, negative size",
|
||||
method: "CONNECT",
|
||||
bodyFunc: newBufferFunc,
|
||||
contentLength: -1,
|
||||
expectedWrite: true,
|
||||
},
|
||||
{
|
||||
name: "buffer, chunked",
|
||||
method: "PUT",
|
||||
bodyFunc: newBufferFunc,
|
||||
transferEncoding: []string{"chunked"},
|
||||
expectedWrite: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
body, cleanup, err := tc.bodyFunc()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
mw := &mockTransferWriter{}
|
||||
tw := &transferWriter{
|
||||
Body: body,
|
||||
ContentLength: tc.contentLength,
|
||||
TransferEncoding: tc.transferEncoding,
|
||||
}
|
||||
|
||||
if err := tw.writeBody(mw); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if tc.expectedReader != nil {
|
||||
if mw.CalledReader == nil {
|
||||
t.Fatal("did not call ReadFrom")
|
||||
}
|
||||
|
||||
var actualReader reflect.Type
|
||||
lr, ok := mw.CalledReader.(*io.LimitedReader)
|
||||
if ok && tc.limitedReader {
|
||||
actualReader = reflect.TypeOf(lr.R)
|
||||
} else {
|
||||
actualReader = reflect.TypeOf(mw.CalledReader)
|
||||
}
|
||||
|
||||
if tc.expectedReader != actualReader {
|
||||
t.Fatalf("got reader %T want %T", actualReader, tc.expectedReader)
|
||||
}
|
||||
}
|
||||
|
||||
if tc.expectedWrite && !mw.WriteCalled {
|
||||
t.Fatal("did not invoke Write")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue