From f892b8564fc3dc2e9c6db4453d8d8860c4a8ba65 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 27 Jun 2025 19:54:52 +0200 Subject: [PATCH] Return `Cow` from `UrlString::with_` methods (#14319) A minor performance improvement as a follow-up to #14245 (and an accompanying test). --- crates/uv-distribution-types/src/file.rs | 53 ++++++++++++++---------- crates/uv-resolver/src/lock/mod.rs | 8 +++- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/crates/uv-distribution-types/src/file.rs b/crates/uv-distribution-types/src/file.rs index 2d30eb0f2..948378c0c 100644 --- a/crates/uv-distribution-types/src/file.rs +++ b/crates/uv-distribution-types/src/file.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::fmt::{self, Display, Formatter}; use std::str::FromStr; @@ -160,27 +161,22 @@ impl UrlString { .unwrap_or(self.as_ref()) } - /// Return the [`UrlString`] with any fragments removed. + /// Return the [`UrlString`] (as a [`Cow`]) with any fragments removed. #[must_use] - pub fn without_fragment(&self) -> Self { - Self( - self.as_ref() - .split_once('#') - .map(|(path, _)| path) - .map(SmallString::from) - .unwrap_or_else(|| self.0.clone()), - ) + pub fn without_fragment(&self) -> Cow<'_, Self> { + self.as_ref() + .split_once('#') + .map(|(path, _)| Cow::Owned(UrlString(SmallString::from(path)))) + .unwrap_or(Cow::Borrowed(self)) } - /// Return the [`UrlString`] with trailing slash removed. + /// Return the [`UrlString`] (as a [`Cow`]) with trailing slash removed. #[must_use] - pub fn without_trailing_slash(&self) -> Self { - Self( - self.as_ref() - .strip_suffix('/') - .map(SmallString::from) - .unwrap_or_else(|| self.0.clone()), - ) + pub fn without_trailing_slash(&self) -> Cow<'_, Self> { + self.as_ref() + .strip_suffix('/') + .map(|path| Cow::Owned(UrlString(SmallString::from(path)))) + .unwrap_or(Cow::Borrowed(self)) } } @@ -263,16 +259,29 @@ mod tests { #[test] fn without_fragment() { + // Borrows a URL without a fragment + let url = UrlString("https://example.com/path".into()); + assert_eq!(url.without_fragment(), Cow::Borrowed(&url)); + + // Removes the fragment if present on the URL let url = UrlString("https://example.com/path?query#fragment".into()); assert_eq!( url.without_fragment(), - UrlString("https://example.com/path?query".into()) + Cow::Owned(UrlString("https://example.com/path?query".into())) ); + } - let url = UrlString("https://example.com/path#fragment".into()); - assert_eq!(url.base_str(), "https://example.com/path"); - + #[test] + fn without_trailing_slash() { + // Borrows a URL without a slash let url = UrlString("https://example.com/path".into()); - assert_eq!(url.base_str(), "https://example.com/path"); + assert_eq!(url.without_trailing_slash(), Cow::Borrowed(&url)); + + // Removes the trailing slash if present on the URL + let url = UrlString("https://example.com/path/".into()); + assert_eq!( + url.without_trailing_slash(), + Cow::Owned(UrlString("https://example.com/path".into())) + ); } } diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index 8ff3097de..beeadc912 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -1490,7 +1490,11 @@ impl Lock { .version .as_ref() .expect("version for registry source"); - return Ok(SatisfiesResult::MissingRemoteIndex(name, version, url)); + return Ok(SatisfiesResult::MissingRemoteIndex( + name, + version, + url.into_owned(), + )); } } RegistrySource::Path(path) => { @@ -4692,7 +4696,7 @@ impl From for Hashes { /// Convert a [`FileLocation`] into a normalized [`UrlString`]. fn normalize_file_location(location: &FileLocation) -> Result { match location { - FileLocation::AbsoluteUrl(absolute) => Ok(absolute.without_fragment()), + FileLocation::AbsoluteUrl(absolute) => Ok(absolute.without_fragment().into_owned()), FileLocation::RelativeUrl(_, _) => Ok(normalize_url(location.to_url()?)), } }