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

fix: Address more clippy warnings #1777

Merged
merged 3 commits into from
Mar 27, 2024
Merged

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented Mar 27, 2024

PR #1764 exports frame and packet if feature fuzzing is enabled. This apparently turns on a bunch of clippy checks that are not on by default? This PR fixes them.

One side effect is that this returns a bunch of panic!s into returning errors.

Made this separate from #1764 to reduce that PR's size.

PR mozilla#1764 exports `frame` and `packet` if feature `fuzzing` is enabled.
This apparently turns on a bunch of clippy checks that are not on by
default? This PR fixes them.

Made this separate from mozilla#1764 to reduce that PR's size.
neqo-transport/src/packet/mod.rs Outdated Show resolved Hide resolved
neqo-transport/src/packet/mod.rs Show resolved Hide resolved
@larseggert
Copy link
Collaborator Author

Anyone know how we can turn on those clippy checks by default? I had thought we cranked thinigs up to 11, but I guess not?

@larseggert larseggert added this pull request to the merge queue Mar 27, 2024
Merged via the queue into mozilla:main with commit efc4813 Mar 27, 2024
13 checks passed
@larseggert larseggert deleted the fix-more-clippy branch March 27, 2024 10:58
Copy link

Benchmark results

Performance differences relative to 1af91f5.

  • drain a timer quickly time: [393.12 ns 401.05 ns 408.49 ns]
    change: [-1.2572% +1.1854% +4.1744%] (p = 0.43 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1+1 entries
    time: [201.73 ns 202.24 ns 202.77 ns]
    change: [+3.2397% +3.6420% +3.9921%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 3+1 entries
    time: [241.13 ns 241.69 ns 242.31 ns]
    change: [+3.0761% +3.4459% +3.8388%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 10+1 entries
    time: [241.69 ns 242.57 ns 243.61 ns]
    change: [+2.8476% +3.7287% +4.7999%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 1000+1 entries
    time: [219.41 ns 219.59 ns 219.79 ns]
    change: [+1.2618% +2.0307% +2.7122%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • RxStreamOrderer::inbound_frame()
    time: [118.62 ms 118.73 ms 118.83 ms]
    change: [+0.4265% +0.5429% +0.6529%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [117.10 ms 117.36 ms 117.62 ms]
    thrpt: [34.009 MiB/s 34.083 MiB/s 34.159 MiB/s]
    change:
    time: [+0.3223% +0.5919% +0.8496%] (p = 0.00 < 0.05)
    thrpt: [-0.8424% -0.5884% -0.3213%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [117.82 ms 117.97 ms 118.12 ms]
    thrpt: [33.863 MiB/s 33.906 MiB/s 33.949 MiB/s]
    change:
    time: [+0.4516% +0.6313% +0.8043%] (p = 0.00 < 0.05)
    thrpt: [-0.7978% -0.6273% -0.4496%]
    Change within noise threshold.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 765.2 ± 388.5 351.1 1337.5 1.00
neqo msquic reno on 1918.9 ± 30.4 1877.0 1970.8 1.00
neqo msquic reno 1922.1 ± 52.3 1859.6 2046.6 1.00
neqo msquic cubic on 1968.1 ± 179.5 1764.0 2447.6 1.00
neqo msquic cubic 1985.0 ± 171.3 1816.4 2417.7 1.00
msquic neqo reno on 4527.8 ± 278.2 4262.9 5061.2 1.00
msquic neqo reno 4395.4 ± 437.2 4079.5 5590.4 1.00
msquic neqo cubic on 4420.5 ± 160.3 4215.4 4636.4 1.00
msquic neqo cubic 4368.3 ± 155.1 4154.3 4585.6 1.00
neqo neqo reno on 3658.2 ± 228.0 3399.3 4095.2 1.00
neqo neqo reno 3756.7 ± 217.3 3283.0 4069.2 1.00
neqo neqo cubic on 4380.4 ± 131.6 4137.2 4513.8 1.00
neqo neqo cubic 4245.5 ± 261.1 3556.6 4443.6 1.00

⬇️ Download logs

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.

2 participants