Skip to content

Commit

Permalink
fix: establish RTT at start of connection
Browse files Browse the repository at this point in the history
A connection ping is required to establish the RTT - as implemented
the ping only happens during the keep alive loop that doesn't kick
in until after 30s by default.

The change is to ping immediately on sinking a source - this means
we are more likely to increase the send window on the remote which
increases the amount of data they send and as such, throughput.
  • Loading branch information
achingbrain committed Nov 13, 2023
1 parent 1ea0b6a commit 7ca199c
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
5 changes: 4 additions & 1 deletion src/muxer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,16 @@ export class YamuxMuxer implements StreamMuxer {
this.nextStreamID = this.client ? 1 : 2

this.nextPingID = 0
this.rtt = 0
this.rtt = -1

this.log?.trace('muxer created')

if (this.config.enableKeepAlive) {
this.keepAliveLoop().catch(e => this.log?.error('keepalive error: %s', e))
}

// send an initial ping to establish RTT
this.ping().catch(e => this.log?.error('ping error: %s', e))
}

get streams (): YamuxStream[] {
Expand Down
2 changes: 1 addition & 1 deletion src/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export class YamuxStream extends AbstractStream {
// then we (up to) double the recvWindow
const now = Date.now()
const rtt = this.getRTT()
if (flags === 0 && rtt > 0 && now - this.epochStart < rtt * 4) {
if (flags === 0 && rtt > -1 && now - this.epochStart < rtt * 4) {
// we've already validated that maxStreamWindowSize can't be more than MAX_UINT32
this.recvWindow = Math.min(this.recvWindow * 2, this.config.maxStreamWindowSize)
}
Expand Down
8 changes: 4 additions & 4 deletions test/stream.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ describe('stream', () => {
// the window capacities should have refilled via window updates as received data was consumed

// eslint-disable-next-line @typescript-eslint/dot-notation
expect(c1['sendWindowCapacity']).to.equal(defaultConfig.initialStreamWindowSize)
expect(c1['sendWindowCapacity']).to.be.gte(defaultConfig.initialStreamWindowSize)
// eslint-disable-next-line @typescript-eslint/dot-notation
expect(s1['recvWindowCapacity']).to.equal(defaultConfig.initialStreamWindowSize)
expect(s1['recvWindowCapacity']).to.be.gte(defaultConfig.initialStreamWindowSize)
})

it('test send data - large', async () => {
Expand All @@ -73,9 +73,9 @@ describe('stream', () => {
// the window capacities should have refilled via window updates as received data was consumed

// eslint-disable-next-line @typescript-eslint/dot-notation
expect(c1['sendWindowCapacity']).to.equal(defaultConfig.initialStreamWindowSize)
expect(c1['sendWindowCapacity']).to.be.gte(defaultConfig.initialStreamWindowSize)
// eslint-disable-next-line @typescript-eslint/dot-notation
expect(s1['recvWindowCapacity']).to.equal(defaultConfig.initialStreamWindowSize)
expect(s1['recvWindowCapacity']).to.be.gte(defaultConfig.initialStreamWindowSize)
})

it('test send data - large with increasing recv window size', async () => {
Expand Down

0 comments on commit 7ca199c

Please sign in to comment.