Replace magic numbers with constants and define RTT calculations #149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Defines how variables such as smoothed_rtt and rttvar are calculated, as well as replacing many numbers with constants. Intends to fix #107
janaiyengar
approved these changes
Jan 13, 2017
| Constants used in loss recovery and congestion control are based on a combination | ||
| of RFCs, papers, and common practice. Some may need to be changed or negotiated | ||
| in order to better suit a variety of environments. | ||
|
|
There was a problem hiding this comment.
We should also justify these constants. I'll create an issue to do this.
|
|
||
| When an ack is received, it may acknowledge 0 or more packets. | ||
|
|
||
| Pseudocode for OnAckReceied and UpdateRtt follow: |
ianswett
added
editorial
| if is_retransmittable: | ||
| SetLossDetectionAlarm() | ||
| # TODO: Clarify the data in sent_packets. | ||
| sent_packets[packet_number] = {now} |
There was a problem hiding this comment.
why braces here, but not below (Line 341)?
| rttvar = rtt_sample / 2 | ||
| else: | ||
| rttvar = 3/4 * rttvar + 1/4 * (smoothed_rtt - rtt_sample) | ||
| smoothed_rtt = 7/8 * smoothed_rtt + 1/8 * rtt_sample |
There was a problem hiding this comment.
Oh, I see that this is just RFC 6298 directly implemented. Might pay to be more direct about that. Only the smoothed_rtt value contains the reference.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Defines how variables such as smoothed_rtt and rttvar are calculated, as well as replacing many numbers with constants.
Intends to fix #107