-
Notifications
You must be signed in to change notification settings - Fork 126
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
chore(transport): Remove module_name_repetitions
#2311
chore(transport): Remove module_name_repetitions
#2311
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2311 +/- ##
=======================================
Coverage 93.34% 93.34%
=======================================
Files 114 114
Lines 36906 36908 +2
Branches 36906 36908 +2
=======================================
+ Hits 34449 34451 +2
Misses 1675 1675
Partials 782 782 ☔ View full report in Codecov by Sentry. |
I would suggest to do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than missing ecn::
in a few places outside of neqo-transport/src/ecn.rs
(and its tests) LGTM.
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to b17233c. neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Benchmark resultsPerformance differences relative to b17233c. decode 4096 bytes, mask ff: No change in performance detected.time: [11.176 µs 11.202 µs 11.234 µs] change: [-0.7199% -0.2629% +0.1777%] (p = 0.26 > 0.05) decode 1048576 bytes, mask ff: No change in performance detected.time: [2.8902 ms 2.9011 ms 2.9127 ms] change: [-0.2484% +0.2491% +0.7782%] (p = 0.34 > 0.05) decode 4096 bytes, mask 7f: No change in performance detected.time: [19.532 µs 19.584 µs 19.646 µs] change: [-0.2538% +0.0827% +0.4472%] (p = 0.66 > 0.05) decode 1048576 bytes, mask 7f: No change in performance detected.time: [5.0664 ms 5.0776 ms 5.0903 ms] change: [-0.4406% -0.0888% +0.2504%] (p = 0.61 > 0.05) decode 4096 bytes, mask 3f: No change in performance detected.time: [5.5290 µs 5.5439 µs 5.5649 µs] change: [-0.5366% +0.0581% +0.7210%] (p = 0.86 > 0.05) decode 1048576 bytes, mask 3f: No change in performance detected.time: [1.4151 ms 1.4207 ms 1.4277 ms] change: [-0.3941% +0.1903% +0.8659%] (p = 0.54 > 0.05) coalesce_acked_from_zero 1+1 entries: No change in performance detected.time: [98.242 ns 98.558 ns 98.882 ns] change: [-0.8203% -0.3979% -0.0095%] (p = 0.06 > 0.05) coalesce_acked_from_zero 3+1 entries: No change in performance detected.time: [116.30 ns 116.59 ns 116.91 ns] change: [-1.0380% -0.4662% +0.0063%] (p = 0.08 > 0.05) coalesce_acked_from_zero 10+1 entries: No change in performance detected.time: [116.24 ns 116.77 ns 117.38 ns] change: [-1.4518% -0.4643% +0.3273%] (p = 0.35 > 0.05) coalesce_acked_from_zero 1000+1 entries: No change in performance detected.time: [97.189 ns 97.396 ns 97.631 ns] change: [-1.6000% -0.5427% +0.4168%] (p = 0.33 > 0.05) RxStreamOrderer::inbound_frame(): No change in performance detected.time: [111.22 ms 111.28 ms 111.35 ms] change: [-0.0392% +0.0398% +0.1184%] (p = 0.33 > 0.05) SentPackets::take_ranges: No change in performance detected.time: [5.4973 µs 5.6744 µs 5.8651 µs] change: [-0.8948% +1.7890% +4.5559%] (p = 0.19 > 0.05) transfer/pacing-false/varying-seeds: Change within noise threshold.time: [42.645 ms 42.719 ms 42.794 ms] change: [-2.5753% -2.3268% -2.0772%] (p = 0.00 < 0.05) transfer/pacing-true/varying-seeds: Change within noise threshold.time: [43.085 ms 43.163 ms 43.239 ms] change: [-1.9060% -1.6568% -1.4175%] (p = 0.00 < 0.05) transfer/pacing-false/same-seed: Change within noise threshold.time: [42.568 ms 42.629 ms 42.690 ms] change: [-1.9172% -1.6960% -1.4687%] (p = 0.00 < 0.05) transfer/pacing-true/same-seed: Change within noise threshold.time: [43.187 ms 43.261 ms 43.335 ms] change: [-1.3239% -1.0901% -0.8666%] (p = 0.00 < 0.05) 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: Change within noise threshold.time: [896.63 ms 906.49 ms 916.67 ms] thrpt: [109.09 MiB/s 110.32 MiB/s 111.53 MiB/s] change: time: [+0.2103% +1.7004% +3.2273%] (p = 0.03 < 0.05) thrpt: [-3.1264% -1.6720% -0.2099%] 1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: Change within noise threshold.time: [300.57 ms 302.61 ms 304.65 ms] thrpt: [32.825 Kelem/s 33.045 Kelem/s 33.270 Kelem/s] change: time: [+0.0684% +1.0415% +2.0540%] (p = 0.04 < 0.05) thrpt: [-2.0126% -1.0307% -0.0683%] 1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.time: [34.261 ms 34.435 ms 34.625 ms] thrpt: [28.881 elem/s 29.040 elem/s 29.188 elem/s] change: time: [-1.0502% -0.1923% +0.6160%] (p = 0.66 > 0.05) thrpt: [-0.6123% +0.1927% +1.0613%] 1-conn/1-100mb-resp/mtu-1504 (aka. Upload)/client: No change in performance detected.time: [1.6473 s 1.6661 s 1.6852 s] thrpt: [59.341 MiB/s 60.020 MiB/s 60.705 MiB/s] change: time: [-0.2615% +1.2702% +2.8625%] (p = 0.11 > 0.05) thrpt: [-2.7828% -1.2543% +0.2621%] Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
This pull-request does two things:
module_name_repetitions
inneqo-transport/src/ecn.rs
neqo-transport
crate for refactoring by removing the crate-wide#![allow(clippy::module_name_repetitions)]
and adding them on a module-level where necessaryOne thing I'd like some feedback on is that because we are explicitly only importing the things we need from a module (e.g.
use ecn::Info;
) it's not always as clear where things come from as it was described in #2284 (comment). See for example inneqo-transport/src/path.rs
line 203:Should we rather import like
use ecn;
or is it fine to leave like this?Related: #2284