fix! make Path::close completely sync (this never returned in the happy case)#419
fix! make Path::close completely sync (this never returned in the happy case)#419divagant-martian merged 5 commits intomainfrom
Path::close completely sync (this never returned in the happy case)#419Conversation
Performance Comparison Report
|
| Scenario | iroh-quinn | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5395.5 Mbps | 7898.5 Mbps | -31.7% | 95.7% / 101.0% |
| medium-concurrent | 5456.2 Mbps | 7463.7 Mbps | -26.9% | 95.3% / 102.0% |
| medium-single | 3819.7 Mbps | 4190.1 Mbps | -8.8% | 98.5% / 153.0% |
| small-concurrent | 3754.2 Mbps | 5121.1 Mbps | -26.7% | 91.3% / 99.5% |
| small-single | 3428.4 Mbps | 4380.7 Mbps | -21.7% | 94.9% / 103.0% |
Netsim Benchmarks (network simulation)
| Condition | iroh-quinn | upstream | Delta |
|---|---|---|---|
| ideal | 3114.5 Mbps | 3956.8 Mbps | -21.3% |
| lan | 782.4 Mbps | 810.4 Mbps | -3.4% |
| lossy | 69.8 Mbps | 55.9 Mbps | +25.0% |
| wan | 83.8 Mbps | 83.8 Mbps | ~0% |
Summary
iroh-quinn is 23.7% slower on average
c109d6754dce8ae191064fff8f87b3f117172b51 - artifacts
Raw Benchmarks (localhost)
| Scenario | iroh-quinn | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5933.3 Mbps | 7885.9 Mbps | -24.8% | 97.3% / 99.0% |
| medium-concurrent | 5603.1 Mbps | 7740.2 Mbps | -27.6% | 95.5% / 97.4% |
| medium-single | 4005.5 Mbps | 4595.4 Mbps | -12.8% | 96.4% / 98.8% |
| small-concurrent | 3660.6 Mbps | 5199.7 Mbps | -29.6% | 96.5% / 98.8% |
| small-single | 3592.5 Mbps | 4752.7 Mbps | -24.4% | 96.5% / 98.7% |
Netsim Benchmarks (network simulation)
| Condition | iroh-quinn | upstream | Delta |
|---|---|---|---|
| ideal | N/A | 3645.6 Mbps | N/A |
| lan | N/A | 796.4 Mbps | N/A |
| lossy | N/A | 69.8 Mbps | N/A |
| wan | N/A | 83.8 Mbps | N/A |
Summary
iroh-quinn is 24.5% slower on average
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/quinn/pr/419/docs/iroh_quinn/ Last updated: 2026-02-10T15:41:05Z |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #419 +/- ##
==========================================
+ Coverage 76.98% 77.24% +0.25%
==========================================
Files 81 81
Lines 23160 23142 -18
==========================================
+ Hits 17830 17875 +45
+ Misses 5330 5267 -63 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I agree about making |
flub
left a comment
There was a problem hiding this comment.
Yeah, fair. This was an attempt to start building an API that didn't require the PathEvents, but I guess we can do this better once we really tackle that. The fact that iroh currently uses the PathEvent already anyway is the main reason this is unused for now.
…appy case) (#419) * remove PathEvent::Closed * add test * remove unused public api types * spelling * make test finish reliably
Description
Closes #418
The pubcli Api
Path::closedepends on the eventPathEvent::Closedbeingemitted. This was never the case, so application callers never actually got
access to the error. There were two options here, fixing this and emitting the
event, or removing it.
I opted for removal because the fact that the path state is kept for some time,
and that this is different from closing the path, is very low into the lore of
multipath and does not really provide the user any useful information. Second
argument is that this seems like it was never actually used, as no users have
complained about this broken api. Users that used this call can relly on events
to know when the path has been closed.
In the unexpecte (to me) case that this API is restored, we should do so using
the
Abandonedevent instead. I don't think we should do this unless we findan use case in which this api, vs events is meaningully more convenient or
useful.
A test is added for the basic functionality of
Path::closeBreaking Changes
PathEvent::Closedinprotohas been removed. UsePathEvent::Abandonedinstead.
Path::closeinquinnno longer has a return value other than(), toknow when a path has been closed use the events api instead.
ClosePathis gone. Refer to the above itemNotes & open questions
n/a