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

feat: Replace HashMap with EnumMap #2434

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

larseggert
Copy link
Collaborator

In several places.

Copy link

codecov bot commented Feb 10, 2025

Codecov Report

Attention: Patch coverage is 95.70312% with 11 lines in your changes missing coverage. Please review.

Project coverage is 95.27%. Comparing base (9734cf2) to head (98885f9).

Files with missing lines Patch % Lines
neqo-transport/src/tparams.rs 94.16% 7 Missing ⚠️
neqo-transport/src/qlog.rs 91.83% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2434      +/-   ##
==========================================
- Coverage   95.28%   95.27%   -0.02%     
==========================================
  Files         114      114              
  Lines       37111    37124      +13     
  Branches    37111    37124      +13     
==========================================
+ Hits        35363    35370       +7     
- Misses       1742     1748       +6     
  Partials        6        6              

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

Copy link

github-actions bot commented Feb 10, 2025

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to cb058cb.

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

github-actions bot commented Feb 10, 2025

Benchmark results

Performance differences relative to 9734cf2.

decode 4096 bytes, mask ff: No change in performance detected.
       time:   [10.883 µs 10.928 µs 10.979 µs]
       change: [-0.0790% +0.3972% +0.8805%] (p = 0.12 > 0.05)

Found 19 outliers among 100 measurements (19.00%)
2 (2.00%) low severe
5 (5.00%) low mild
12 (12.00%) high severe

decode 1048576 bytes, mask ff: No change in performance detected.
       time:   [3.1278 ms 3.1370 ms 3.1477 ms]
       change: [-0.3435% +0.0774% +0.5065%] (p = 0.72 > 0.05)

Found 10 outliers among 100 measurements (10.00%)
1 (1.00%) high mild
9 (9.00%) high severe

decode 4096 bytes, mask 7f: No change in performance detected.
       time:   [17.670 µs 17.727 µs 17.785 µs]
       change: [-0.2851% +0.0766% +0.4274%] (p = 0.69 > 0.05)

Found 21 outliers among 100 measurements (21.00%)
1 (1.00%) low severe
2 (2.00%) low mild
1 (1.00%) high mild
17 (17.00%) high severe

decode 1048576 bytes, mask 7f: No change in performance detected.
       time:   [5.4025 ms 5.4142 ms 5.4273 ms]
       change: [-0.3636% -0.0378% +0.2910%] (p = 0.82 > 0.05)

Found 15 outliers among 100 measurements (15.00%)
1 (1.00%) high mild
14 (14.00%) high severe

decode 4096 bytes, mask 3f: No change in performance detected.
       time:   [6.6542 µs 6.6786 µs 6.7054 µs]
       change: [-0.5278% +0.1345% +0.8746%] (p = 0.70 > 0.05)

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

decode 1048576 bytes, mask 3f: No change in performance detected.
       time:   [1.7580 ms 1.7581 ms 1.7583 ms]
       change: [-0.0130% -0.0026% +0.0073%] (p = 0.62 > 0.05)

Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [91.006 ns 91.301 ns 91.604 ns]
       change: [-0.6340% -0.1124% +0.3670%] (p = 0.67 > 0.05)

Found 10 outliers among 100 measurements (10.00%)
8 (8.00%) high mild
2 (2.00%) high severe

coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [109.42 ns 109.71 ns 110.03 ns]
       change: [-0.3850% +0.0241% +0.4713%] (p = 0.91 > 0.05)

Found 15 outliers among 100 measurements (15.00%)
2 (2.00%) low mild
2 (2.00%) high mild
11 (11.00%) high severe

coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [108.94 ns 109.28 ns 109.72 ns]
       change: [-0.5127% -0.0140% +0.4881%] (p = 0.96 > 0.05)

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

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [93.509 ns 93.663 ns 93.824 ns]
       change: [-1.0293% -0.0611% +0.8517%] (p = 0.90 > 0.05)

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

RxStreamOrderer::inbound_frame(): No change in performance detected.
       time:   [111.45 ms 111.51 ms 111.57 ms]
       change: [-0.0648% +0.0051% +0.0752%] (p = 0.89 > 0.05)

Found 8 outliers among 100 measurements (8.00%)
7 (7.00%) low mild
1 (1.00%) high mild

SentPackets::take_ranges: No change in performance detected.
       time:   [5.2739 µs 5.4841 µs 5.7227 µs]
       change: [-1.8321% +1.0953% +4.2502%] (p = 0.49 > 0.05)

Found 7 outliers among 100 measurements (7.00%)
6 (6.00%) high mild
1 (1.00%) high severe

transfer/pacing-false/varying-seeds: Change within noise threshold.
       time:   [37.114 ms 37.193 ms 37.281 ms]
       change: [+0.7097% +0.9949% +1.2954%] (p = 0.00 < 0.05)

Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

transfer/pacing-true/varying-seeds: Change within noise threshold.
       time:   [37.020 ms 37.098 ms 37.187 ms]
       change: [+1.3268% +1.6221% +1.9456%] (p = 0.00 < 0.05)

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

transfer/pacing-false/same-seed: Change within noise threshold.
       time:   [36.841 ms 36.898 ms 36.955 ms]
       change: [+0.8428% +1.0837% +1.3225%] (p = 0.00 < 0.05)

Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) low mild

transfer/pacing-true/same-seed: Change within noise threshold.
       time:   [37.439 ms 37.501 ms 37.562 ms]
       change: [+2.4179% +2.6554% +2.8913%] (p = 0.00 < 0.05)
1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: Change within noise threshold.
       time:   [833.85 ms 844.05 ms 854.64 ms]
       thrpt:  [117.01 MiB/s 118.48 MiB/s 119.93 MiB/s]
change:
       time:   [-3.7879% -2.1275% -0.6143%] (p = 0.01 < 0.05)
       thrpt:  [+0.6181% +2.1737% +3.9370%]
1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: No change in performance detected.
       time:   [315.84 ms 319.26 ms 322.69 ms]
       thrpt:  [30.989 Kelem/s 31.322 Kelem/s 31.662 Kelem/s]
change:
       time:   [-1.8003% -0.2271% +1.3499%] (p = 0.78 > 0.05)
       thrpt:  [-1.3319% +0.2276% +1.8333%]
1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.
       time:   [25.423 ms 25.574 ms 25.723 ms]
       thrpt:  [38.876  elem/s 39.102  elem/s 39.334  elem/s]
change:
       time:   [-0.9017% -0.0401% +0.8596%] (p = 0.92 > 0.05)
       thrpt:  [-0.8523% +0.0401% +0.9099%]
1-conn/1-100mb-resp/mtu-1504 (aka. Upload)/client: 💚 Performance has improved.
       time:   [1.8316 s 1.8519 s 1.8725 s]
       thrpt:  [53.405 MiB/s 54.000 MiB/s 54.596 MiB/s]
change:
       time:   [-4.9163% -3.5835% -2.1294%] (p = 0.00 < 0.05)
       thrpt:  [+2.1758% +3.7167% +5.1705%]

Client/server transfer results

Performance differences relative to 9734cf2.

Transfer of 33554432 bytes over loopback, 30 runs. All unit-less numbers are in milliseconds.

Client Server CC Pacing Mean ± σ Min Max Δ main Δ main
neqo neqo reno on 506.8 ± 56.3 428.2 639.7 💚 -44.1 -2.1%
neqo neqo reno 495.9 ± 50.1 433.1 629.2 💚 -43.0 -2.1%
neqo neqo cubic on 528.9 ± 79.8 443.2 920.0 💚 -16.0 -0.7%
neqo neqo cubic 515.4 ± 43.4 454.7 658.6 💚 -28.6 -1.3%
google neqo reno on 887.3 ± 110.0 647.2 1168.9 💚 -17.2 -0.5%
google neqo reno 879.5 ± 98.6 626.7 967.3 💚 -22.3 -0.6%
google neqo cubic on 886.9 ± 107.4 641.9 1145.4 💚 -16.6 -0.5%
google neqo cubic 878.9 ± 97.2 644.1 980.2 💚 -16.4 -0.5%
google google 540.3 ± 14.1 523.2 583.5 💚 -0.1 -0.0%
neqo msquic reno on 226.7 ± 37.0 202.8 411.8 💚 9.0 1.0%
neqo msquic reno 217.8 ± 10.7 202.8 240.9 💚 -2.5 -0.3%
neqo msquic cubic on 223.6 ± 24.5 202.5 335.0 💚 3.7 0.4%
neqo msquic cubic 228.7 ± 39.1 200.7 396.7 💚 11.8 1.3%
msquic msquic 115.8 ± 24.6 95.5 225.5 💚 -1.2 -0.3%

⬇️ 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.

Thanks for the cleanup!

pub const MAX_DATAGRAM_FRAME_SIZE: TransportParameterId = 0x0020;
#[allow(clippy::enum_clike_unportable_variant)]
#[derive(Debug, Clone, Enum, PartialEq, Eq)]
pub enum TransportParameterId {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very much in favor of this enum!

neqo-transport/src/tparams.rs Outdated Show resolved Hide resolved
neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved
neqo-transport/src/connection/mod.rs Show resolved Hide resolved
neqo-transport/src/crypto.rs Outdated Show resolved Hide resolved
neqo-transport/src/qlog.rs Outdated Show resolved Hide resolved
neqo-transport/src/tparams.rs Outdated Show resolved Hide resolved
neqo-transport/src/version.rs Show resolved Hide resolved
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.

3 participants