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

chore: Bump quinn-udp to 0.5.5 #2107

Closed
wants to merge 1 commit into from

Conversation

larseggert
Copy link
Collaborator

No description provided.

@mxinden
Copy link
Collaborator

mxinden commented Sep 13, 2024

I would like to hold back here for now. Larger context posted on Matrix:

Upgrading mozilla-central to windows-sys v0.59.0

phabricator#D220890 upgrades neqo_glue to use quinn-udp v0.5.5.

quinn-udp v0.5.5 uses windows-sys v0.59.0. All other crates in mozilla-central, 13 in total, use windows-sys v0.52.0. See each linked in this SearchFox search.

If I understand the Mozilla policy on duplicate crate versions correctly, one should strive for a single version of each crate in mozilla-central only. This makes sense to me.

Upgrading each of these 13 crates to windows-sys v0.59.0 is a significant undertaking which I assume will take weeks.

Before I start this large effort, posting here to make sure I am not misunderstanding the policy.

https://matrix.to/#/!OYZMPVBAZnIpKdnqHG:mozilla.org/$dytLNfz94wPzYCs__CnIjD3bXsyd4u-FfeZm1lE2gtg?via=mozilla.org&via=matrix.org&via=seirdy.one

The only relevant fix of v0.5.5 is quinn-rs/quinn#1947. Given the low popularity of Android x86, I don't think this is a blocker for Neqo, right?

(Note, given that it is a patch release, I can update quinn-udp in mozilla-central without updating mozilla/neqo first.)

@larseggert
Copy link
Collaborator Author

We can definitely hold back here. Thanks for the background!

@larseggert larseggert added the blocked Blocked on something else label Sep 13, 2024
Copy link

codecov bot commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.35%. Comparing base (2ca29c1) to head (bf82a5a).
Report is 73 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2107      +/-   ##
==========================================
- Coverage   95.35%   95.35%   -0.01%     
==========================================
  Files         112      112              
  Lines       36325    36325              
==========================================
- Hits        34638    34637       -1     
- Misses       1687     1688       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Failed Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Copy link

Benchmark results

Performance differences relative to 0b106c2.

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [99.262 ns 99.600 ns 99.967 ns]
       change: [-0.4834% +0.1321% +0.6536%] (p = 0.67 > 0.05)

Found 10 outliers among 100 measurements (10.00%)
6 (6.00%) high mild
4 (4.00%) high severe

coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [117.37 ns 117.77 ns 118.19 ns]
       change: [-0.0167% +0.4792% +1.0002%] (p = 0.06 > 0.05)

Found 18 outliers among 100 measurements (18.00%)
2 (2.00%) low mild
16 (16.00%) high severe

coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [116.78 ns 117.19 ns 117.70 ns]
       change: [-0.1247% +0.6974% +1.7956%] (p = 0.15 > 0.05)

Found 14 outliers among 100 measurements (14.00%)
3 (3.00%) low severe
1 (1.00%) high mild
10 (10.00%) high severe

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [97.327 ns 97.445 ns 97.582 ns]
       change: [-1.3123% -0.5102% +0.2133%] (p = 0.21 > 0.05)

Found 10 outliers among 100 measurements (10.00%)
4 (4.00%) high mild
6 (6.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [111.76 ms 111.81 ms 111.86 ms]
       change: [+0.3179% +0.3878% +0.4561%] (p = 0.00 < 0.05)

Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) low mild

transfer/pacing-false/varying-seeds: No change in performance detected.
       time:   [25.794 ms 26.883 ms 27.961 ms]
       change: [-9.0898% -4.1188% +1.1104%] (p = 0.13 > 0.05)
transfer/pacing-true/varying-seeds: No change in performance detected.
       time:   [34.263 ms 36.011 ms 37.816 ms]
       change: [-7.1292% -0.4220% +6.3358%] (p = 0.89 > 0.05)

Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild

transfer/pacing-false/same-seed: No change in performance detected.
       time:   [30.910 ms 31.775 ms 32.603 ms]
       change: [-6.3582% -2.9567% +0.3754%] (p = 0.09 > 0.05)

Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) low mild

transfer/pacing-true/same-seed: No change in performance detected.
       time:   [38.850 ms 41.687 ms 44.546 ms]
       change: [-17.188% -8.3531% +1.0388%] (p = 0.08 > 0.05)
1-conn/1-100mb-resp (aka. Download)/client: Change within noise threshold.
       time:   [113.39 ms 113.83 ms 114.27 ms]
       thrpt:  [875.14 MiB/s 878.49 MiB/s 881.91 MiB/s]
change:
       time:   [-1.5799% -1.0621% -0.5212%] (p = 0.00 < 0.05)
       thrpt:  [+0.5239% +1.0735% +1.6052%]

Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) low severe
1 (1.00%) low mild
2 (2.00%) high mild

1-conn/10_000-parallel-1b-resp (aka. RPS)/client: 💔 Performance has regressed.
       time:   [323.81 ms 326.90 ms 330.00 ms]
       thrpt:  [30.303 Kelem/s 30.590 Kelem/s 30.882 Kelem/s]
change:
       time:   [+1.1660% +2.6251% +4.2072%] (p = 0.00 < 0.05)
       thrpt:  [-4.0373% -2.5580% -1.1526%]
1-conn/1-1b-resp (aka. HPS)/client: Change within noise threshold.
       time:   [33.491 ms 33.656 ms 33.830 ms]
       thrpt:  [29.560  elem/s 29.712  elem/s 29.859  elem/s]
change:
       time:   [-2.2615% -1.4308% -0.6364%] (p = 0.00 < 0.05)
       thrpt:  [+0.6405% +1.4515% +2.3138%]

Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

Client/server transfer results

Transfer of 33554432 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 192.6 ± 94.5 94.0 355.6 1.00
neqo msquic reno on 278.1 ± 89.1 217.5 462.3 1.00
neqo msquic reno 268.9 ± 86.9 214.3 460.4 1.00
neqo msquic cubic on 222.9 ± 24.9 201.1 288.7 1.00
neqo msquic cubic 267.7 ± 93.0 203.6 442.0 1.00
msquic neqo reno on 181.8 ± 127.3 83.2 581.9 1.00
msquic neqo reno 155.0 ± 99.7 83.1 364.2 1.00
msquic neqo cubic on 127.8 ± 91.1 80.9 432.6 1.00
msquic neqo cubic 154.9 ± 100.1 81.5 388.5 1.00
neqo neqo reno on 249.8 ± 124.5 140.3 580.2 1.00
neqo neqo reno 251.6 ± 126.0 133.8 542.3 1.00
neqo neqo cubic on 217.9 ± 95.1 136.8 422.8 1.00
neqo neqo cubic 258.4 ± 126.4 141.9 582.9 1.00

⬇️ Download logs

Copy link

Firefox builds for this PR

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

@mxinden
Copy link
Collaborator

mxinden commented Oct 30, 2024

Closing here in favor of #2209.

@mxinden mxinden closed this Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked on something else
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants