Skip to content

chore: check-in Cargo.lock#2208

Merged
mxinden merged 5 commits into
mozilla:mainfrom
mxinden:cargo-lock
Nov 25, 2024
Merged

chore: check-in Cargo.lock#2208
mxinden merged 5 commits into
mozilla:mainfrom
mxinden:cargo-lock

Conversation

@mxinden
Copy link
Copy Markdown
Member

@mxinden mxinden commented Oct 28, 2024

This commit checks the Cargo.lock file into git.

Version controlling Cargo.lock makes e.g. our CI builds more reproducible, where two consecutive CI runs on the same commit use the same set of dependencies, even if a compatible update of a dependency was published in between the two runs.

This is also helpful when cutting patch releases of old Neqo versions, where dependencies since shipped a breaking change in a patch version, e.g. a MSRV update. See for example pinned dependencies in a recent Neqo patch release to the Neqo v0.6 family.

mxinden@66e60f3

While previously the recommendation by the cargo team was for libraries to not check in their Cargo.lock, this recommendation has since been replaced by "do what is best for the project".

https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html


I generated a fresh Cargo.lock, i.e. ran rm Cargo.lock && cargo check && git add Cargo.lock.

We could as well base our Cargo.lock on mozilla-central's Cargo.lock. Though I can not think of a simple way to then keep the two in sync going forward.

Dependabot will attempt to keep the Cargo.lock up-to-date, in other words will open many pull requests updating patch versions. I don't know how to prevent this. Unfortunately, the Dependabot version-strategy parameter is not supported for the cargo ecosystem.

What do folks think?

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.40%. Comparing base (f3d0191) to head (c6bd67c).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2208      +/-   ##
==========================================
+ Coverage   95.37%   95.40%   +0.02%     
==========================================
  Files         112      112              
  Lines       36569    36569              
==========================================
+ Hits        34879    34888       +9     
+ Misses       1690     1681       -9     

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


🚨 Try these New Features:

@mxinden mxinden marked this pull request as ready for review October 28, 2024 19:04
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 28, 2024

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

mxinden added a commit to mxinden/neqo that referenced this pull request Oct 30, 2024
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`.
@larseggert
Copy link
Copy Markdown
Collaborator

Don't have a strong opinion here due to limited experience maintaining Rust libs. Do we think this is "best for the project"?

github-merge-queue Bot pushed a commit that referenced this pull request Nov 1, 2024
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 #2208 lands, future compatible
version updates would touch the `Cargo.lock` file, not `Cargo.toml`.

Co-authored-by: Lars Eggert <lars@eggert.org>
@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 5, 2024

@martinthomson objections?

Would catch things like #2219 early by making the url, idna and arbitrary version upgrades explicit, breaking the upgrade pull request only, not our entire CI.

@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 19, 2024

Friendly ping @martinthomson.

In case there are no objections, I will update the pull request before merge.

@martinthomson
Copy link
Copy Markdown
Member

No objections, though dependabot spam seems like the real cost. If it gets annoying, we can back it out.

@mxinden mxinden mentioned this pull request Nov 19, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 19, 2024

Benchmark results

Performance differences relative to f3d0191.

coalesce_acked_from_zero 1+1 entries: 💔 Performance has regressed.
       time:   [99.584 ns 99.926 ns 100.27 ns]
       change: [+11.813% +12.462% +13.107%] (p = 0.00 < 0.05)

Found 13 outliers among 100 measurements (13.00%)
4 (4.00%) high mild
9 (9.00%) high severe

coalesce_acked_from_zero 3+1 entries: 💔 Performance has regressed.
       time:   [117.63 ns 117.95 ns 118.30 ns]
       change: [+18.148% +19.086% +19.991%] (p = 0.00 < 0.05)

Found 17 outliers among 100 measurements (17.00%)
1 (1.00%) low severe
2 (2.00%) low mild
1 (1.00%) high mild
13 (13.00%) high severe

coalesce_acked_from_zero 10+1 entries: 💔 Performance has regressed.
       time:   [117.29 ns 117.78 ns 118.35 ns]
       change: [+18.795% +19.275% +19.819%] (p = 0.00 < 0.05)

Found 18 outliers among 100 measurements (18.00%)
5 (5.00%) low severe
3 (3.00%) low mild
3 (3.00%) high mild
7 (7.00%) high severe

coalesce_acked_from_zero 1000+1 entries: 💔 Performance has regressed.
       time:   [98.212 ns 101.89 ns 110.21 ns]
       change: [+19.574% +24.253% +34.244%] (p = 0.00 < 0.05)

Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [111.77 ms 111.83 ms 111.88 ms]
       change: [+0.4140% +0.4866% +0.5602%] (p = 0.00 < 0.05)

Found 19 outliers among 100 measurements (19.00%)
1 (1.00%) low severe
6 (6.00%) low mild
10 (10.00%) high mild
2 (2.00%) high severe

transfer/pacing-false/varying-seeds: No change in performance detected.
       time:   [26.888 ms 27.973 ms 29.085 ms]
       change: [-3.6318% +1.4463% +6.8814%] (p = 0.60 > 0.05)

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

transfer/pacing-true/varying-seeds: No change in performance detected.
       time:   [35.408 ms 37.102 ms 38.796 ms]
       change: [-3.0243% +3.8076% +10.882%] (p = 0.27 > 0.05)
transfer/pacing-false/same-seed: No change in performance detected.
       time:   [24.645 ms 25.338 ms 26.040 ms]
       change: [-7.5445% -3.5393% +0.5740%] (p = 0.11 > 0.05)
transfer/pacing-true/same-seed: No change in performance detected.
       time:   [41.193 ms 43.375 ms 45.618 ms]
       change: [-4.9679% +1.9588% +9.2446%] (p = 0.58 > 0.05)

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

1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: Change within noise threshold.
       time:   [905.29 ms 915.06 ms 925.22 ms]
       thrpt:  [108.08 MiB/s 109.28 MiB/s 110.46 MiB/s]
change:
       time:   [-3.2264% -1.7583% -0.2723%] (p = 0.02 < 0.05)
       thrpt:  [+0.2730% +1.7898% +3.3339%]
1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: No change in performance detected.
       time:   [316.16 ms 319.33 ms 322.59 ms]
       thrpt:  [30.999 Kelem/s 31.316 Kelem/s 31.630 Kelem/s]
change:
       time:   [-1.8599% -0.5220% +0.8649%] (p = 0.46 > 0.05)
       thrpt:  [-0.8575% +0.5247% +1.8952%]

Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild

1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.
       time:   [33.741 ms 33.901 ms 34.066 ms]
       thrpt:  [29.355  elem/s 29.498  elem/s 29.638  elem/s]
change:
       time:   [-0.0963% +0.6513% +1.3936%] (p = 0.10 > 0.05)
       thrpt:  [-1.3745% -0.6471% +0.0964%]

Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild

1-conn/1-100mb-resp/mtu-1504 (aka. Upload)/client: No change in performance detected.
       time:   [1.6178 s 1.6360 s 1.6550 s]
       thrpt:  [60.425 MiB/s 61.125 MiB/s 61.812 MiB/s]
change:
       time:   [-1.0012% +0.7766% +2.6995%] (p = 0.40 > 0.05)
       thrpt:  [-2.6286% -0.7706% +1.0113%]

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

Client/server transfer results

Transfer of 33554432 bytes over loopback.

Client Server CC Pacing MTU Mean [ms] Min [ms] Max [ms]
gquiche gquiche 1504 530.0 ± 26.5 504.8 572.9
neqo gquiche reno on 1504 795.7 ± 60.7 754.3 937.0
neqo gquiche reno 1504 801.1 ± 69.5 750.7 963.7
neqo gquiche cubic on 1504 778.2 ± 74.2 740.6 986.3
neqo gquiche cubic 1504 781.4 ± 56.3 746.9 933.4
msquic msquic 1504 209.1 ± 122.2 99.9 572.5
neqo msquic reno on 1504 214.6 ± 13.6 198.4 240.7
neqo msquic reno 1504 218.3 ± 16.8 200.1 252.4
neqo msquic cubic on 1504 281.8 ± 81.9 215.7 426.1
neqo msquic cubic 1504 283.6 ± 110.2 213.4 583.1
gquiche neqo reno on 1504 745.0 ± 103.1 604.5 928.4
gquiche neqo reno 1504 811.4 ± 148.0 617.7 1082.2
gquiche neqo cubic on 1504 772.0 ± 101.5 616.6 935.7
gquiche neqo cubic 1504 758.6 ± 87.8 643.7 875.3
msquic neqo reno on 1504 755.9 ± 85.1 685.7 923.0
msquic neqo reno 1504 694.8 ± 10.4 680.9 710.1
msquic neqo cubic on 1504 720.9 ± 28.4 690.7 774.3
msquic neqo cubic 1504 763.9 ± 107.2 689.2 983.6
neqo neqo reno on 1504 659.9 ± 36.6 640.1 762.1
neqo neqo reno 1504 697.5 ± 93.2 592.1 923.8
neqo neqo cubic on 1504 645.4 ± 31.0 620.3 725.3
neqo neqo cubic 1504 655.6 ± 99.3 513.2 839.6

⬇️ Download logs

@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 19, 2024

though dependabot spam seems like the real cost

I did some more digging. Setting the open-pull-requests-limit to 0 disables all updates excluding security updates. In my eyes, that is exactly what we want.

This option has no impact on security updates, which have a separate, internal limit of ten open pull requests.

https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#open-pull-requests-limit

Yes, we won't get any updates for non-security releases. That said, we have to coordinate those with mozilla-central anyways. Might as well do it manually before a release.


I updated the pull request:


Depends on #2240 for now.

This commit checks the `Cargo.lock` file into git.

Version controlling `Cargo.lock` makes e.g. our CI builds more reproducible,
where two consecutive CI runs on the same commit use the same set of
dependencies, even if a compatible update of a dependency was published in
between the two runs.

This is also helpful when cutting patch releases of old Neqo versions, where
dependencies since shipped a breaking change in a patch version, e.g. a MSRV
update. See for example pinned dependencies in a recent Neqo patch release to
the Neqo v0.6 family.

66e60f3

While previously the recommendation by the cargo team was for libraries to not
check in their `Cargo.lock`, this recommendation has since been replaced by "do
what is best for the project".

https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 19, 2024

Dependency Review CI failure will be fixed with https://bugzilla.mozilla.org/show_bug.cgi?id=1932137.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 20, 2024
@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 21, 2024

@mxinden mxinden enabled auto-merge November 22, 2024 10:55
@mxinden mxinden added this pull request to the merge queue Nov 22, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 22, 2024
@mxinden
Copy link
Copy Markdown
Member Author

mxinden commented Nov 24, 2024

@mxinden mxinden added this pull request to the merge queue Nov 25, 2024
Merged via the queue into mozilla:main with commit 2fb1a3b Nov 25, 2024
@mxinden mxinden deleted the cargo-lock branch November 25, 2024 11:30
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Nov 26, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.3.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: afdd9e4681c18d09d2c124c1af5033cbd49acf90
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Nov 27, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.3.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: afdd9e4681c18d09d2c124c1af5033cbd49acf90
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Nov 27, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.3.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: afdd9e4681c18d09d2c124c1af5033cbd49acf90
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 27, 2024
mxinden added a commit to mxinden/mtu that referenced this pull request Nov 29, 2024
mxinden added a commit to mxinden/mtu that referenced this pull request Nov 29, 2024
mxinden added a commit to mxinden/mtu that referenced this pull request Nov 29, 2024
mxinden added a commit to mxinden/mtu that referenced this pull request Nov 29, 2024
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 1, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.4.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: ab95d9361f380b29216548f78ce04b1827754c58
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 1, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.4.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: ab95d9361f380b29216548f78ce04b1827754c58
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 1, 2024
…-reviewers

See `url` GitHub release https://github.com/servo/rust-url/releases/tag/v2.5.4.

Unblocks mozilla/neqo#2208.

Differential Revision: https://phabricator.services.mozilla.com/D229493

UltraBlame original commit: ab95d9361f380b29216548f78ce04b1827754c58
larseggert added a commit to mozilla/mtu that referenced this pull request Dec 2, 2024
* chore: check-in Cargo.lock

See same change in Neqo mozilla/neqo#2208.

* Include mozbuild and mtu 0.2.3

---------

Co-authored-by: Lars Eggert <lars@eggert.org>
jwidar pushed a commit to jwidar/LatencyZeroGithub that referenced this pull request Sep 16, 2025
jwidar pushed a commit to jwidar/LatencyZeroGithub that referenced this pull request Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants