http/transfer wrapping request body in io.LimitReader denies io.Copy optimisations?

197 views
Skip to first unread message

Piers

unread,
May 29, 2015, 9:46:30 AM5/29/15
to golan...@googlegroups.com
Seeking some thoughts as to whether this would materially affect large HTTP uploads...

I'm using a custom io.ReadCloser which is also an io.WriterTo to implement encryption and checksumming during a large HTTP PUT.

Seems like the optimisations available to io.Copy of using WriteTo if available is not possible for HTTP request bodies with a known content length, because in http/transfer.go's transferWrite.WriteBody() method the Body is wrapped in by io.LimitReader() to an *io.LimitedReader, which does not implement the WriterTo or ReaderFrom interface, even if the underlying Body did.

   214 ncopy, err = io.Copy(w, io.LimitReader(t.Body, t.ContentLength))
   215 if err != nil {
   216 return err
   217 }


So even if you supply a Body which implements io.WriterTo, the WriteTo method is not called (except see below) - instead the transport's connection's ReadFrom ends up calling Read() on the Body in default 4096-byte buffers. I don't have any real sense myself of whether this is more or less good.

 (actually WriteTo is called by the 'extra' cleanup bit, generally a no-op, but shows that t.Body is capable of being a WriterTo up to the point above):

   218 var nextra int64
   219 nextra, err = io.Copy(ioutil.Discard, t.Body)
   220 ncopy += nextra


Piers

unread,
Jun 4, 2015, 5:03:25 AM6/4/15
to golan...@googlegroups.com
Only partially to bump this ;) I've found (I think) that with TLS each Write on the connection's io.Writer results in one TLS Record on the wire, with some fixed(?) protocol overhead. Being able to use WriteTo to send larger Writes to the TLS connection would seem to allow a few percent more throughput than you currently get with the connection's default-buffer-sized ReadFrom. 

But I'm not an expert on TLS at all - it took me couple hours of puzzling "why are 4096+53 bytes being sent on the wire for each 4096 read from my source" to even get here!

There's probably a "better way" overall that I'm not aware of.
Reply all
Reply to author
Forward
0 new messages