refactor(noq)!: Use FourTuple in open_path#661
Conversation
Frando
force-pushed
the
Frando/open-path-four-tuple
branch
from
d35bf99 to
860c688
Compare
Performance Comparison Report
|
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5393.0 Mbps | 7781.9 Mbps | -30.7% | 91.6% / 96.9% |
| medium-concurrent | 5549.3 Mbps | 7482.4 Mbps | -25.8% | 97.4% / 149.0% |
| medium-single | 3854.6 Mbps | 4749.2 Mbps | -18.8% | 93.5% / 101.0% |
| small-concurrent | 3883.7 Mbps | 5362.0 Mbps | -27.6% | 95.9% / 104.0% |
| small-single | 3567.2 Mbps | 4814.2 Mbps | -25.9% | 89.5% / 97.5% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 2982.9 Mbps | 4064.8 Mbps | -26.6% |
| lan | 782.4 Mbps | 810.3 Mbps | -3.4% |
| lossy | 69.8 Mbps | 56.5 Mbps | +23.5% |
| wan | 83.8 Mbps | 83.8 Mbps | ~0% |
Summary
noq is 25.7% slower on average
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/661/docs/noq/ Last updated: 2026-05-19T13:33:46Z |
flub
left a comment
flub
left a comment
There was a problem hiding this comment.
I'm half-tempted to make the public functions take impl Into<FourTuple> and than impl that for SocketAddr. But I'm fine with current version too.
| Ok(network_path) | ||
| } | ||
|
|
||
| impl Connection { |
There was a problem hiding this comment.
Erm, you close the impl block and than open it again? That's pretty unusual... I've grown to be not a fan of multiple impl blocks. Can you move this to after the end of the block entirely? I'd also accept it as a pub(crate) method on FourTuple, which would probably force you to not abuse the PathError and instead have to map it at the usage point 😉
2 participants
Description
This changes
noq::Connection::open_pathandnoq::Connection::open_path_ensureto take aFourTupleinstead of aSocketAddr. This lets callers specify the local IP for the new path, which previously could only be discovered after the path was opened.The proto-side API is unchanged; both
noq-proto::Connection::open_pathandnoq-proto::Connection::open_path_ensurealready took aFourTuple.Breaking Changes
noq::Connection::open_pathnow takes aFourTupleargument. UseFourTuple::from_remote(socket_addr)for previous behavior.noq::Connection::open_path_ensurenow takes aFourTupleargument. UseFourTuple::from_remote(socket_addr)for previous behavior.Change checklist