implement receive window auto tuning #53
Conversation
(I didn't look to see if that flakiness was already present.) It looks like the test may need updating as the dynamic window size can increase the buffer more than the test expects. |
|
@willscott Not really happy with my fix (or the test case as a whole, for that matter), but at least it shouldn't be flaky any more. |
marten-seemann
force-pushed
the
rcv-window-tuning
branch
from
3c9a48c
to
824fc1c
Compare
| @@ -50,6 +50,8 @@ type Session struct { | |||
| pingID uint32 | |||
| activePing *ping | |||
|
|
|||
| rtt int64 // to be accessed atomically, in nanoseconds | |||
There was a problem hiding this comment.
This needs to be at the very top for alignment (really, the 32bit atomic ints don't need to be there, but it's nice to do that anyways.
At the moment, this may technically be fine depending on how go packs the struct, but I wouldn't count on it.
| @@ -114,7 +114,8 @@ const ( | |||
|
|
|||
| const ( | |||
| // initialStreamWindow is the initial stream window size | |||
| initialStreamWindow uint32 = 256 * 1024 | |||
| initialStreamWindow uint32 = 64 * 1024 | |||
There was a problem hiding this comment.
consider making this configurable?
| // Update our window | ||
| needed, delta := s.recvBuf.GrowTo(max, flags != 0) | ||
| needed, delta := s.recvBuf.GrowTo(flags != 0, time.Now()) |
There was a problem hiding this comment.
We'll have to watch this (have had some trouble with time.Now() being slow in the past but I think go has fixed those issues).
| @@ -88,20 +102,37 @@ func (s *segmentedBuffer) Len() uint32 { | |||
|
|
|||
| // If the space to write into + current buffer size has grown to half of the window size, | |||
| // grow up to that max size, and indicate how much additional space was reserved. | |||
| func (s *segmentedBuffer) GrowTo(max uint32, force bool) (bool, uint32) { | |||
| func (s *segmentedBuffer) GrowTo(force bool, now time.Time) (bool, uint32) { | |||
There was a problem hiding this comment.
This logic doesn't really belong in this type. Can't we compute the target window outside this type and pass it in?
|
@marten-seemann what's the status of this PR? |
|
@aschmahmann It's in the very unfortunate status of "submitted PR, received review, and totally forgot about it". Sorry for that! I'll clean it up today. |
Fixes #9.
Based on the algorithm used in QUIC: https://docs.google.com/document/d/1F2YfdDXKpy20WVKJueEf4abn_LVZHhMUMS5gX6Pgjl4/edit#heading=h.hcm2y5x4qmqt.
What we do is compare the frequency we're updating the receive window. Updates happen when we've consumed more than half the available. If we're updating more often than once every 2 RTTs, we double the window size (up to a configurable maximum window size).
Unfortunately, we don't have a good way to get an up-to-date RTT measurement here. Instead, we use the ping function to generate an estimate right when we initialize yamux, and use that estimate for the rest of the connection.