-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement receive window auto tuning #53
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
// streams maps a stream id to a stream, and inflight has an entry | ||
// for any outgoing stream that has not yet been established. Both are | ||
// protected by streamLock. | ||
|
@@ -129,6 +131,7 @@ func newSession(config *Config, conn net.Conn, client bool, readBuf int) *Sessio | |
} | ||
go s.recv() | ||
go s.send() | ||
go s.measureRTT() | ||
return s | ||
} | ||
|
||
|
@@ -291,6 +294,19 @@ func (s *Session) goAway(reason uint32) header { | |
return hdr | ||
} | ||
|
||
func (s *Session) measureRTT() { | ||
rtt, err := s.Ping() | ||
if err != nil { | ||
return | ||
} | ||
atomic.StoreInt64(&s.rtt, rtt.Nanoseconds()) | ||
} | ||
|
||
// 0 if we don't yet have a measurement | ||
func (s *Session) getRTT() time.Duration { | ||
return time.Duration(atomic.LoadInt64(&s.rtt)) | ||
} | ||
|
||
// Ping is used to measure the RTT response time | ||
func (s *Session) Ping() (dur time.Duration, err error) { | ||
// Prepare a ping. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ func newStream(session *Session, id uint32, state streamState) *Stream { | |
sendWindow: initialStreamWindow, | ||
readDeadline: makePipeDeadline(), | ||
writeDeadline: makePipeDeadline(), | ||
recvBuf: newSegmentedBuffer(initialStreamWindow), | ||
recvBuf: newSegmentedBuffer(initialStreamWindow, session.config.MaxStreamWindowSize, session.getRTT), | ||
recvNotifyCh: make(chan struct{}, 1), | ||
sendNotifyCh: make(chan struct{}, 1), | ||
} | ||
|
@@ -202,11 +202,8 @@ func (s *Stream) sendWindowUpdate() error { | |
// Determine the flags if any | ||
flags := s.sendFlags() | ||
|
||
// Determine the delta update | ||
max := s.session.config.MaxStreamWindowSize | ||
|
||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll have to watch this (have had some trouble with |
||
if !needed { | ||
return nil | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,9 @@ package yamux | |
import ( | ||
"fmt" | ||
"io" | ||
"math" | ||
"sync" | ||
"time" | ||
|
||
pool "github.com/libp2p/go-buffer-pool" | ||
) | ||
|
@@ -65,18 +67,30 @@ func min(values ...uint32) uint32 { | |
// < len (5) > < cap (5) > | ||
// | ||
type segmentedBuffer struct { | ||
cap uint32 | ||
len uint32 | ||
bm sync.Mutex | ||
cap uint32 | ||
len uint32 | ||
windowSize uint32 | ||
maxWindowSize uint32 | ||
bm sync.Mutex | ||
// read position in b[0]. | ||
// We must not reslice any of the buffers in b, as we need to put them back into the pool. | ||
readPos int | ||
b [][]byte | ||
|
||
epochStart time.Time | ||
getRTT func() time.Duration | ||
} | ||
|
||
// NewSegmentedBuffer allocates a ring buffer. | ||
func newSegmentedBuffer(initialCapacity uint32) segmentedBuffer { | ||
return segmentedBuffer{cap: initialCapacity, b: make([][]byte, 0)} | ||
func newSegmentedBuffer(initialCapacity, maxWindowSize uint32, getRTT func() time.Duration) segmentedBuffer { | ||
return segmentedBuffer{ | ||
cap: initialCapacity, | ||
windowSize: initialCapacity, | ||
maxWindowSize: maxWindowSize, | ||
b: make([][]byte, 0), | ||
epochStart: time.Now(), | ||
getRTT: getRTT, | ||
} | ||
} | ||
|
||
// Len is the amount of data in the receive buffer. | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic doesn't really belong in this type. Can't we compute the target window outside this type and pass it in? |
||
grow, delta := s.growTo(force, now) | ||
if grow { | ||
s.epochStart = now | ||
} | ||
return grow, delta | ||
} | ||
|
||
func (s *segmentedBuffer) growTo(force bool, now time.Time) (bool, uint32) { | ||
s.bm.Lock() | ||
defer s.bm.Unlock() | ||
|
||
currentWindow := s.cap + s.len | ||
if currentWindow >= max { | ||
if currentWindow >= s.windowSize { | ||
return force, 0 | ||
} | ||
delta := max - currentWindow | ||
delta := s.windowSize - currentWindow | ||
|
||
if delta < (max/2) && !force { | ||
if delta < (s.windowSize/2) && !force { | ||
return false, 0 | ||
} | ||
|
||
if rtt := s.getRTT(); rtt > 0 && now.Sub(s.epochStart) < 2*rtt { | ||
if s.windowSize > math.MaxUint32/2 { | ||
s.windowSize = min(math.MaxUint32, s.maxWindowSize) | ||
} else { | ||
s.windowSize = min(s.windowSize*2, s.maxWindowSize) | ||
} | ||
delta = s.windowSize - currentWindow | ||
} | ||
|
||
s.cap += delta | ||
return true, delta | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider making this configurable?