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: Log rttvar in qlog #2005

Merged
merged 3 commits into from
Jul 25, 2024
Merged

fix: Log rttvar in qlog #2005

merged 3 commits into from
Jul 25, 2024

Conversation

larseggert
Copy link
Collaborator

No description provided.

Copy link

github-actions bot commented Jul 23, 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

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.99%. Comparing base (88d3ed1) to head (b64a7df).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2005   +/-   ##
=======================================
  Coverage   94.99%   94.99%           
=======================================
  Files         112      112           
  Lines       36371    36372    +1     
=======================================
+ Hits        34552    34553    +1     
  Misses       1819     1819           

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

Copy link

github-actions bot commented Jul 23, 2024

Benchmark results

Performance differences relative to 608c0a7.

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [193.30 ns 193.73 ns 194.17 ns]
       change: [-0.7435% -0.2437% +0.1736%] (p = 0.33 > 0.05)

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

coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [237.05 ns 237.73 ns 238.43 ns]
       change: [-0.9925% -0.2951% +0.3160%] (p = 0.41 > 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:   [236.30 ns 237.04 ns 237.92 ns]
       change: [-0.5894% -0.2239% +0.1316%] (p = 0.25 > 0.05)

Found 8 outliers among 100 measurements (8.00%)
2 (2.00%) low mild
1 (1.00%) high mild
5 (5.00%) high severe

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [217.05 ns 217.22 ns 217.40 ns]
       change: [-0.3995% +0.2847% +1.0225%] (p = 0.46 > 0.05)

Found 11 outliers among 100 measurements (11.00%)
3 (3.00%) high mild
8 (8.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [119.91 ms 119.99 ms 120.08 ms]
       change: [-0.5646% -0.3439% -0.1781%] (p = 0.00 < 0.05)

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

transfer/Run multiple transfers with varying seeds: No change in performance detected.
       time:   [55.995 ms 59.163 ms 62.307 ms]
       thrpt:  [64.198 MiB/s 67.610 MiB/s 71.435 MiB/s]
change:
       time:   [-10.578% -3.4847% +3.8872%] (p = 0.36 > 0.05)
       thrpt:  [-3.7418% +3.6105% +11.830%]
transfer/Run multiple transfers with the same seed: No change in performance detected.
       time:   [68.304 ms 73.395 ms 78.390 ms]
       thrpt:  [51.027 MiB/s 54.500 MiB/s 58.562 MiB/s]
change:
       time:   [-5.8779% +5.0685% +17.802%] (p = 0.39 > 0.05)
       thrpt:  [-15.112% -4.8240% +6.2450%]
1-conn/1-100mb-resp (aka. Download)/client: No change in performance detected.
       time:   [274.45 ms 288.46 ms 299.04 ms]
       thrpt:  [334.41 MiB/s 346.67 MiB/s 364.37 MiB/s]
change:
       time:   [-7.1290% -2.3989% +1.9128%] (p = 0.39 > 0.05)
       thrpt:  [-1.8769% +2.4579% +7.6762%]

Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) low mild

1-conn/10_000-parallel-1b-resp (aka. RPS)/client: No change in performance detected.
       time:   [411.80 ms 415.18 ms 418.61 ms]
       thrpt:  [23.888 Kelem/s 24.086 Kelem/s 24.284 Kelem/s]
change:
       time:   [-1.2585% -0.1799% +0.9976%] (p = 0.75 > 0.05)
       thrpt:  [-0.9877% +0.1803% +1.2745%]
1-conn/1-1b-resp (aka. HPS)/client: No change in performance detected.
       time:   [67.797 ms 68.052 ms 68.356 ms]
       thrpt:  [14.629  elem/s 14.695  elem/s 14.750  elem/s]
change:
       time:   [-1.3208% -0.6245% +0.1168%] (p = 0.09 > 0.05)
       thrpt:  [-0.1167% +0.6284% +1.3384%]

Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) low mild
3 (3.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 114.0 ± 16.8 102.7 157.9 1.00
neqo msquic reno on 291.4 ± 58.4 254.0 435.1 1.00
neqo msquic reno 277.2 ± 11.7 258.7 297.0 1.00
neqo msquic cubic on 284.7 ± 47.1 259.7 416.4 1.00
neqo msquic cubic 266.7 ± 10.0 255.3 286.5 1.00
msquic neqo reno on 180.6 ± 61.3 150.8 399.3 1.00
msquic neqo reno 171.8 ± 64.4 138.1 373.5 1.00
msquic neqo cubic on 170.2 ± 68.6 141.5 362.0 1.00
msquic neqo cubic 174.7 ± 25.7 111.4 210.0 1.00
neqo neqo reno on 211.2 ± 61.4 157.9 407.1 1.00
neqo neqo reno 174.3 ± 11.4 157.9 201.5 1.00
neqo neqo cubic on 226.1 ± 80.1 164.3 400.3 1.00
neqo neqo cubic 183.6 ± 17.5 168.4 228.7 1.00

⬇️ Download logs

Copy link
Collaborator

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks good to me.

; all following rtt fields are expressed in ms

https://quicwg.org/qlog/draft-ietf-quic-qlog-quic-events.html#name-metrics_updated

Slightly related note, why is qlog not using SI units?

neqo-transport/src/qlog.rs Show resolved Hide resolved
larseggert and others added 2 commits July 23, 2024 22:23
Co-authored-by: Max Inden <mail@max-inden.de>
Signed-off-by: Lars Eggert <lars@eggert.org>
This reverts commit cf986bb.

`as_millis_f32` is nightly experimental.
Copy link

Firefox builds for this PR

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

@larseggert larseggert added the needs-review Needs a review by a CODEOWNER. label Jul 24, 2024
@larseggert
Copy link
Collaborator Author

Are there other qlog probes we are missing?

@mxinden
Copy link
Collaborator

mxinden commented Jul 24, 2024

I assume most missing ones are tracked in #528 @larseggert.

@larseggert larseggert disabled auto-merge July 25, 2024 15:09
@larseggert larseggert merged commit 1815c3c into mozilla:main Jul 25, 2024
57 checks passed
@larseggert larseggert deleted the fix-rttvar branch July 25, 2024 15:09
@larseggert larseggert removed the needs-review Needs a review by a CODEOWNER. label Jul 25, 2024
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