Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Simplify neqo_common::log and enforce clippy format checks #2291

Merged
merged 19 commits into from
Jan 10, 2025

Conversation

larseggert
Copy link
Collaborator

@larseggert larseggert commented Dec 18, 2024

Also move variables into format strings where possible and make some other minor improvements.

Offshoot off #1963.

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 83.69781% with 82 lines in your changes missing coverage. Please review.

Project coverage is 95.31%. Comparing base (e664280) to head (6cede61).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
neqo-transport/src/connection/mod.rs 83.69% 15 Missing ⚠️
neqo-transport/src/send_stream.rs 61.53% 10 Missing ⚠️
neqo-transport/src/server.rs 52.63% 9 Missing ⚠️
neqo-transport/src/crypto.rs 81.25% 6 Missing ⚠️
neqo-http3/src/connection_client.rs 82.75% 5 Missing ⚠️
neqo-qpack/src/encoder_instructions.rs 50.00% 5 Missing ⚠️
neqo-http3/src/push_controller.rs 78.57% 3 Missing ⚠️
neqo-http3/src/recv_message.rs 70.00% 3 Missing ⚠️
neqo-common/src/qlog.rs 0.00% 2 Missing ⚠️
neqo-crypto/src/agent.rs 77.77% 2 Missing ⚠️
... and 18 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2291      +/-   ##
==========================================
+ Coverage   93.33%   95.31%   +1.98%     
==========================================
  Files         114      114              
  Lines       36899    36844      -55     
  Branches    36899    36844      -55     
==========================================
+ Hits        34440    35119     +679     
- Misses       1682     1717      +35     
+ Partials      777        8     -769     

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

Copy link

github-actions bot commented Dec 18, 2024

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to 648863b.

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 changed the title fix: Replace neqo_common::log with log crate fix: Replace neqo_common::log with log crate Dec 18, 2024
@larseggert larseggert changed the title fix: Replace neqo_common::log with log crate fix: Simplify neqo_common::log and enforce clippy format checks Dec 18, 2024
@larseggert larseggert linked an issue Jan 7, 2025 that may be closed by this pull request
Copy link

github-actions bot commented Jan 7, 2025

Benchmark results

Performance differences relative to 6013bde.

decode 4096 bytes, mask ff: No change in performance detected.
       time:   [11.154 µs 11.191 µs 11.233 µs]
       change: [-0.2669% +0.0566% +0.4206%] (p = 0.74 > 0.05)

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

decode 1048576 bytes, mask ff: No change in performance detected.
       time:   [3.0157 ms 3.0242 ms 3.0343 ms]
       change: [-0.6219% -0.1451% +0.3373%] (p = 0.55 > 0.05)

Found 8 outliers among 100 measurements (8.00%)
8 (8.00%) high severe

decode 4096 bytes, mask 7f: No change in performance detected.
       time:   [19.563 µs 19.643 µs 19.750 µs]
       change: [-0.7127% +0.0233% +0.6606%] (p = 0.95 > 0.05)

Found 25 outliers among 100 measurements (25.00%)
1 (1.00%) low severe
4 (4.00%) low mild
1 (1.00%) high mild
19 (19.00%) high severe

decode 1048576 bytes, mask 7f: No change in performance detected.
       time:   [5.1591 ms 5.1705 ms 5.1835 ms]
       change: [-0.4603% -0.1092% +0.2477%] (p = 0.54 > 0.05)

Found 13 outliers among 100 measurements (13.00%)
13 (13.00%) high severe

decode 4096 bytes, mask 3f: No change in performance detected.
       time:   [5.5319 µs 5.5497 µs 5.5751 µs]
       change: [-0.8053% +0.0390% +0.9298%] (p = 0.93 > 0.05)

Found 9 outliers among 100 measurements (9.00%)
3 (3.00%) high mild
6 (6.00%) high severe

decode 1048576 bytes, mask 3f: No change in performance detected.
       time:   [1.7622 ms 1.7693 ms 1.7787 ms]
       change: [-0.3241% +0.2275% +0.7815%] (p = 0.46 > 0.05)

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

coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [99.030 ns 99.351 ns 99.679 ns]
       change: [-0.0994% +0.2960% +0.7023%] (p = 0.16 > 0.05)

Found 13 outliers among 100 measurements (13.00%)
10 (10.00%) high mild
3 (3.00%) high severe

coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [116.99 ns 117.38 ns 117.78 ns]
       change: [-0.6739% +0.1787% +0.8422%] (p = 0.69 > 0.05)

Found 20 outliers among 100 measurements (20.00%)
3 (3.00%) low mild
1 (1.00%) high mild
16 (16.00%) high severe

coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [116.47 ns 116.98 ns 117.58 ns]
       change: [-0.2539% +0.7440% +1.9239%] (p = 0.23 > 0.05)

Found 17 outliers among 100 measurements (17.00%)
6 (6.00%) low severe
2 (2.00%) high mild
9 (9.00%) high severe

coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [97.209 ns 101.96 ns 112.55 ns]
       change: [-1.2602% +1.9954% +7.1418%] (p = 0.55 > 0.05)

Found 12 outliers among 100 measurements (12.00%)
7 (7.00%) high mild
5 (5.00%) high severe

RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [111.99 ms 112.04 ms 112.09 ms]
       change: [+0.6478% +0.8483% +0.9784%] (p = 0.00 < 0.05)

Found 21 outliers among 100 measurements (21.00%)
3 (3.00%) low severe
5 (5.00%) low mild
11 (11.00%) high mild
2 (2.00%) high severe

SentPackets::take_ranges: Change within noise threshold.
       time:   [5.5200 µs 5.6941 µs 5.8815 µs]
       change: [+0.1899% +2.7566% +5.2939%] (p = 0.04 < 0.05)

Found 8 outliers among 100 measurements (8.00%)
8 (8.00%) high mild

transfer/pacing-false/varying-seeds: Change within noise threshold.
       time:   [41.783 ms 41.864 ms 41.950 ms]
       change: [+0.0554% +0.3413% +0.6400%] (p = 0.02 < 0.05)

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

transfer/pacing-true/varying-seeds: Change within noise threshold.
       time:   [42.061 ms 42.146 ms 42.236 ms]
       change: [+0.0417% +0.3023% +0.5641%] (p = 0.03 < 0.05)

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

transfer/pacing-false/same-seed: Change within noise threshold.
       time:   [41.887 ms 41.960 ms 42.036 ms]
       change: [+0.0720% +0.3403% +0.5984%] (p = 0.01 < 0.05)

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

transfer/pacing-true/same-seed: No change in performance detected.
       time:   [41.972 ms 42.042 ms 42.116 ms]
       change: [-0.3954% -0.1432% +0.0976%] (p = 0.26 > 0.05)

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

1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: No change in performance detected.
       time:   [886.52 ms 897.01 ms 907.78 ms]
       thrpt:  [110.16 MiB/s 111.48 MiB/s 112.80 MiB/s]
change:
       time:   [-0.3979% +1.1399% +2.8221%] (p = 0.17 > 0.05)
       thrpt:  [-2.7446% -1.1271% +0.3995%]

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

1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: 💔 Performance has regressed.
       time:   [317.18 ms 319.55 ms 322.01 ms]
       thrpt:  [31.055 Kelem/s 31.294 Kelem/s 31.528 Kelem/s]
change:
       time:   [+4.6062% +5.6804% +6.7336%] (p = 0.00 < 0.05)
       thrpt:  [-6.3088% -5.3751% -4.4034%]

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

1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.
       time:   [34.263 ms 34.498 ms 34.755 ms]
       thrpt:  [28.773  elem/s 28.987  elem/s 29.186  elem/s]
change:
       time:   [-0.3736% +0.5612% +1.4828%] (p = 0.24 > 0.05)
       thrpt:  [-1.4611% -0.5581% +0.3750%]

Found 16 outliers among 100 measurements (16.00%)
5 (5.00%) low mild
2 (2.00%) high mild
9 (9.00%) high severe

1-conn/1-100mb-resp/mtu-1504 (aka. Upload)/client: Change within noise threshold.
       time:   [1.6682 s 1.6859 s 1.7039 s]
       thrpt:  [58.690 MiB/s 59.316 MiB/s 59.943 MiB/s]
change:
       time:   [+0.0887% +1.6030% +3.2145%] (p = 0.05 < 0.05)
       thrpt:  [-3.1144% -1.5777% -0.0886%]

Client/server transfer results

Transfer of 33554432 bytes over loopback.

Client Server CC Pacing MTU Mean [ms] Min [ms] Max [ms]
gquiche gquiche 1504 546.9 ± 74.8 511.9 758.5
neqo gquiche reno on 1504 771.1 ± 57.6 733.6 898.3
neqo gquiche reno 1504 753.0 ± 48.6 713.6 884.7
neqo gquiche cubic on 1504 764.7 ± 73.7 706.9 928.4
neqo gquiche cubic 1504 743.3 ± 62.4 693.5 898.9
msquic msquic 1504 197.1 ± 106.9 101.5 350.9
neqo msquic reno on 1504 240.5 ± 59.7 204.5 414.8
neqo msquic reno 1504 242.9 ± 61.2 208.8 420.7
neqo msquic cubic on 1504 260.5 ± 104.6 200.8 587.4
neqo msquic cubic 1504 275.2 ± 99.8 211.0 463.1
gquiche neqo reno on 1504 716.5 ± 107.0 560.8 903.8
gquiche neqo reno 1504 708.5 ± 108.0 560.5 886.2
gquiche neqo cubic on 1504 715.2 ± 122.2 556.3 971.3
gquiche neqo cubic 1504 713.4 ± 95.1 595.8 833.9
msquic neqo reno on 1504 495.7 ± 40.1 468.8 602.5
msquic neqo reno 1504 529.8 ± 63.5 465.2 615.2
msquic neqo cubic on 1504 518.9 ± 85.9 463.2 721.3
msquic neqo cubic 1504 510.4 ± 74.6 474.4 721.5
neqo neqo reno on 1504 534.3 ± 136.1 439.2 780.1
neqo neqo reno 1504 445.3 ± 15.1 427.1 467.2
neqo neqo cubic on 1504 487.0 ± 79.2 448.5 707.2
neqo neqo cubic 1504 470.2 ± 46.6 436.8 599.3

⬇️ Download logs

Signed-off-by: Lars Eggert <lars@eggert.org>
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.

Thank you for doing this!

@larseggert larseggert enabled auto-merge January 10, 2025 13:21
@larseggert larseggert disabled auto-merge January 10, 2025 14:40
@larseggert larseggert merged commit 085fa62 into mozilla:main Jan 10, 2025
64 of 67 checks passed
@larseggert larseggert deleted the fix-1963 branch January 10, 2025 15:07
mxinden added a commit to mxinden/neqo that referenced this pull request Jan 17, 2025
mozilla#2291 added additional formatting to our log
output via `env_logger`'s `auto-color` feature. The `auto-color` feature adds
the `is-terminal` crate dependency. `is-terminal` depends on `hermit-abi`
`v0.4`.

Pulling latest Neqo `v0.12.0` into mozilla-central adds `is-terminal` and
`hermit-abi` `v0.4.0` to the Firefox dependency tree.

In order to keep our dependency footprint low, I suggest not enabling the two
`env-logger` features.
github-merge-queue bot pushed a commit that referenced this pull request Jan 20, 2025
)

* deps(common/log): drop env_logger's auto-color and regex features

#2291 added additional formatting to our log
output via `env_logger`'s `auto-color` feature. The `auto-color` feature adds
the `is-terminal` crate dependency. `is-terminal` depends on `hermit-abi`
`v0.4`.

Pulling latest Neqo `v0.12.0` into mozilla-central adds `is-terminal` and
`hermit-abi` `v0.4.0` to the Firefox dependency tree.

In order to keep our dependency footprint low, I suggest not enabling the two
`env-logger` features.

* chore: bump version to v0.12.1
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace neqo_common::log with log crate
3 participants