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(qns): run all tests and remove msquic & quic-go #1785

Closed
wants to merge 2 commits into from

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Apr 2, 2024

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.

Long term we can consider parallelizing the test execution as suggested by Marten in quic-go/quic-go#4339 (comment).

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.
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.05%. Comparing base (27a7250) to head (b3e23c4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1785      +/-   ##
==========================================
- Coverage   93.06%   93.05%   -0.01%     
==========================================
  Files         117      117              
  Lines       36364    36366       +2     
==========================================
  Hits        33841    33841              
- Misses       2523     2525       +2     

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

Copy link

github-actions bot commented Apr 2, 2024

Benchmark results

Performance differences relative to 0751429.

  • drain a timer quickly time: [356.15 ns 363.19 ns 369.67 ns]
    change: [-0.5743% +1.4558% +3.4234%] (p = 0.15 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1+1 entries
    time: [195.09 ns 195.52 ns 195.98 ns]
    change: [-0.5407% -0.1653% +0.2256%] (p = 0.41 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 3+1 entries
    time: [236.36 ns 236.96 ns 237.59 ns]
    change: [-0.1068% +0.5035% +1.0499%] (p = 0.08 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 10+1 entries
    time: [235.49 ns 236.47 ns 237.57 ns]
    change: [+0.2131% +0.6601% +1.1715%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 1000+1 entries
    time: [217.24 ns 217.44 ns 217.65 ns]
    change: [-0.4992% +0.2006% +0.8846%] (p = 0.59 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [121.17 ms 121.24 ms 121.31 ms]
    change: [+2.0983% +2.1912% +2.2823%] (p = 0.00 < 0.05)
    💔 Performance has regressed.

  • transfer/Run multiple transfers with varying seeds
    time: [120.43 ms 120.67 ms 120.91 ms]
    thrpt: [33.084 MiB/s 33.148 MiB/s 33.215 MiB/s]
    change:
    time: [+2.1369% +2.4523% +2.7586%] (p = 0.00 < 0.05)
    thrpt: [-2.6845% -2.3936% -2.0922%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [121.20 ms 121.35 ms 121.51 ms]
    thrpt: [32.920 MiB/s 32.962 MiB/s 33.003 MiB/s]
    change:
    time: [+1.9415% +2.1380% +2.3329%] (p = 0.00 < 0.05)
    thrpt: [-2.2797% -2.0933% -1.9045%]
    Change within noise threshold.

  • 1-conn/1-100mb-resp (aka. Download)/client
    time: [1.1120 s 1.1178 s 1.1242 s]
    thrpt: [88.954 MiB/s 89.461 MiB/s 89.927 MiB/s]
    change:
    time: [-1.3587% -0.1724% +0.9772%] (p = 0.80 > 0.05)
    thrpt: [-0.9678% +0.1727% +1.3774%]
    No change in performance detected.

  • 1-conn/10_000-1b-seq-resp (aka. RPS)/client
    time: [427.14 ms 429.18 ms 431.22 ms]
    thrpt: [23.190 Kelem/s 23.300 Kelem/s 23.411 Kelem/s]
    change:
    time: [-0.1859% +0.4666% +1.1440%] (p = 0.17 > 0.05)
    thrpt: [-1.1310% -0.4645% +0.1863%]
    No change in performance detected.

  • 100-seq-conn/1-1b-resp (aka. HPS)/client
    time: [3.3593 s 3.3626 s 3.3660 s]
    thrpt: [29.709 elem/s 29.739 elem/s 29.768 elem/s]
    change:
    time: [-0.0220% +0.1014% +0.2251%] (p = 0.11 > 0.05)
    thrpt: [-0.2246% -0.1013% +0.0220%]
    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 701.8 ± 274.7 418.6 1114.7 1.00
neqo msquic reno on 2049.6 ± 304.1 1878.4 2803.8 1.00
neqo msquic reno 2149.7 ± 324.2 1867.3 2834.8 1.00
neqo msquic cubic on 2157.3 ± 304.6 1869.1 2589.3 1.00
neqo msquic cubic 2073.9 ± 250.5 1867.7 2566.2 1.00
msquic neqo reno on 4555.2 ± 281.0 4057.5 4999.6 1.00
msquic neqo reno 4384.7 ± 167.9 4122.9 4559.7 1.00
msquic neqo cubic on 4475.2 ± 197.8 4157.3 4849.8 1.00
msquic neqo cubic 4444.1 ± 202.5 4105.8 4798.9 1.00
neqo neqo reno on 3890.4 ± 279.0 3356.3 4306.3 1.00
neqo neqo reno 3905.1 ± 292.4 3552.5 4502.8 1.00
neqo neqo cubic on 4311.6 ± 259.2 3942.7 4680.4 1.00
neqo neqo cubic 4322.8 ± 422.1 3413.9 4926.7 1.00

⬇️ Download logs

mxinden added a commit to mxinden/neqo that referenced this pull request Apr 3, 2024
In mozilla#1785 the QUIC Network Simulator workflow
failed.

https://github.com/mozilla/neqo/actions/runs/8523273988/job/23345192160?pr=1785

This triggers the QUIC Network Simulator Comment workflow.

https://github.com/mozilla/neqo/actions/runs/8524906268

Though currently, the former does not upload the _comment_ artifact on failure,
which is to be consumed by the latter.

This commit makes the former always upload the comment data, such that the
latter can consume it on failure.
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
In #1785 the QUIC Network Simulator workflow
failed.

https://github.com/mozilla/neqo/actions/runs/8523273988/job/23345192160?pr=1785

This triggers the QUIC Network Simulator Comment workflow.

https://github.com/mozilla/neqo/actions/runs/8524906268

Though currently, the former does not upload the _comment_ artifact on failure,
which is to be consumed by the latter.

This commit makes the former always upload the comment data, such that the
latter can consume it on failure.
@larseggert
Copy link
Collaborator

Would we not also want to remove neqo and only run neqo-latest?

@mxinden
Copy link
Collaborator Author

mxinden commented Apr 8, 2024

I had in mind to catch regressions between neqo versions, i.e. between neqo-latest and neqo (last main commit). I don't feel strongly about it.

@mxinden
Copy link
Collaborator Author

mxinden commented Apr 8, 2024

Maybe also worth excluding the QUIC Interop measurements via -t onlyTests.

https://github.com/quic-interop/quic-interop-runner/blob/4be6491794a08899f295dc5cdf9eeba8e9fa5431/run.py#L121

I need to play with this some more.

@larseggert
Copy link
Collaborator

Yes, tests should be sufficient.

@larseggert
Copy link
Collaborator

I had in mind to catch regressions between neqo versions, i.e. between neqo-latest and neqo (last main commit). I don't feel strongly about it.

Yeah, that makes sense. Leave it in. Esp. if we do -t onlyTests.

@mxinden
Copy link
Collaborator Author

mxinden commented Apr 24, 2024

Replaced by #1847.

@mxinden mxinden closed this Apr 24, 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