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

refactor(transport/recovery): remove needless mut #1927

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jun 17, 2024

No description provided.

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.

Interesting that clippy didn't find this.

@mxinden
Copy link
Collaborator Author

mxinden commented Jun 17, 2024

Interesting that clippy didn't find this.

I am surprised as well.

Tried to figure out where process_output is not idempotent (see #1917 (comment)). Still looking.

@mxinden mxinden marked this pull request as ready for review June 17, 2024 13:31
@mxinden mxinden changed the title fix(transport/recovery): remove needless mut refactor(transport/recovery): remove needless mut Jun 17, 2024
Copy link

Failed Interop Tests

QUIC Interop Runner, client vs. server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.82%. Comparing base (6664452) to head (71bc8fc).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1927   +/-   ##
=======================================
  Coverage   94.82%   94.82%           
=======================================
  Files         110      110           
  Lines       35792    35792           
=======================================
  Hits        33938    33938           
  Misses       1854     1854           

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

Copy link

Benchmark results

Performance differences relative to 6664452.

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [196.94 ns 197.41 ns 197.91 ns]
       change: [-0.1377% +0.2602% +0.6748%] (p = 0.21 > 0.05)
Found 18 outliers among 100 measurements (18.00%)
  7 (7.00%) high mild
  11 (11.00%) high severe
coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [238.24 ns 238.79 ns 239.38 ns]
       change: [-0.3351% +0.1358% +0.6343%] (p = 0.59 > 0.05)
Found 14 outliers among 100 measurements (14.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  11 (11.00%) high severe
coalesce_acked_from_zero 10+1 entries: Change within noise threshold.
       time:   [237.44 ns 238.20 ns 239.10 ns]
       change: [+0.0746% +0.5662% +1.1094%] (p = 0.03 < 0.05)
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) low mild
  7 (7.00%) high severe
coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [217.15 ns 217.43 ns 217.76 ns]
       change: [-4.4447% -1.1277% +0.9330%] (p = 0.61 > 0.05)
Found 13 outliers among 100 measurements (13.00%)
  5 (5.00%) high mild
  8 (8.00%) high severe
RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [119.61 ms 119.71 ms 119.81 ms]
       change: [+0.3259% +0.4394% +0.5526%] (p = 0.00 < 0.05)
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild
transfer/Run multiple transfers with varying seeds: Change within noise threshold.
       time:   [120.25 ms 120.56 ms 120.87 ms]
       thrpt:  [33.093 MiB/s 33.179 MiB/s 33.263 MiB/s]
change:
       time:   [+0.1124% +0.4875% +0.8214%] (p = 0.01 < 0.05)
       thrpt:  [-0.8147% -0.4852% -0.1123%]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low mild
  1 (1.00%) high severe
transfer/Run multiple transfers with the same seed: Change within noise threshold.
       time:   [120.66 ms 120.85 ms 121.05 ms]
       thrpt:  [33.043 MiB/s 33.099 MiB/s 33.150 MiB/s]
change:
       time:   [+0.2663% +0.4758% +0.6839%] (p = 0.00 < 0.05)
       thrpt:  [-0.6793% -0.4735% -0.2656%]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
1-conn/1-100mb-resp (aka. Download)/client: 💚 Performance has improved.
       time:   [1.0562 s 1.0825 s 1.1156 s]
       thrpt:  [89.639 MiB/s 92.379 MiB/s 94.683 MiB/s]
change:
       time:   [-8.1243% -4.9535% -1.3187%] (p = 0.02 < 0.05)
       thrpt:  [+1.3363% +5.2117% +8.8427%]
Found 2 outliers among 10 measurements (20.00%)
  2 (20.00%) high severe
1-conn/10_000-parallel-1b-resp (aka. RPS)/client: No change in performance detected.
       time:   [387.37 ms 390.91 ms 394.46 ms]
       thrpt:  [25.351 Kelem/s 25.581 Kelem/s 25.815 Kelem/s]
change:
       time:   [-1.8699% -0.6289% +0.4875%] (p = 0.30 > 0.05)
       thrpt:  [-0.4851% +0.6329% +1.9056%]

1-conn/1-1b-resp (aka. HPS)/client
time: [41.875 ms 41.995 ms 42.134 ms]
thrpt: [23.734 elem/s 23.813 elem/s 23.880 elem/s]
change:
time: [-0.4690% +0.0105% +0.4821%] (p = 0.96 > 0.05)
thrpt: [-0.4798% -0.0105% +0.4712%]
No change in performance detected.
Found 25 outliers among 100 measurements (25.00%)
13 (13.00%) low severe
1 (1.00%) low mild
11 (11.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 136.7 ± 33.6 96.8 214.7 1.00
neqo msquic reno on 270.5 ± 12.7 257.2 291.8 1.00
neqo msquic reno 269.3 ± 11.2 251.2 284.8 1.00
neqo msquic cubic on 266.3 ± 11.0 249.5 282.8 1.00
neqo msquic cubic 261.7 ± 3.8 256.4 271.0 1.00
msquic neqo reno on 941.5 ± 16.1 921.4 963.3 1.00
msquic neqo reno 917.4 ± 12.1 906.3 944.9 1.00
msquic neqo cubic on 897.3 ± 27.5 876.7 954.4 1.00
msquic neqo cubic 884.8 ± 10.6 876.4 901.1 1.00
neqo neqo reno on 925.9 ± 10.8 917.8 948.5 1.00
neqo neqo reno 911.0 ± 18.0 883.0 947.0 1.00
neqo neqo cubic on 913.1 ± 14.3 901.9 948.0 1.00
neqo neqo cubic 863.0 ± 11.2 843.5 882.8 1.00

⬇️ Download logs

Copy link

Firefox builds for this PR

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

@larseggert larseggert added this pull request to the merge queue Jun 17, 2024
Merged via the queue into mozilla:main with commit 290d1f0 Jun 17, 2024
56 checks passed
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