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.

Sign up for GitHub

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

Jump to bottom

ci: Parallelize the QNS run #1847

Merged
merged 76 commits into from
Apr 23, 2024
Merged

ci: Parallelize the QNS run #1847

merged 76 commits into from
Apr 23, 2024

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented Apr 18, 2024 •
edited
Loading

WIP

mxinden and others added 4 commits April 2, 2024 14:54
@mxinden
ci(qns): run all tests and remove msquic & quic-go
b3e23c4 
Run all QUIC Interop Runner testcases.

For the sake of reducing runtime, this commit also removes msquic and quic-go,
leaving only ngtcp2 and neqo itself. Reason being, that ngtcp2 is the only
implementation that passes all testcases, especially the ECN testcase.
@mxinden
Trigger CI
6bb4884
@larseggert
WIP
ccea071
@larseggert
ci: Parallelize the QNS run
29f2192 
WIP
@larseggert larseggert changed the title Ci matrix qns ci: Parallelize the QNS run Apr 18, 2024
@larseggert
Merge branch 'main' into ci-matrix-qns
0f2eacc 
Signed-off-by: Lars Eggert <lars@eggert.org>
@mxinden mxinden mentioned this pull request Apr 18, 2024
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 18, 2024 •
edited
Loading

Benchmark results

Performance differences relative to 168e06c.

  • drain a timer quickly time: [327.51 ns 335.42 ns 342.78 ns]
    change: [-1.4040% +0.9029% +3.2054%] (p = 0.43 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1+1 entries
    time: [202.09 ns 202.50 ns 202.94 ns]
    change: [+1.3989% +1.8089% +2.2126%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • coalesce_acked_from_zero 3+1 entries
    time: [244.93 ns 245.57 ns 246.26 ns]
    change: [+0.4516% +1.1569% +1.7948%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 10+1 entries
    time: [243.94 ns 244.80 ns 245.81 ns]
    change: [+0.9779% +1.6068% +2.3630%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 1000+1 entries
    time: [221.85 ns 222.03 ns 222.25 ns]
    change: [-5.6972% -0.2499% +3.0450%] (p = 0.91 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [118.67 ms 118.73 ms 118.79 ms]
    change: [+0.1270% +0.2152% +0.2983%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [118.67 ms 118.95 ms 119.21 ms]
    thrpt: [33.554 MiB/s 33.629 MiB/s 33.707 MiB/s]
    change:
    time: [-1.3567% -1.0581% -0.7418%] (p = 0.00 < 0.05)
    thrpt: [+0.7473% +1.0694% +1.3753%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [119.00 ms 119.15 ms 119.31 ms]
    thrpt: [33.527 MiB/s 33.570 MiB/s 33.613 MiB/s]
    change:
    time: [-1.5386% -1.3510% -1.1555%] (p = 0.00 < 0.05)
    thrpt: [+1.1690% +1.3695% +1.5627%]
    Change within noise threshold.

  • 1-conn/1-100mb-resp (aka. Download)/client
    time: [1.0997 s 1.1072 s 1.1152 s]
    thrpt: [89.669 MiB/s 90.319 MiB/s 90.937 MiB/s]
    change:
    time: [-2.9156% -2.0440% -1.2164%] (p = 0.00 < 0.05)
    thrpt: [+1.2314% +2.0866% +3.0031%]
    💚 Performance has improved.

  • 1-conn/10_000-parallel-1b-resp (aka. RPS)/client
    time: [427.82 ms 430.11 ms 432.43 ms]
    thrpt: [23.125 Kelem/s 23.250 Kelem/s 23.374 Kelem/s]
    change:
    time: [-1.0334% -0.3051% +0.4428%] (p = 0.44 > 0.05)
    thrpt: [-0.4409% +0.3061% +1.0442%]
    No change in performance detected.

  • 1-conn/1-1b-resp (aka. HPS)/client
    time: [49.222 ms 49.556 ms 49.890 ms]
    thrpt: [20.044 elem/s 20.179 elem/s 20.316 elem/s]
    change:
    time: [-0.4161% +0.5589% +1.6195%] (p = 0.28 > 0.05)
    thrpt: [-1.5936% -0.5558% +0.4178%]
    No change in performance detected.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 398.5 ± 35.6 361.7 477.1 1.00
neqo msquic reno on 773.6 ± 13.4 757.6 802.2 1.00
neqo msquic reno 785.2 ± 35.7 740.9 870.1 1.00
neqo msquic cubic on 793.0 ± 22.0 769.4 839.5 1.00
neqo msquic cubic 788.9 ± 41.8 729.3 854.0 1.00
msquic neqo reno on 4337.7 ± 143.6 4124.4 4486.8 1.00
msquic neqo reno 4399.2 ± 317.9 4112.7 5237.0 1.00
msquic neqo cubic on 4374.9 ± 152.4 4100.7 4622.3 1.00
msquic neqo cubic 4329.5 ± 137.3 4116.9 4512.1 1.00
neqo neqo reno on 3213.9 ± 181.3 2954.1 3519.4 1.00
neqo neqo reno 3514.8 ± 261.1 2960.4 3806.2 1.00
neqo neqo cubic on 4142.0 ± 353.8 3385.4 4570.5 1.00
neqo neqo cubic 4056.9 ± 458.6 2819.2 4399.4 1.00

⬇️ Download logs

@codecov Codecov
Copy link

codecov bot commented Apr 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.23%. Comparing base (168e06c) to head (c2ecd84).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1847   +/-   ##
=======================================
  Coverage   93.23%   93.23%           
=======================================
  Files         110      110           
  Lines       35749    35749           
=======================================
  Hits        33330    33330           
  Misses       2419     2419

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

larseggert added a commit to larseggert/neqo that referenced this pull request Apr 23, 2024
@larseggert
ci: Handle empty log-url when commenting on PRs
c2e6636 
Extracted from mozilla#1847
@larseggert larseggert mentioned this pull request Apr 23, 2024
Merged
larseggert added a commit that referenced this pull request Apr 23, 2024
@larseggert
ci: Handle empty log-url when commenting on PRs (#1851)
e53d60f 
Extracted from #1847
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 23, 2024 •
edited
Loading

Failed Interop Tests

Succeeded and unsupported tests

Succeeded Interop Tests

Unsupported Interop Tests

@larseggert larseggert marked this pull request as ready for review April 23, 2024 11:41
@larseggert larseggert added this pull request to the merge queue Apr 23, 2024
Merged via the queue into mozilla:main with commit 266c22c Apr 23, 2024
48 checks passed
@larseggert larseggert deleted the ci-matrix-qns branch April 23, 2024 14:45
@mxinden
Copy link
Collaborator

mxinden commented Apr 24, 2024

~10 minutes per run -- cool! Lots better than the previous sequential execution.

With the new QNS comment, I am afraid we will not notice regressions. It is difficult for me to see whether a failure is new, or exists on main as well.

Once we have #1849, we could reduce the set of implementations to test against, to the subset that succeeds across all testcases.

Or we could show the diff of failures between main and a pull request in the QNS comment.

Thoughts?

@larseggert
Copy link
Collaborator Author

Yeah, I need to cache the results for the main branch and highlight differences to the PR, similar how we do it for the benchmarks.

@mxinden mxinden mentioned this pull request Apr 24, 2024
Closed
@mxinden mxinden mentioned this pull request May 3, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KershawChang KershawChang KershawChang approved these changes

@martinthomson martinthomson Awaiting requested review from martinthomson martinthomson is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@larseggert @mxinden @KershawChang