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

ci: Run the sanitizers in parallel #2166

Merged
merged 22 commits into from
Oct 11, 2024

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented Oct 10, 2024

This speeds up the matrix.

Also grab MSRV from Cargo.toml instead of hardcoding it.

Copy link

github-actions bot commented Oct 10, 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

@larseggert larseggert marked this pull request as ready for review October 10, 2024 10:03
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.38%. Comparing base (5f8d876) to head (6cc61f0).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2166      +/-   ##
==========================================
- Coverage   95.39%   95.38%   -0.01%     
==========================================
  Files         112      112              
  Lines       36373    36373              
==========================================
- Hits        34697    34696       -1     
- Misses       1676     1677       +1     

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

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.

Definitely in favor of the toolchains job.

In case the gain from parallelizing the sanitizers is worth the complexity, 👍.

.github/workflows/check.yml Outdated Show resolved Hide resolved
Co-authored-by: Max Inden <[email protected]>
Signed-off-by: Lars Eggert <[email protected]>
@larseggert
Copy link
Collaborator Author

larseggert commented Oct 10, 2024

Do you think I should just do a separate workflow for the sanitizer runs, like we do for clippy? May be more maintainable.

Yes, I like a separate workflow better.

@larseggert
Copy link
Collaborator Author

@mxinden any ideas on what's causing the two failures?

Copy link

github-actions bot commented Oct 10, 2024

Benchmark results

Performance differences relative to 5f8d876.

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [98.801 ns 99.064 ns 99.330 ns]
       change: [-0.4137% +0.1527% +0.6230%] (p = 0.59 > 0.05)

Found 8 outliers among 100 measurements (8.00%)
6 (6.00%) high mild
2 (2.00%) high severe

coalesce_acked_from_zero 3+1 entries: Change within noise threshold.
       time:   [116.82 ns 117.17 ns 117.55 ns]
       change: [+0.0552% +0.5266% +1.0686%] (p = 0.03 < 0.05)

Found 19 outliers among 100 measurements (19.00%)
2 (2.00%) low severe
1 (1.00%) low mild
4 (4.00%) high mild
12 (12.00%) high severe

coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [116.50 ns 116.95 ns 117.50 ns]
       change: [+0.0359% +0.8467% +2.0023%] (p = 0.07 > 0.05)

Found 14 outliers among 100 measurements (14.00%)
4 (4.00%) low severe
1 (1.00%) low mild
2 (2.00%) high mild
7 (7.00%) high severe

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [98.017 ns 98.163 ns 98.342 ns]
       change: [-0.7539% +0.1719% +1.2076%] (p = 0.74 > 0.05)

Found 9 outliers among 100 measurements (9.00%)
4 (4.00%) high mild
5 (5.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [111.17 ms 111.22 ms 111.27 ms]
       change: [-0.8529% -0.7912% -0.7246%] (p = 0.00 < 0.05)

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

transfer/pacing-false/varying-seeds: No change in performance detected.
       time:   [26.927 ms 28.140 ms 29.392 ms]
       change: [-3.6506% +1.9873% +7.9025%] (p = 0.49 > 0.05)
transfer/pacing-true/varying-seeds: No change in performance detected.
       time:   [35.284 ms 36.997 ms 38.729 ms]
       change: [-2.8743% +4.3193% +11.825%] (p = 0.24 > 0.05)
transfer/pacing-false/same-seed: No change in performance detected.
       time:   [25.354 ms 26.170 ms 26.996 ms]
       change: [-5.4377% -1.5867% +2.6055%] (p = 0.45 > 0.05)
transfer/pacing-true/same-seed: No change in performance detected.
       time:   [39.681 ms 41.495 ms 43.314 ms]
       change: [-7.4968% -1.3183% +4.6377%] (p = 0.67 > 0.05)
1-conn/1-100mb-resp (aka. Download)/client: No change in performance detected.
       time:   [113.78 ms 114.30 ms 114.77 ms]
       thrpt:  [871.33 MiB/s 874.93 MiB/s 878.87 MiB/s]
change:
       time:   [-0.2018% +0.3948% +0.9476%] (p = 0.18 > 0.05)
       thrpt:  [-0.9387% -0.3932% +0.2022%]

Found 6 outliers among 100 measurements (6.00%)
3 (3.00%) low severe
2 (2.00%) low mild
1 (1.00%) high mild

1-conn/10_000-parallel-1b-resp (aka. RPS)/client: No change in performance detected.
       time:   [314.42 ms 318.32 ms 322.19 ms]
       thrpt:  [31.038 Kelem/s 31.415 Kelem/s 31.804 Kelem/s]
change:
       time:   [-0.9214% +0.7517% +2.3260%] (p = 0.38 > 0.05)
       thrpt:  [-2.2731% -0.7461% +0.9300%]
1-conn/1-1b-resp (aka. HPS)/client: Change within noise threshold.
       time:   [33.971 ms 34.124 ms 34.290 ms]
       thrpt:  [29.163  elem/s 29.305  elem/s 29.437  elem/s]
change:
       time:   [+0.2410% +0.9596% +1.7039%] (p = 0.01 < 0.05)
       thrpt:  [-1.6754% -0.9505% -0.2404%]

Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.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 171.0 ± 92.0 91.6 390.8 1.00
neqo msquic reno on 310.3 ± 99.9 219.4 464.8 1.00
neqo msquic reno 238.2 ± 71.5 201.0 462.7 1.00
neqo msquic cubic on 216.9 ± 13.7 203.5 245.3 1.00
neqo msquic cubic 212.5 ± 8.0 203.3 233.9 1.00
msquic neqo reno on 149.2 ± 88.7 88.2 334.1 1.00
msquic neqo reno 142.9 ± 82.6 84.5 359.2 1.00
msquic neqo cubic on 126.3 ± 79.4 81.8 355.9 1.00
msquic neqo cubic 159.7 ± 94.3 85.5 369.6 1.00
neqo neqo reno on 266.0 ± 137.0 135.4 481.5 1.00
neqo neqo reno 279.9 ± 139.4 139.2 553.2 1.00
neqo neqo cubic on 169.0 ± 64.4 124.5 402.9 1.00
neqo neqo cubic 194.2 ± 87.5 121.6 461.8 1.00

⬇️ Download logs

@larseggert larseggert added this pull request to the merge queue Oct 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 10, 2024
@larseggert larseggert added this pull request to the merge queue Oct 11, 2024
Merged via the queue into mozilla:main with commit 249d062 Oct 11, 2024
63 of 70 checks passed
@larseggert larseggert deleted the ci-parallel-sanitizers branch October 11, 2024 05:12
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