-
Notifications
You must be signed in to change notification settings - Fork 124
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
perf(bin/server): increase msg size and don't allocate msg per resp #1772
Conversation
Previously `neqo-server` would respond to a request by repeatedly sending a static 440 byte message (Major-General's Song). Instead of sending 440 bytes, increase the batch size to 4096 bytes. This also matches the `neqo-client` receive buffer size. https://github.com/mozilla/neqo/blob/76630a5ebb6c6b94de6a40cf3f439b9a846f6ab7/neqo-bin/src/bin/client/http3.rs#L165 Previously `ResponseData::repeat` would convert the provided `buf: &[u8]` to ` Vec<u8>`, i.e. re-allocate the buf. Instead keep a reference to the original buf, thus removing the allocation.
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.
Fix clippy
, but otherwise LGTM.
Head branch was pushed to by a user without write access
Benchmark resultsPerformance differences relative to 76630a5.
Client/server transfer resultsTransfer of 134217728 bytes over loopback.
|
There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - mozilla#1564 - mozilla#1569 - mozilla#1578 - mozilla#1581 - mozilla#1604 - mozilla#1612 - mozilla#1676 - mozilla#1692 - mozilla#1707 - mozilla#1708 - mozilla#1727 - mozilla#1753 - mozilla#1756 - mozilla#1766 - mozilla#1772 - mozilla#1786 - mozilla#1787 - mozilla#1788 - mozilla#1794 - mozilla#1806 - mozilla#1808 - mozilla#1848 - mozilla#1866 At this point, bugs in (2) are hard to fix, see e.g. mozilla#1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1).
There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - mozilla#1564 - mozilla#1569 - mozilla#1578 - mozilla#1581 - mozilla#1604 - mozilla#1612 - mozilla#1676 - mozilla#1692 - mozilla#1707 - mozilla#1708 - mozilla#1727 - mozilla#1753 - mozilla#1756 - mozilla#1766 - mozilla#1772 - mozilla#1786 - mozilla#1787 - mozilla#1788 - mozilla#1794 - mozilla#1806 - mozilla#1808 - mozilla#1848 - mozilla#1866 At this point, bugs in (2) are hard to fix, see e.g. mozilla#1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1).
* refactor(bin): introduce server/http3.rs and server/http09.rs The QUIC Interop Runner requires an http3 and http09 implementation for both client and server. The client code is already structured into an http3 and an http09 implementation since #1727. This commit does the same for the server side, i.e. splits the http3 and http09 implementation into separate Rust modules. * refactor: merge mozilla-central http3 server into neqo-bin There are two server implementations based on neqo: 1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server - http3 and http09 implementation - used for manual testing and QUIC Interop 2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs - used to test Firefox I assume one was once an exact copy of the other. Both implement their own I/O, event loop, ... Since then, the two implementations diverged significantly. Especially (1) saw a lot of improvements in recent months: - #1564 - #1569 - #1578 - #1581 - #1604 - #1612 - #1676 - #1692 - #1707 - #1708 - #1727 - #1753 - #1756 - #1766 - #1772 - #1786 - #1787 - #1788 - #1794 - #1806 - #1808 - #1848 - #1866 At this point, bugs in (2) are hard to fix, see e.g. #1801. This commit merges (2) into (1), thus removing all duplicate logic and having (2) benefit from all the recent improvements to (1). * Move firefox.rs to mozilla-central * Reduce HttpServer trait functions * Extract constructor * Remove unused deps * Remove clap color feature Nice to have. Adds multiple dependencies. Hard to justify for mozilla-central.
Previously
neqo-server
would respond to a request by repeatedly sending a static 440 byte message (Major-General's Song). Instead of sending 440 bytes, increase the batch size to 4096 bytes. This also matches theneqo-client
receive buffer size.neqo/neqo-bin/src/bin/client/http3.rs
Line 165 in 76630a5
Previously
ResponseData::repeat
would convert the providedbuf: &[u8]
toVec<u8>
, i.e. re-allocate the buf. Instead keep a reference to the original buf, thus removing the allocation.Preliminary benchmark comparison:
main
:this pull request:
Also visible in qviz where client reads MANY small (440 bytes) chunks from server:
I suggest merging #1758 first. We can then validate this pull request using the CI benchmark server.
I will take a look whether a long lived receive buffer on the client side has any impact next.
neqo/neqo-bin/src/bin/client/http3.rs
Line 165 in 76630a5