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.

Sign up for GitHub

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

Jump to bottom

feat(bin): use quinn-udp crates.io release instead of git ref #1899

Merged
merged 2 commits into from
May 13, 2024
Merged

feat(bin): use quinn-udp crates.io release instead of git ref #1899

merged 2 commits into from
May 13, 2024

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented May 13, 2024

neqo-bin has been importing quinn-udp as a git reference, in order to include quinn-rs/quinn#1765. The quinn project has since released quinn-udp v0.5.0.

This commit upgrades neqo-bin to use quinn-udp v0.5.0.

quinn-udp now takes a data reference (&[u8]) instead of owned data (bytes::Bytes) on its send path, thus no longer requiring neqo-bin to convert, but simply pass a reference. See
quinn-rs/quinn#1729 (comment) for details.

quinn-udp has dropped sendmmsg support in the v0.5.0 release (quinn-rs/quinn@ee08826). neqo-bin does not (yet) use sendmmsg. This might change in the future (#1693).

@mxinden
feat(bin): use quinn-udp crates.io release instead of git ref
1590a93 
`neqo-bin` has been importing `quinn-udp` as a git reference, in order to
include quinn-rs/quinn#1765. The quinn project has since
released `quinn-udp` `v0.5.0`.

This commit upgrades `neqo-bin` to use `quinn-udp` `v0.5.0`.

`quinn-udp` now takes a data reference (`&[u8]`) instead of owned
data (`bytes::Bytes`) on its send path, thus no longer requiring `neqo-bin` to
convert, but simply pass a reference. See
quinn-rs/quinn#1729 (comment) for details.

`quinn-udp` has dropped `sendmmsg` support in the `v0.5.0`
release (quinn-rs/quinn@ee08826).
`neqo-bin` does not (yet) use `sendmmsg`. This might change in the
future (mozilla#1693).
larseggert
larseggert approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@larseggert larseggert left a comment

Choose a reason for hiding this comment

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

See comment, otherwise LGTM.

@larseggert
Copy link
Collaborator

I'm not loving the way in which sendmmsg/recvmmsg support was removed from quinn-udp, i.e., not really based on a performance analysis/measurement.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 13, 2024 •
edited
Loading

Failed Interop Tests

None 🎉

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

@github-actions GitHub Actions
Copy link

github-actions bot commented May 13, 2024 •
edited
Loading

Benchmark results

Performance differences relative to d7f33e2.

  • drain a timer quickly time: [323.02 ns 330.35 ns 337.10 ns]
    change: [+2.6482% +4.9188% +7.1812%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 1+1 entries
    time: [194.57 ns 195.02 ns 195.54 ns]
    change: [-0.8916% -0.5014% -0.1078%] (p = 0.01 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 3+1 entries
    time: [236.04 ns 236.53 ns 237.08 ns]
    change: [-0.8796% -0.4409% +0.0127%] (p = 0.05 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 10+1 entries
    time: [236.05 ns 236.96 ns 237.99 ns]
    change: [-2.6036% -0.8715% +0.2520%] (p = 0.32 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1000+1 entries
    time: [218.20 ns 218.40 ns 218.63 ns]
    change: [-0.2125% +0.4180% +1.0987%] (p = 0.21 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [120.23 ms 120.36 ms 120.58 ms]
    change: [+0.8976% +1.1442% +1.3888%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [120.64 ms 120.91 ms 121.17 ms]
    thrpt: [33.012 MiB/s 33.083 MiB/s 33.156 MiB/s]
    change:
    time: [+1.0708% +1.4074% +1.7532%] (p = 0.00 < 0.05)
    thrpt: [-1.7229% -1.3878% -1.0595%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [120.61 ms 120.86 ms 121.12 ms]
    thrpt: [33.024 MiB/s 33.095 MiB/s 33.165 MiB/s]
    change:
    time: [+1.0935% +1.3988% +1.6876%] (p = 0.00 < 0.05)
    thrpt: [-1.6596% -1.3795% -1.0817%]
    Change within noise threshold.

  • 1-conn/1-100mb-resp (aka. Download)/client
    time: [1.1036 s 1.1077 s 1.1121 s]
    thrpt: [89.920 MiB/s 90.273 MiB/s 90.613 MiB/s]
    change:
    time: [-2.4762% -1.8674% -1.2561%] (p = 0.00 < 0.05)
    thrpt: [+1.2721% +1.9029% +2.5391%]
    💚 Performance has improved.

  • 1-conn/10_000-parallel-1b-resp (aka. RPS)/client
    time: [427.18 ms 429.39 ms 431.61 ms]
    thrpt: [23.169 Kelem/s 23.289 Kelem/s 23.410 Kelem/s]
    change:
    time: [-0.5162% +0.2451% +1.0057%] (p = 0.53 > 0.05)
    thrpt: [-0.9957% -0.2445% +0.5189%]
    No change in performance detected.

  • 1-conn/1-1b-resp (aka. HPS)/client
    time: [49.717 ms 49.844 ms 50.003 ms]
    thrpt: [19.999 elem/s 20.062 elem/s 20.114 elem/s]
    change:
    time: [-0.5913% +0.0343% +0.6076%] (p = 0.91 > 0.05)
    thrpt: [-0.6039% -0.0343% +0.5948%]
    No change in performance detected.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 538.2 ± 294.8 341.9 1353.4 1.00
neqo msquic reno on 783.3 ± 17.6 764.0 822.7 1.00
neqo msquic reno 838.0 ± 150.9 731.3 1242.9 1.00
neqo msquic cubic on 821.5 ± 53.3 766.6 951.9 1.00
neqo msquic cubic 864.6 ± 95.9 789.6 1089.0 1.00
msquic neqo reno on 5330.0 ± 232.7 4971.5 5672.4 1.00
msquic neqo reno 5241.1 ± 334.9 4831.0 6042.9 1.00
msquic neqo cubic on 5163.1 ± 301.2 4787.8 5692.9 1.00
msquic neqo cubic 4973.0 ± 183.1 4699.7 5275.8 1.00
neqo neqo reno on 4381.8 ± 300.7 3936.7 4787.3 1.00
neqo neqo reno 3924.2 ± 141.4 3665.3 4097.4 1.00
neqo neqo cubic on 4677.1 ± 265.0 4196.3 4971.1 1.00
neqo neqo cubic 4818.0 ± 119.2 4638.8 4962.7 1.00

⬇️ Download logs

@github-actions GitHub Actions
Copy link

github-actions bot commented May 13, 2024 •
edited
Loading

Firefox builds for this PR

The following builds are available for testing. Crossed-out builds did not succeed.

@larseggert larseggert enabled auto-merge May 13, 2024 15:30
@larseggert larseggert added this pull request to the merge queue May 13, 2024
Merged via the queue into mozilla:main with commit e44c472 May 13, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larseggert larseggert larseggert approved these changes

@KershawChang KershawChang Awaiting requested review from KershawChang KershawChang is a code owner

@martinthomson martinthomson Awaiting requested review from martinthomson martinthomson is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mxinden @larseggert