Skip to content

Commit

Permalink
refactor(iroh-base): introduce an Arc into RelayUrl (#3065)
Browse files Browse the repository at this point in the history
## Description

This makes the `clone`ing of `RelayUrl` much cheaper. 
It turned out the whole code base was already using the `RelayUrl` as an
immutable type already, so no changes otherwise necessary.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
  • Loading branch information
dignifiedquire authored Dec 19, 2024
1 parent a4d4e7d commit 834ab78
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 9 deletions.
2 changes: 1 addition & 1 deletion iroh-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ derive_more = { version = "1.0.0", features = ["display"], optional = true }
url = { version = "2.5", features = ["serde"], optional = true }
postcard = { version = "1", default-features = false, features = ["alloc", "use-std", "experimental-derive"], optional = true }
rand_core = { version = "0.6.4", optional = true }
serde = { version = "1", features = ["derive"] }
serde = { version = "1", features = ["derive", "rc"] }
thiserror = { version = "2", optional = true }

# wasm
Expand Down
12 changes: 7 additions & 5 deletions iroh-base/src/relay_url.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use std::{fmt, ops::Deref, str::FromStr};
use std::{fmt, ops::Deref, str::FromStr, sync::Arc};

use serde::{Deserialize, Serialize};
use url::Url;

/// A URL identifying a relay server.
///
/// This is but a wrapper around [`Url`], with a few custom tweaks:
/// It is cheaply clonable, as the underlying type is wrapped into an `Arc`.
/// The main type under the hood though is [`Url`], with a few custom tweaks:
///
/// - A relay URL is never a relative URL, so an implicit `.` is added at the end of the
/// domain name if missing.
Expand All @@ -16,7 +18,7 @@ use url::Url;
#[derive(
Clone, derive_more::Display, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize,
)]
pub struct RelayUrl(Url);
pub struct RelayUrl(Arc<Url>);

impl From<Url> for RelayUrl {
fn from(mut url: Url) -> Self {
Expand All @@ -31,7 +33,7 @@ impl From<Url> for RelayUrl {
url.set_host(Some(&domain)).ok();
}
}
Self(url)
Self(Arc::new(url))
}
}

Expand All @@ -55,7 +57,7 @@ impl FromStr for RelayUrl {

impl From<RelayUrl> for Url {
fn from(value: RelayUrl) -> Self {
value.0
Arc::unwrap_or_clone(value.0)
}
}

Expand Down
6 changes: 3 additions & 3 deletions iroh-net-report/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ impl Actor {
self.handle_run_check(relay_map, opts, response_tx);
}
Message::ReportReady { report } => {
self.handle_report_ready(report);
self.handle_report_ready(*report);
}
Message::ReportAborted { err } => {
self.handle_report_aborted(err);
Expand Down Expand Up @@ -694,8 +694,8 @@ impl Actor {
});
}

fn handle_report_ready(&mut self, report: Box<Report>) {
let report = self.finish_and_store_report(*report);
fn handle_report_ready(&mut self, report: Report) {
let report = self.finish_and_store_report(report);
self.in_flight_stun_requests.clear();
if let Some(ReportRun { report_tx, .. }) = self.current_report_run.take() {
report_tx.send(Ok(report)).ok();
Expand Down

0 comments on commit 834ab78

Please sign in to comment.