974932 - spdy: large upload chunk sizes, and fewer TLS application data records

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

TLS records are processed as atomic units.. so the bigger the unit the more latency added to the first byte (because the last byte needs to arrive too).. otoh there is framing overhead added to each record and significant CPU overhead too. So there is a tension between these two approaches.

Obviously the download path needs to use small records and chunks in order to render in a streaming fashion and preserve priority. Both streaming and priority are less important on the upstream path.

This patch does 3 things:
 1] it uses a copy when necessary to coalesce the spdy frame header into the data frame - creating 1 TLS application record when there were 2

2] it increases our max spdy chunk size to 16000 from 4000. I think that's too big for the downstream path beacuse of the high degree of multiplexing and responsiveness we expect, but the tradeoff is right for the upstream path.

3] it sizes the buffer given to the input stream to a whole number of max data frames, preventing runts if the frames are filled all the way.

This does not enforce max sized records, that really depends on whatever the nsIInputStream feeding HTTP does. Indeed I regularly see smallish ones come out of an upload to youtube test even with this patch - but the patch greatly reduces the total number of records.

testing a youtube upload with this patch reduced the number of data records produced from 7700 to 1850... an optimal number would be around 1400. So it gets most of the way there.

Comment 1

[Patrick McManus [:mcmanus]]

I'm going to call this a feature and not a bug fix, and therefore only apply it to the leading edge of spdy (3.1 and http/2)

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: spdy large upload chunk sizes, and fewer TLS application data records

Comment 3

[Patrick McManus [:mcmanus]]

https://tbpl.mozilla.org/?tree=Try&rev=1b7901f07e9f

Comment 4

[u408661]

Comment on attachment 8379032 [details] [diff] [review]
spdy large upload chunk sizes, and fewer TLS application data records

Review of attachment 8379032 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/SpdyStream31.cpp
@@ +699,5 @@
>    // This function calls mSegmentReader->OnReadSegment to report the actual SPDY
>    // bytes through to the SpdySession31 and then the HttpConnection which calls
>    // the socket write function. It will accept all of the inline and stream
>    // data because of the above 'commitment' even if it has to buffer
> +  

nit: whitespace

Comment 5

[Patrick McManus [:mcmanus]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b8164a8665f6

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8164a8665f6