From 2525532ff047ab7e1e64546a3c05358252cdd229 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 19:35:04 +0000 Subject: [PATCH 1/6] move ActivationsKey and SemverCompatibility --- src/cargo/core/resolver/context.rs | 35 ++--------------------------- src/cargo/core/resolver/types.rs | 36 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 08c289d9f8b..720bf0ce1ca 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -1,13 +1,12 @@ use super::dep_cache::RegistryQueryer; use super::errors::ActivateResult; -use super::types::{ConflictMap, ConflictReason, FeaturesSet, ResolveOpts}; +use super::types::{ActivationsKey, ConflictMap, ConflictReason, FeaturesSet, ResolveOpts}; use super::RequestedFeatures; -use crate::core::{Dependency, PackageId, SourceId, Summary}; +use crate::core::{Dependency, PackageId, Summary}; use crate::util::interning::InternedString; use crate::util::Graph; use anyhow::format_err; use std::collections::HashMap; -use std::num::NonZeroU64; use tracing::debug; // A `Context` is basically a bunch of local resolution information which is @@ -39,39 +38,9 @@ pub type ContextAge = usize; /// By storing this in a hash map we ensure that there is only one /// semver compatible version of each crate. /// This all so stores the `ContextAge`. -pub type ActivationsKey = (InternedString, SourceId, SemverCompatibility); - pub type Activations = im_rc::HashMap; -/// A type that represents when cargo treats two Versions as compatible. -/// Versions `a` and `b` are compatible if their left-most nonzero digit is the -/// same. -#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] -pub enum SemverCompatibility { - Major(NonZeroU64), - Minor(NonZeroU64), - Patch(u64), -} - -impl From<&semver::Version> for SemverCompatibility { - fn from(ver: &semver::Version) -> Self { - if let Some(m) = NonZeroU64::new(ver.major) { - return SemverCompatibility::Major(m); - } - if let Some(m) = NonZeroU64::new(ver.minor) { - return SemverCompatibility::Minor(m); - } - SemverCompatibility::Patch(ver.patch) - } -} - -impl PackageId { - pub fn as_activations_key(self) -> ActivationsKey { - (self.name(), self.source_id(), self.version().into()) - } -} - impl ResolverContext { pub fn new() -> ResolverContext { ResolverContext { diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 2dc5d44ca33..86a1e132c31 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -1,10 +1,11 @@ use super::features::{CliFeatures, RequestedFeatures}; -use crate::core::{Dependency, PackageId, Summary}; +use crate::core::{Dependency, PackageId, SourceId, Summary}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::GlobalContext; use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; +use std::num::NonZeroU64; use std::rc::Rc; use std::time::{Duration, Instant}; @@ -163,6 +164,39 @@ impl ResolveOpts { } } +/// A key that when stord in a hash map ensures that there is only one +/// semver compatible version of each crate. +/// Find the activated version of a crate based on the name, source, and semver compatibility. +pub type ActivationsKey = (InternedString, SourceId, SemverCompatibility); + +/// A type that represents when cargo treats two Versions as compatible. +/// Versions `a` and `b` are compatible if their left-most nonzero digit is the +/// same. +#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] +pub enum SemverCompatibility { + Major(NonZeroU64), + Minor(NonZeroU64), + Patch(u64), +} + +impl From<&semver::Version> for SemverCompatibility { + fn from(ver: &semver::Version) -> Self { + if let Some(m) = NonZeroU64::new(ver.major) { + return SemverCompatibility::Major(m); + } + if let Some(m) = NonZeroU64::new(ver.minor) { + return SemverCompatibility::Minor(m); + } + SemverCompatibility::Patch(ver.patch) + } +} + +impl PackageId { + pub fn as_activations_key(self) -> ActivationsKey { + (self.name(), self.source_id(), self.version().into()) + } +} + #[derive(Clone)] pub struct DepsFrame { pub parent: Summary, From a6992390a94166eeb5865f63866b026c68fff37f Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 19:38:35 +0000 Subject: [PATCH 2/6] chagne ActivationsKey into a struct --- src/cargo/core/resolver/context.rs | 3 ++- src/cargo/core/resolver/types.rs | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 720bf0ce1ca..88fb4d341ea 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -106,7 +106,8 @@ impl ResolverContext { // versions came from a `[patch]` source. if let Some((_, dep)) = parent { if dep.source_id() != id.source_id() { - let key = (id.name(), dep.source_id(), id.version().into()); + let key = + ActivationsKey::new(id.name(), dep.source_id(), id.version().into()); let prev = self.activations.insert(key, (summary.clone(), age)); if let Some((previous_summary, _)) = prev { return Err( diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 86a1e132c31..c4597fb4e5a 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -167,7 +167,14 @@ impl ResolveOpts { /// A key that when stord in a hash map ensures that there is only one /// semver compatible version of each crate. /// Find the activated version of a crate based on the name, source, and semver compatibility. -pub type ActivationsKey = (InternedString, SourceId, SemverCompatibility); +#[derive(Clone, PartialEq, Eq, Debug, Ord, PartialOrd, Hash)] +pub struct ActivationsKey(InternedString, SourceId, SemverCompatibility); + +impl ActivationsKey { + pub fn new(name: InternedString, source_id: SourceId, ver: SemverCompatibility) -> ActivationsKey { + ActivationsKey(name, source_id, ver) + } +} /// A type that represents when cargo treats two Versions as compatible. /// Versions `a` and `b` are compatible if their left-most nonzero digit is the @@ -193,7 +200,7 @@ impl From<&semver::Version> for SemverCompatibility { impl PackageId { pub fn as_activations_key(self) -> ActivationsKey { - (self.name(), self.source_id(), self.version().into()) + ActivationsKey(self.name(), self.source_id(), self.version().into()) } } From f2b499847b3cbb702628bf74f48deac7d5c8186c Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 19:44:00 +0000 Subject: [PATCH 3/6] reorder ActivationKey so that the source is last The source is the field the most expensive to do Eq on and the one least likely to be different. By moving it to the end calles to comparing keys that are different returns faster. --- src/cargo/core/resolver/context.rs | 2 +- src/cargo/core/resolver/types.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 88fb4d341ea..d1bae02f8fa 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -107,7 +107,7 @@ impl ResolverContext { if let Some((_, dep)) = parent { if dep.source_id() != id.source_id() { let key = - ActivationsKey::new(id.name(), dep.source_id(), id.version().into()); + ActivationsKey::new(id.name(), id.version().into(), dep.source_id()); let prev = self.activations.insert(key, (summary.clone(), age)); if let Some((previous_summary, _)) = prev { return Err( diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index c4597fb4e5a..1758b81045f 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -168,11 +168,11 @@ impl ResolveOpts { /// semver compatible version of each crate. /// Find the activated version of a crate based on the name, source, and semver compatibility. #[derive(Clone, PartialEq, Eq, Debug, Ord, PartialOrd, Hash)] -pub struct ActivationsKey(InternedString, SourceId, SemverCompatibility); +pub struct ActivationsKey(InternedString, SemverCompatibility, SourceId); impl ActivationsKey { - pub fn new(name: InternedString, source_id: SourceId, ver: SemverCompatibility) -> ActivationsKey { - ActivationsKey(name, source_id, ver) + pub fn new(name: InternedString, ver: SemverCompatibility, source_id: SourceId) -> ActivationsKey { + ActivationsKey(name, ver, source_id) } } @@ -200,7 +200,7 @@ impl From<&semver::Version> for SemverCompatibility { impl PackageId { pub fn as_activations_key(self) -> ActivationsKey { - ActivationsKey(self.name(), self.source_id(), self.version().into()) + ActivationsKey(self.name(), self.version().into(), self.source_id()) } } From 91e6bf8b434c4ad5d7588d75d6a36fef8bae0c94 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 19:45:02 +0000 Subject: [PATCH 4/6] a faster hash for ActivationsKey --- src/cargo/core/resolver/types.rs | 10 +++++++++- src/cargo/util/interning.rs | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 1758b81045f..4cce53f3280 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -167,7 +167,7 @@ impl ResolveOpts { /// A key that when stord in a hash map ensures that there is only one /// semver compatible version of each crate. /// Find the activated version of a crate based on the name, source, and semver compatibility. -#[derive(Clone, PartialEq, Eq, Debug, Ord, PartialOrd, Hash)] +#[derive(Clone, PartialEq, Eq, Debug, Ord, PartialOrd)] pub struct ActivationsKey(InternedString, SemverCompatibility, SourceId); impl ActivationsKey { @@ -176,6 +176,14 @@ impl ActivationsKey { } } +impl std::hash::Hash for ActivationsKey { + fn hash(&self, state: &mut H) { + self.0.fast_hash(state); + self.1.hash(state); + // self.2.hash(state); // Packages that only differ by SourceId are rare enough to not be worth hashing + } +} + /// A type that represents when cargo treats two Versions as compatible. /// Versions `a` and `b` are compatible if their left-most nonzero digit is the /// same. diff --git a/src/cargo/util/interning.rs b/src/cargo/util/interning.rs index 595968eb654..185324d6b9a 100644 --- a/src/cargo/util/interning.rs +++ b/src/cargo/util/interning.rs @@ -90,6 +90,12 @@ impl InternedString { pub fn as_str(&self) -> &'static str { self.inner } + + /// A faster implementation of hash that is completely compatible with HashMap, + /// but does not have a stable value between runs of the program. + pub fn fast_hash(&self, state: &mut H) { + std::ptr::NonNull::from(self.inner).hash(state); + } } impl Deref for InternedString { From 7a67e88baf0f73196973cadf887bf9a31c13e95f Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 19:54:25 +0000 Subject: [PATCH 5/6] cargo fmt --- src/cargo/core/resolver/types.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 4cce53f3280..8dafd5d94fc 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -171,7 +171,11 @@ impl ResolveOpts { pub struct ActivationsKey(InternedString, SemverCompatibility, SourceId); impl ActivationsKey { - pub fn new(name: InternedString, ver: SemverCompatibility, source_id: SourceId) -> ActivationsKey { + pub fn new( + name: InternedString, + ver: SemverCompatibility, + source_id: SourceId, + ) -> ActivationsKey { ActivationsKey(name, ver, source_id) } } From 2cafa2876cb8f197ffb29571f34fd5e02ef3fb27 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 10 Dec 2024 21:18:04 +0000 Subject: [PATCH 6/6] oddly this is fasster --- src/cargo/core/resolver/types.rs | 2 +- src/cargo/util/interning.rs | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 8dafd5d94fc..2248d79bbf3 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -182,7 +182,7 @@ impl ActivationsKey { impl std::hash::Hash for ActivationsKey { fn hash(&self, state: &mut H) { - self.0.fast_hash(state); + self.0.hash(state); self.1.hash(state); // self.2.hash(state); // Packages that only differ by SourceId are rare enough to not be worth hashing } diff --git a/src/cargo/util/interning.rs b/src/cargo/util/interning.rs index 185324d6b9a..595968eb654 100644 --- a/src/cargo/util/interning.rs +++ b/src/cargo/util/interning.rs @@ -90,12 +90,6 @@ impl InternedString { pub fn as_str(&self) -> &'static str { self.inner } - - /// A faster implementation of hash that is completely compatible with HashMap, - /// but does not have a stable value between runs of the program. - pub fn fast_hash(&self, state: &mut H) { - std::ptr::NonNull::from(self.inner).hash(state); - } } impl Deref for InternedString {