Skip to content

Commit 335abf9

Browse files
committed
net/http: optimize Transport request body writes
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 writer 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 ID Ic88d4ac254f665223536fcba4d551fc32ae105b6 to properly function, as the lack of ReaderFrom implementation otherwise means that this functionality is essentially walled off. Updates #30377.
1 parent 01f34cb commit 335abf9

2 files changed

Lines changed: 40 additions & 19 deletions

File tree

src/net/http/transfer.go

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,6 @@ func (br *byteReader) Read(p []byte) (n int, err error) {
5353
return 1, io.EOF
5454
}
5555

56-
// transferBodyReader is an io.Reader that reads from tw.Body
57-
// and records any non-EOF error in tw.bodyReadError.
58-
// It is exactly 1 pointer wide to avoid allocations into interfaces.
59-
type transferBodyReader struct{ tw *transferWriter }
60-
61-
func (br transferBodyReader) Read(p []byte) (n int, err error) {
62-
n, err = br.tw.Body.Read(p)
63-
if err != nil && err != io.EOF {
64-
br.tw.bodyReadError = err
65-
}
66-
return
67-
}
68-
6956
// transferWriter inspects the fields of a user-supplied Request or Response,
7057
// sanitizes them without changing the user object and provides methods for
7158
// writing the respective header, body and trailer in wire format.
@@ -347,15 +334,18 @@ func (t *transferWriter) writeBody(w io.Writer) error {
347334
var err error
348335
var ncopy int64
349336

350-
// Write body
337+
// Write body. We "unwrap" the body first if it was wrapped in a
338+
// nopCloser. This is to ensure that we can take advantage of
339+
// OS-level optimizations in the event that the body is an
340+
// *os.File.
351341
if t.Body != nil {
352-
var body = transferBodyReader{t}
342+
var body = t.unwrapBody()
353343
if chunked(t.TransferEncoding) {
354344
if bw, ok := w.(*bufio.Writer); ok && !t.IsResponse {
355345
w = &internal.FlushAfterChunkWriter{Writer: bw}
356346
}
357347
cw := internal.NewChunkedWriter(w)
358-
_, err = io.Copy(cw, body)
348+
_, err = t.doBodyCopy(cw, body)
359349
if err == nil {
360350
err = cw.Close()
361351
}
@@ -364,14 +354,14 @@ func (t *transferWriter) writeBody(w io.Writer) error {
364354
if t.Method == "CONNECT" {
365355
dst = bufioFlushWriter{dst}
366356
}
367-
ncopy, err = io.Copy(dst, body)
357+
ncopy, err = t.doBodyCopy(dst, body)
368358
} else {
369-
ncopy, err = io.Copy(w, io.LimitReader(body, t.ContentLength))
359+
ncopy, err = t.doBodyCopy(w, io.LimitReader(body, t.ContentLength))
370360
if err != nil {
371361
return err
372362
}
373363
var nextra int64
374-
nextra, err = io.Copy(ioutil.Discard, body)
364+
nextra, err = t.doBodyCopy(ioutil.Discard, body)
375365
ncopy += nextra
376366
}
377367
if err != nil {
@@ -402,6 +392,31 @@ func (t *transferWriter) writeBody(w io.Writer) error {
402392
return err
403393
}
404394

395+
// doBodyCopy wraps a copy operation, with any resulting error also
396+
// being saved in bodyReadError.
397+
//
398+
// This function is only intended for use in writeBody.
399+
func (t *transferWriter) doBodyCopy(dst io.Writer, src io.Reader) (n int64, err error) {
400+
n, err = io.Copy(dst, src)
401+
if err != nil && err != io.EOF {
402+
t.bodyReadError = err
403+
}
404+
return
405+
}
406+
407+
// unwrapBodyReader unwraps the body's inner reader if it's a
408+
// nopCloser. This is to ensure that body writes sourced from local
409+
// files (*os.File types) are properly optimized.
410+
//
411+
// This function is only intended for use in writeBody.
412+
func (t *transferWriter) unwrapBody() io.Reader {
413+
if reflect.TypeOf(t.Body) == nopCloserType {
414+
return reflect.ValueOf(t.Body).Field(0).Interface().(io.Reader)
415+
}
416+
417+
return t.Body
418+
}
419+
405420
type transferReader struct {
406421
// Input
407422
Header Header

src/net/http/transport.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,12 @@ func (w persistConnWriter) Write(p []byte) (n int, err error) {
13751375
return
13761376
}
13771377

1378+
func (w persistConnWriter) ReadFrom(r io.Reader) (n int64, err error) {
1379+
n, err = io.Copy(w.pc.conn, r)
1380+
w.pc.nwrite += n
1381+
return
1382+
}
1383+
13781384
// connectMethod is the map key (in its String form) for keeping persistent
13791385
// TCP connections alive for subsequent HTTP requests.
13801386
//

0 commit comments

Comments
 (0)