chore: update to quinn-udp v0.5.6#2209
Merged
Merged
Conversation
Currently we use `quinn-udp` `v0.5.4`. `quinn-udp` `v0.5.5` fixes [`recvmmsg` calls on Android x86](quinn-rs/quinn#1966). `quinn-udp` `v0.5.6` adds [experimental multi-message support on Apple platforms](quinn-rs/quinn#1993) and [fixes an unnecessary `windows-sys` version restriction](quinn-rs/quinn#2021). While not strictly necessary, given that our current version specification (i.e. `version = "0.5.4"`) already allows users to use Neqo with `quinn-udp` `v0.5.6`, this commit updates to `quinn-udp` `v0.5.6` anyways, thus making sure CI tests with latest version. In case mozilla#2208 lands, future compatible version updates would touch the `Cargo.lock` file, not `Cargo.toml`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2209 +/- ##
==========================================
+ Coverage 95.38% 95.39% +0.01%
==========================================
Files 112 112
Lines 36395 36447 +52
==========================================
+ Hits 34714 34768 +54
+ Misses 1681 1679 -2 ☔ View full report in Codecov by Sentry. |
mxinden
requested review from
KershawChang,
larseggert and
martinthomson
as code owners
Contributor
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
larseggert
reviewed
Oct 30, 2024
larseggert
approved these changes
Oct 30, 2024
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Collaborator
|
Merge queue CI might be failing because there is no 0.5.6? https://github.com/quinn-rs/quinn/releases (Ah, but there is a tag.) |
Member
Author
|
It is green now: https://github.com/mozilla/neqo/actions/runs/11591731031/job/32320460781?pr=2209 @larseggert I assume you took some action to fix it? I will attempt another merge through the queue.
I commented on quinn-rs/quinn#2022. That said, agreed, a tag should suffice. If a tag isn't enough, we will have to adjust our tooling, as we can't require all upstream dependencies to maintain GitHub releases. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Member
Author
|
I assume failure is due to actions/dependency-review-action#841. I will create a separate pull request to upgrade to |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Contributor
Benchmark resultsPerformance differences relative to 8e36f63. coalesce_acked_from_zero 1+1 entries: Change within noise threshold. time: [99.570 ns 99.851 ns 100.15 ns]
change: [+0.3009% +0.9034% +1.7945%] (p = 0.01 < 0.05)
coalesce_acked_from_zero 3+1 entries: Change within noise threshold. time: [118.13 ns 118.52 ns 118.94 ns]
change: [+0.4849% +0.9381% +1.3892%] (p = 0.00 < 0.05)
coalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [117.85 ns 118.41 ns 119.05 ns]
change: [-0.2016% +0.4457% +1.0273%] (p = 0.16 > 0.05)
coalesce_acked_from_zero 1000+1 entries: Change within noise threshold. time: [98.080 ns 98.174 ns 98.293 ns]
change: [+0.4679% +1.1150% +1.7792%] (p = 0.00 < 0.05)
RxStreamOrderer::inbound_frame(): Change within noise threshold. time: [111.89 ms 111.94 ms 112.00 ms]
change: [+0.1771% +0.2439% +0.3111%] (p = 0.00 < 0.05)
transfer/pacing-false/varying-seeds: Change within noise threshold. time: [26.091 ms 27.144 ms 28.208 ms]
change: [-11.049% -6.0257% -0.5444%] (p = 0.03 < 0.05)
transfer/pacing-true/varying-seeds: No change in performance detected. time: [34.497 ms 36.222 ms 37.958 ms]
change: [-10.467% -4.4680% +2.0692%] (p = 0.18 > 0.05)
transfer/pacing-false/same-seed: Change within noise threshold. time: [25.992 ms 26.735 ms 27.473 ms]
change: [-8.4494% -4.5039% -0.3212%] (p = 0.04 < 0.05)
transfer/pacing-true/same-seed: No change in performance detected. time: [41.584 ms 43.547 ms 45.538 ms]
change: [-11.755% -5.2422% +1.5231%] (p = 0.12 > 0.05)
1-conn/1-100mb-resp/mtu-1500 (aka. Download)/client: 💔 Performance has regressed. time: [927.93 ms 936.79 ms 945.58 ms]
thrpt: [105.76 MiB/s 106.75 MiB/s 107.77 MiB/s]
change:
time: [+2.8046% +4.2253% +5.5879%] (p = 0.00 < 0.05)
thrpt: [-5.2922% -4.0540% -2.7281%]
1-conn/10_000-parallel-1b-resp/mtu-1500 (aka. RPS)/client: No change in performance detected. time: [318.57 ms 321.62 ms 324.69 ms]
thrpt: [30.799 Kelem/s 31.093 Kelem/s 31.391 Kelem/s]
change:
time: [-0.9453% +0.4601% +1.8350%] (p = 0.53 > 0.05)
thrpt: [-1.8019% -0.4580% +0.9543%]
1-conn/1-1b-resp/mtu-1500 (aka. HPS)/client: Change within noise threshold. time: [33.546 ms 33.756 ms 33.980 ms]
thrpt: [29.429 elem/s 29.625 elem/s 29.810 elem/s]
change:
time: [-1.8603% -0.9702% -0.0183%] (p = 0.04 < 0.05)
thrpt: [+0.0183% +0.9797% +1.8956%]
1-conn/1-100mb-resp/mtu-1500 (aka. Upload)/client: No change in performance detected. time: [1.7579 s 1.7740 s 1.7902 s]
thrpt: [55.860 MiB/s 56.370 MiB/s 56.885 MiB/s]
change:
time: [-1.5490% -0.2313% +1.2148%] (p = 0.74 > 0.05)
thrpt: [-1.2002% +0.2318% +1.5734%]
1-conn/1-100mb-resp/mtu-65536 (aka. Download)/client: 💔 Performance has regressed. time: [112.12 ms 112.39 ms 112.65 ms]
thrpt: [887.67 MiB/s 889.79 MiB/s 891.94 MiB/s]
change:
time: [+1.9913% +2.3486% +2.7034%] (p = 0.00 < 0.05)
thrpt: [-2.6322% -2.2947% -1.9524%]
1-conn/10_000-parallel-1b-resp/mtu-65536 (aka. RPS)/client: No change in performance detected. time: [314.03 ms 317.22 ms 320.37 ms]
thrpt: [31.214 Kelem/s 31.524 Kelem/s 31.844 Kelem/s]
change:
time: [-1.3568% +0.1410% +1.6126%] (p = 0.86 > 0.05)
thrpt: [-1.5870% -0.1408% +1.3755%]
1-conn/1-1b-resp/mtu-65536 (aka. HPS)/client: No change in performance detected. time: [34.005 ms 34.171 ms 34.354 ms]
thrpt: [29.109 elem/s 29.265 elem/s 29.408 elem/s]
change:
time: [-1.1471% -0.3312% +0.4934%] (p = 0.42 > 0.05)
thrpt: [-0.4909% +0.3323% +1.1604%]
1-conn/1-100mb-resp/mtu-65536 (aka. Upload)/client: No change in performance detected. time: [269.70 ms 281.52 ms 294.77 ms]
thrpt: [339.25 MiB/s 355.22 MiB/s 370.78 MiB/s]
change:
time: [-19.908% -9.2796% +0.6596%] (p = 0.12 > 0.05)
thrpt: [-0.6553% +10.229% +24.857%]
Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
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
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.
Currently we use
quinn-udpv0.5.4.quinn-udpv0.5.5fixesrecvmmsgcalls on Android x86.quinn-udpv0.5.6adds experimental multi-message support on Apple platforms and fixes an unnecessarywindows-sysversion restriction.While not strictly necessary, given that our current version specification (i.e.
version = "0.5.4") already allows users to use Neqo withquinn-udpv0.5.6, this commit updates toquinn-udpv0.5.6anyways, thus making sure CI tests with latest version.In case #2208 lands, future compatible version updates would touch the
Cargo.lockfile, notCargo.toml.