From 3d91b795413e0c375c3df7e3f0c0eb6a2464ef67 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 18 Nov 2018 21:06:15 +0000 Subject: [PATCH 1/4] Intern SourceId --- src/cargo/core/source/source_id.rs | 45 ++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index f30d18ffd05..6b640aefe73 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -1,8 +1,9 @@ use std::cmp::{self, Ordering}; +use std::collections::HashSet; use std::fmt::{self, Formatter}; use std::hash::{self, Hash}; use std::path::Path; -use std::sync::Arc; +use std::sync::Mutex; use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT}; use std::sync::atomic::Ordering::SeqCst; @@ -16,13 +17,17 @@ use sources::{GitSource, PathSource, RegistrySource, CRATES_IO_INDEX}; use sources::DirectorySource; use util::{CargoResult, Config, ToUrl}; +lazy_static! { + static ref SOURCE_ID_CACHE: Mutex> = Mutex::new(HashSet::new()); +} + /// Unique identifier for a source of packages. #[derive(Clone, Eq, Debug)] pub struct SourceId { - inner: Arc, + inner: &'static SourceIdInner, } -#[derive(Eq, Clone, Debug)] +#[derive(Eq, Clone, Debug, Hash)] struct SourceIdInner { /// The source URL url: Url, @@ -68,18 +73,28 @@ impl SourceId { /// /// The canonical url will be calculated, but the precise field will not fn new(kind: Kind, url: Url) -> CargoResult { - let source_id = SourceId { - inner: Arc::new(SourceIdInner { + let source_id = SourceId::wrap( + SourceIdInner { kind, canonical_url: git::canonicalize_url(&url)?, url, precise: None, name: None, - }), - }; + } + ); Ok(source_id) } + fn wrap(inner: SourceIdInner) -> SourceId { + let mut cache = SOURCE_ID_CACHE.lock().unwrap(); + let inner = cache.get(&inner).map(|&x| x).unwrap_or_else(|| { + let inner = Box::leak(Box::new(inner)); + cache.insert(inner); + inner + }); + SourceId { inner } + } + /// Parses a source URL and returns the corresponding ID. /// /// ## Example @@ -193,15 +208,15 @@ impl SourceId { pub fn alt_registry(config: &Config, key: &str) -> CargoResult { let url = config.get_registry_index(key)?; - Ok(SourceId { - inner: Arc::new(SourceIdInner { + Ok(SourceId::wrap( + SourceIdInner { kind: Kind::Registry, canonical_url: git::canonicalize_url(&url)?, url, precise: None, name: Some(key.to_string()), - }), - }) + } + )) } /// Get this source URL @@ -288,12 +303,12 @@ impl SourceId { /// Create a new SourceId from this source with the given `precise` pub fn with_precise(&self, v: Option) -> SourceId { - SourceId { - inner: Arc::new(SourceIdInner { + SourceId::wrap( + SourceIdInner { precise: v, ..(*self.inner).clone() - }), - } + } + ) } /// Whether the remote registry is the standard https://crates.io From 653ce4d4c37766b0aa1858c25ae6fd63c63cbc13 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 12:14:56 +0000 Subject: [PATCH 2/4] Move the custom equality up to SourceId That way we: * preserve that SourceIds that have slightly different URLs are actually equal * make SourceIdInner lawful in its Hash/Eq definition. --- src/cargo/core/source/source_id.rs | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 6b640aefe73..3b54003dcaa 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -27,7 +27,7 @@ pub struct SourceId { inner: &'static SourceIdInner, } -#[derive(Eq, Clone, Debug, Hash)] +#[derive(PartialEq, Eq, Clone, Debug, Hash)] struct SourceIdInner { /// The source URL url: Url, @@ -341,12 +341,6 @@ impl SourceId { } } -impl PartialEq for SourceId { - fn eq(&self, other: &SourceId) -> bool { - (*self.inner).eq(&*other.inner) - } -} - impl PartialOrd for SourceId { fn partial_cmp(&self, other: &SourceId) -> Option { Some(self.cmp(other)) @@ -440,24 +434,20 @@ impl fmt::Display for SourceId { } } -// This custom implementation handles situations such as when two git sources -// point at *almost* the same URL, but not quite, even when they actually point -// to the same repository. -/// This method tests for self and other values to be equal, and is used by ==. -/// -/// For git repositories, the canonical url is checked. -impl PartialEq for SourceIdInner { - fn eq(&self, other: &SourceIdInner) -> bool { - if self.kind != other.kind { +// Custom equality defined as canonical URL equality for git sources and +// URL equality for other sources, ignoring the `precise` and `name` fields. +impl PartialEq for SourceId { + fn eq(&self, other: &SourceId) -> bool { + if self.inner.kind != other.inner.kind { return false; } - if self.url == other.url { + if self.inner.url == other.inner.url { return true; } - match (&self.kind, &other.kind) { - (&Kind::Git(ref ref1), &Kind::Git(ref ref2)) => { - ref1 == ref2 && self.canonical_url == other.canonical_url + match (&self.inner.kind, &other.inner.kind) { + (Kind::Git(ref1), Kind::Git(ref2)) => { + ref1 == ref2 && self.inner.canonical_url == other.inner.canonical_url } _ => false, } From ba175dcc4da4a3fef0a872cf972a6ef722493d19 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 12:23:45 +0000 Subject: [PATCH 3/4] Add pointer equality to PartialEq for SourceId --- src/cargo/core/source/source_id.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 3b54003dcaa..63eb8773e89 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::fmt::{self, Formatter}; use std::hash::{self, Hash}; use std::path::Path; +use std::ptr; use std::sync::Mutex; use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT}; use std::sync::atomic::Ordering::SeqCst; @@ -438,6 +439,9 @@ impl fmt::Display for SourceId { // URL equality for other sources, ignoring the `precise` and `name` fields. impl PartialEq for SourceId { fn eq(&self, other: &SourceId) -> bool { + if ptr::eq(self.inner, other.inner) { + return true; + } if self.inner.kind != other.inner.kind { return false; } From 9b22eb1998752dbaccbd125c74f263e0ea6f7c3b Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 25 Nov 2018 13:00:34 +0000 Subject: [PATCH 4/4] Make SourceId derive Copy --- src/cargo/core/source/source_id.rs | 2 +- src/cargo/ops/resolve.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 63eb8773e89..47e8ef546f1 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -23,7 +23,7 @@ lazy_static! { } /// Unique identifier for a source of packages. -#[derive(Clone, Eq, Debug)] +#[derive(Clone, Copy, Eq, Debug)] pub struct SourceId { inner: &'static SourceIdInner, } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index da01a776318..275fbe752a4 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -150,7 +150,7 @@ pub fn resolve_with_previous<'a, 'cfg>( // // TODO: This seems like a hokey reason to single out the registry as being // different - let mut to_avoid_sources = HashSet::new(); + let mut to_avoid_sources: HashSet<&SourceId> = HashSet::new(); if let Some(to_avoid) = to_avoid { to_avoid_sources.extend( to_avoid @@ -161,7 +161,7 @@ pub fn resolve_with_previous<'a, 'cfg>( } let keep = |p: &&'a PackageId| { - !to_avoid_sources.contains(&p.source_id()) && match to_avoid { + !to_avoid_sources.contains(p.source_id()) && match to_avoid { Some(set) => !set.contains(p), None => true, }