Implementation of the Ack Frequency draft for delayed ack #2136
Conversation
This file contains hidden or 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
…ENCY frame and TP min_ack_delay
The Ack Frequency draft mixes milliseconds and microseconds for this parameter so it's safer to use a Duration internally.
…ated max_ack_delay
…ring_threshold from the latest draft (11)
…s_ack_freq_received
|
To add a bit more context, @mactul was a summer intern at UNamur, and we aimed at analyzing reasons for the low performance of Curl's usage of HTTP/3 and quiche displayed in @icing's results (https://github.com/icing/blog/blob/main/curl-h3-performance.md). We have seen several corners to cut. In quiche, support of delayed acknowledgement would help Curl's client reducing the cost of sendmsg() sending acknowledgments. The Ack frequency draft isn't trivial to implement, and many threshold values are left to the implementer to choose. These may need to be tweaked from further experiments that may involve real world setups. I wonder if you could help us here finishing this PR :-) Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Hello,
While creating flamegraphs of cURL in HTTP/3 with quiche, I noticed that in certain situations, processing acknowledgements took as long as processing the incoming stream. So I wanted to explore the changes that implementing the draft for delayed ACKs would bring.
I couldn't find anything in the quiche code to delay ACKs, except for commit b3e2bb4.
So I'm offering my contribution to fill this gap.
I made sure that as long as the extension is disabled (min_ack_delay parameter not specified), quiche's behaviour remains unchanged.
The extension works well, the tests I have done seem to show that the acks are well delayed and, above all, I observe a difference in the cURL flamegraph in which the impact of sendmsg is greatly reduced.
Unfortunately, the overall performance is not up to par.
The acks sometimes seem to be delayed too much, which triggers a flow control response. Ultimately, downloading 2GB of data sometimes takes 10% longer with the extension enabled. I assume it should be possible to fix my code to resolve this issue, which is why I am opening this PR as a draft.