Skip to content
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

Gorry's editorial comments #296

Merged
merged 3 commits into from
May 31, 2024
Merged

Gorry's editorial comments #296

merged 3 commits into from
May 31, 2024

Conversation

mirjak
Copy link
Contributor

@mirjak mirjak commented May 21, 2024

Thanks!

Copy link
Contributor

@gorryfair gorryfair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

Copy link
Collaborator

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion and one Question, but otherwise LG

@@ -145,7 +145,7 @@ endpoint performance in the following ways:
- Sending UDP datagrams is very CPU intensive on some platforms. A data
receiver can decrease its CPU usage by reducing the number of
acknowledgement-only packets sent. Experience shows that this
reduction can be critical for high bandwidth connections.
reduction can be critical for high-rate connections.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure what this means, but I can see why bandwidth may not be exactly correct. What about "high packet rate connections."?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packet rate is fine also

draft-ietf-quic-ack-frequency.md Outdated Show resolved Hide resolved
@ianswett ianswett merged commit 9bf1487 into main May 31, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants