From 9aa617dcfb08b10686f7484cecd7062d8c834694 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 16:59:51 +0200 Subject: [PATCH 1/7] perf: cache candidates seperate from their sorting --- crates/libsolv_rs/src/conda_util.rs | 123 +++++++++++++++++-------- crates/libsolv_rs/src/pool.rs | 25 +++-- crates/libsolv_rs/src/problem.rs | 2 +- crates/libsolv_rs/src/solver/clause.rs | 4 +- crates/libsolv_rs/src/solver/mod.rs | 36 +++++--- 5 files changed, 130 insertions(+), 60 deletions(-) diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index e7db6b6b0..88300673c 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -4,6 +4,7 @@ use crate::mapping::Mapping; use crate::solvable::Solvable; use crate::MatchSpecId; use rattler_conda_types::{MatchSpec, Version}; +use std::cell::{OnceCell}; use std::cmp::Ordering; use std::collections::HashMap; @@ -15,6 +16,7 @@ pub(crate) fn compare_candidates( interned_strings: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, + match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, ) -> Ordering { let a_solvable = solvables[a].package(); let b_solvable = solvables[b].package(); @@ -48,28 +50,46 @@ pub(crate) fn compare_candidates( // Otherwise, compare the dependencies of the variants. If there are similar // dependencies select the variant that selects the highest version of the dependency. - let a_match_specs = a_solvable.dependencies.iter().map(|id| &match_specs[*id]); - let b_match_specs = b_solvable.dependencies.iter().map(|id| &match_specs[*id]); + let a_match_specs = a_solvable + .dependencies + .iter() + .map(|id| (*id, &match_specs[*id])); + let b_match_specs = b_solvable + .dependencies + .iter() + .map(|id| (*id, &match_specs[*id])); let b_specs_by_name: HashMap<_, _> = b_match_specs - .filter_map(|spec| spec.name.as_ref().map(|name| (name, spec))) + .filter_map(|(spec_id, spec)| spec.name.as_ref().map(|name| (name, (spec_id)))) .collect(); - let a_specs_by_name = - a_match_specs.filter_map(|spec| spec.name.as_ref().map(|name| (name, spec))); + let a_specs_by_name = a_match_specs + .filter_map(|(spec_id, spec)| spec.name.as_ref().map(|name| (name, (spec_id)))); let mut total_score = 0; - for (a_dep_name, a_spec) in a_specs_by_name { - if let Some(b_spec) = b_specs_by_name.get(&a_dep_name) { - if &a_spec == b_spec { + for (a_dep_name, a_spec_id) in a_specs_by_name { + if let Some(b_spec_id) = b_specs_by_name.get(&a_dep_name) { + if &a_spec_id == b_spec_id { continue; } // Find which of the two specs selects the highest version - let highest_a = - find_highest_version(solvables, interned_strings, packages_by_name, a_spec); - let highest_b = - find_highest_version(solvables, interned_strings, packages_by_name, b_spec); + let highest_a = find_highest_version( + a_spec_id, + solvables, + interned_strings, + packages_by_name, + match_specs, + match_spec_to_candidates, + ); + let highest_b = find_highest_version( + *b_spec_id, + solvables, + interned_strings, + packages_by_name, + match_specs, + match_spec_to_candidates, + ); // Skip version if no package is selected by either spec let (a_version, a_tracked_features, b_version, b_tracked_features) = if let ( @@ -114,34 +134,65 @@ pub(crate) fn compare_candidates( } pub(crate) fn find_highest_version( + match_spec_id: MatchSpecId, solvables: &Arena<SolvableId, Solvable>, interned_strings: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, - match_spec: &MatchSpec, + match_specs: &Arena<MatchSpecId, MatchSpec>, + match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, ) -> Option<(Version, bool)> { - let name = match_spec.name.as_deref().unwrap(); - let name_id = interned_strings[name]; - - // For each record that matches the spec - let candidates = packages_by_name[name_id] + let candidates = find_candidates( + match_spec_id, + match_specs, + interned_strings, + packages_by_name, + solvables, + match_spec_to_candidates, + ); + + candidates .iter() - .map(|&s| solvables[s].package().record) - .filter(|s| match_spec.matches(s)); - - candidates.fold(None, |init, record| { - Some(init.map_or_else( - || { - ( - record.version.version().clone(), - !record.track_features.is_empty(), - ) - }, - |(version, has_tracked_features)| { - ( - version.max(record.version.version().clone()), - has_tracked_features && record.track_features.is_empty(), - ) - }, - )) + .map(|id| &solvables[*id].package().record) + .fold(None, |init, record| { + Some(init.map_or_else( + || { + ( + record.version.version().clone(), + !record.track_features.is_empty(), + ) + }, + |(version, has_tracked_features)| { + ( + version.max(record.version.version().clone()), + has_tracked_features && record.track_features.is_empty(), + ) + }, + )) + }) +} + +pub(crate) fn find_candidates<'a, 'b>( + match_spec_id: MatchSpecId, + match_specs: &Arena<MatchSpecId, MatchSpec>, + names_to_ids: &HashMap<String, NameId>, + packages_by_name: &Mapping<NameId, Vec<SolvableId>>, + solvables: &Arena<SolvableId, Solvable<'a>>, + match_spec_to_candidates: &'b Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, +) -> &'b Vec<SolvableId> { + match_spec_to_candidates[match_spec_id].get_or_init(|| { + let match_spec = &match_specs[match_spec_id]; + let match_spec_name = match_spec + .name + .as_deref() + .expect("match spec without name!"); + let name_id = names_to_ids + .get(match_spec_name) + .expect("cannot find name in name lookup"); + + packages_by_name[*name_id] + .iter() + .cloned() + .filter(|&solvable| match_spec.matches(solvables[solvable].package().record)) + .collect() }) } diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index cea82fb28..251924e3b 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -1,3 +1,4 @@ +use std::cell::{OnceCell}; use crate::arena::Arena; use crate::conda_util; use crate::id::{MatchSpecId, NameId, RepoId, SolvableId}; @@ -35,7 +36,7 @@ pub struct Pool<'a> { match_specs_to_ids: HashMap<String, MatchSpecId>, /// Cached candidates for each match spec - pub(crate) match_spec_to_candidates: Mapping<MatchSpecId, Vec<SolvableId>>, + pub(crate) match_spec_to_sorted_candidates: Mapping<MatchSpecId, Vec<SolvableId>>, /// Cached forbidden solvables for each match spec pub(crate) match_spec_to_forbidden: Mapping<MatchSpecId, Vec<SolvableId>>, @@ -56,7 +57,7 @@ impl<'a> Default for Pool<'a> { match_specs_to_ids: HashMap::default(), match_specs: Arena::new(), - match_spec_to_candidates: Mapping::empty(), + match_spec_to_sorted_candidates: Mapping::empty(), match_spec_to_forbidden: Mapping::empty(), } } @@ -127,7 +128,8 @@ impl<'a> Pool<'a> { &self, match_spec_id: MatchSpecId, favored_map: &HashMap<NameId, SolvableId>, - match_spec_to_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, + match_spec_to_sorted_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, + match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec @@ -139,11 +141,15 @@ impl<'a> Pool<'a> { Some(&name_id) => name_id, }; - let mut pkgs: Vec<_> = self.packages_by_name[name_id] - .iter() - .cloned() - .filter(|&solvable| match_spec.matches(self.solvables[solvable].package().record)) - .collect(); + let mut pkgs = conda_util::find_candidates( + match_spec_id, + &self.match_specs, + &self.names_to_ids, + &self.packages_by_name, + &self.solvables, + match_spec_to_candidates, + ) + .clone(); pkgs.sort_by(|&p1, &p2| { conda_util::compare_candidates( @@ -153,6 +159,7 @@ impl<'a> Pool<'a> { &self.names_to_ids, &self.packages_by_name, &self.match_specs, + match_spec_to_candidates, ) }); @@ -163,7 +170,7 @@ impl<'a> Pool<'a> { } } - match_spec_to_candidates[match_spec_id] = pkgs; + match_spec_to_sorted_candidates[match_spec_id] = pkgs; } /// Populates the list of forbidden packages for the provided match spec diff --git a/crates/libsolv_rs/src/problem.rs b/crates/libsolv_rs/src/problem.rs index 2edf11b9d..d8c1f89cb 100644 --- a/crates/libsolv_rs/src/problem.rs +++ b/crates/libsolv_rs/src/problem.rs @@ -52,7 +52,7 @@ impl Problem { Clause::Requires(package_id, match_spec_id) => { let package_node = Self::add_node(&mut graph, &mut nodes, package_id); - let candidates = &solver.pool().match_spec_to_candidates[match_spec_id]; + let candidates = &solver.pool().match_spec_to_sorted_candidates[match_spec_id]; if candidates.is_empty() { tracing::info!( "{package_id:?} requires {match_spec_id:?}, which has no candidates" diff --git a/crates/libsolv_rs/src/solver/clause.rs b/crates/libsolv_rs/src/solver/clause.rs index b913dc37d..e1e2b9990 100644 --- a/crates/libsolv_rs/src/solver/clause.rs +++ b/crates/libsolv_rs/src/solver/clause.rs @@ -130,7 +130,7 @@ impl Clause { negate: true, }); - for &solvable_id in &pool.match_spec_to_candidates[match_spec_id] { + for &solvable_id in &pool.match_spec_to_sorted_candidates[match_spec_id] { visit(Literal { solvable_id, negate: false, @@ -345,7 +345,7 @@ impl ClauseState { } // The available candidates - for &candidate in &pool.match_spec_to_candidates[match_spec_id] { + for &candidate in &pool.match_spec_to_sorted_candidates[match_spec_id] { let lit = Literal { solvable_id: candidate, negate: false, diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index 4c3fffa32..efad8f2e6 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -7,6 +7,7 @@ use crate::problem::Problem; use crate::solvable::SolvableInner; use crate::solve_jobs::SolveJobs; use crate::transaction::Transaction; +use std::cell::{OnceCell}; use itertools::Itertools; use rattler_conda_types::MatchSpec; @@ -72,7 +73,7 @@ impl<'a> Solver<'a> { self.clauses = vec![ClauseState::new( Clause::InstallRoot, &self.learnt_clauses, - &self.pool.match_spec_to_candidates, + &self.pool.match_spec_to_sorted_candidates, )]; // Favored map @@ -99,7 +100,7 @@ impl<'a> Solver<'a> { self.clauses.push(ClauseState::new( Clause::ForbidMultipleInstances(candidate, other_candidate), &self.learnt_clauses, - &self.pool.match_spec_to_candidates, + &self.pool.match_spec_to_sorted_candidates, )); } } @@ -114,7 +115,7 @@ impl<'a> Solver<'a> { self.clauses.push(ClauseState::new( Clause::Lock(locked_solvable_id, other_candidate), &self.learnt_clauses, - &self.pool.match_spec_to_candidates, + &self.pool.match_spec_to_sorted_candidates, )); } } @@ -162,10 +163,14 @@ impl<'a> Solver<'a> { stack.push(SolvableId::root()); - let mut match_spec_to_candidates = + let mut match_spec_to_sorted_candidates = Mapping::new(vec![Vec::new(); self.pool.match_specs.len()]); let mut match_spec_to_forbidden = Mapping::new(vec![Vec::new(); self.pool.match_specs.len()]); + let match_spec_to_candidates = Mapping::new(vec![ + OnceCell::new(); + self.pool.match_specs.len() + ]); let mut seen_requires = HashSet::new(); let mut seen_forbidden = HashSet::new(); let empty_vec = Vec::new(); @@ -179,11 +184,18 @@ impl<'a> Solver<'a> { // Enqueue the candidates of the dependencies for &dep in deps { if seen_requires.insert(dep) { - self.pool - .populate_candidates(dep, favored_map, &mut match_spec_to_candidates); + self.pool.populate_candidates( + dep, + favored_map, + &mut match_spec_to_sorted_candidates, + &match_spec_to_candidates, + ); } - for &candidate in match_spec_to_candidates.get(dep).unwrap_or(&empty_vec) { + for &candidate in match_spec_to_sorted_candidates + .get(dep) + .unwrap_or(&empty_vec) + { // Note: we skip candidates we have already seen if visited.insert(candidate) { stack.push(candidate); @@ -196,7 +208,7 @@ impl<'a> Solver<'a> { self.clauses.push(ClauseState::new( Clause::Requires(solvable_id, dep), &self.learnt_clauses, - &match_spec_to_candidates, + &match_spec_to_sorted_candidates, )); } @@ -211,13 +223,13 @@ impl<'a> Solver<'a> { self.clauses.push(ClauseState::new( Clause::Constrains(solvable_id, dep), &self.learnt_clauses, - &match_spec_to_candidates, + &match_spec_to_sorted_candidates, )); } } } - self.pool.match_spec_to_candidates = match_spec_to_candidates; + self.pool.match_spec_to_sorted_candidates = match_spec_to_sorted_candidates; self.pool.match_spec_to_forbidden = match_spec_to_forbidden; } @@ -339,7 +351,7 @@ impl<'a> Solver<'a> { } // Consider only clauses in which no candidates have been installed - let candidates = &self.pool.match_spec_to_candidates[deps]; + let candidates = &self.pool.match_spec_to_sorted_candidates[deps]; if candidates .iter() .any(|&c| self.decision_tracker.assigned_value(c) == Some(true)) @@ -844,7 +856,7 @@ impl<'a> Solver<'a> { let mut clause = ClauseState::new( Clause::Learnt(learnt_id), &self.learnt_clauses, - &self.pool.match_spec_to_candidates, + &self.pool.match_spec_to_sorted_candidates, ); if clause.has_watches() { From c166ff14227a7706bc413ae18470a456c53ac8dc Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 17:06:07 +0200 Subject: [PATCH 2/7] fix: fmt and clippy --- crates/libsolv_rs/src/conda_util.rs | 2 +- crates/libsolv_rs/src/pool.rs | 2 +- crates/libsolv_rs/src/solver/mod.rs | 8 +++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index 88300673c..4e3de0193 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -4,7 +4,7 @@ use crate::mapping::Mapping; use crate::solvable::Solvable; use crate::MatchSpecId; use rattler_conda_types::{MatchSpec, Version}; -use std::cell::{OnceCell}; +use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::HashMap; diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index 251924e3b..9c72e66c1 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -1,10 +1,10 @@ -use std::cell::{OnceCell}; use crate::arena::Arena; use crate::conda_util; use crate::id::{MatchSpecId, NameId, RepoId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; use rattler_conda_types::{MatchSpec, PackageRecord}; +use std::cell::OnceCell; use std::collections::hash_map::Entry; use std::collections::HashMap; use std::str::FromStr; diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index efad8f2e6..684a136bc 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -7,7 +7,7 @@ use crate::problem::Problem; use crate::solvable::SolvableInner; use crate::solve_jobs::SolveJobs; use crate::transaction::Transaction; -use std::cell::{OnceCell}; +use std::cell::OnceCell; use itertools::Itertools; use rattler_conda_types::MatchSpec; @@ -167,10 +167,8 @@ impl<'a> Solver<'a> { Mapping::new(vec![Vec::new(); self.pool.match_specs.len()]); let mut match_spec_to_forbidden = Mapping::new(vec![Vec::new(); self.pool.match_specs.len()]); - let match_spec_to_candidates = Mapping::new(vec![ - OnceCell::new(); - self.pool.match_specs.len() - ]); + let match_spec_to_candidates = + Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); let mut seen_requires = HashSet::new(); let mut seen_forbidden = HashSet::new(); let empty_vec = Vec::new(); From 50b49ef35958e297ea57a7b4701fe9806b9be7dd Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 17:24:23 +0200 Subject: [PATCH 3/7] perf: also cache find_highest_version --- crates/libsolv_rs/src/conda_util.rs | 68 +++++++++++++++++------------ crates/libsolv_rs/src/pool.rs | 4 +- crates/libsolv_rs/src/solver/mod.rs | 3 ++ 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index 4e3de0193..860e3362d 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -9,6 +9,7 @@ use std::cmp::Ordering; use std::collections::HashMap; /// Returns the order of two candidates based on the order used by conda. +#[allow(clippy::too_many_arguments)] pub(crate) fn compare_candidates( a: SolvableId, b: SolvableId, @@ -17,6 +18,7 @@ pub(crate) fn compare_candidates( packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, + match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, ) -> Ordering { let a_solvable = solvables[a].package(); let b_solvable = solvables[b].package(); @@ -81,6 +83,7 @@ pub(crate) fn compare_candidates( packages_by_name, match_specs, match_spec_to_candidates, + match_spec_highest_version, ); let highest_b = find_highest_version( *b_spec_id, @@ -89,6 +92,7 @@ pub(crate) fn compare_candidates( packages_by_name, match_specs, match_spec_to_candidates, + match_spec_highest_version, ); // Skip version if no package is selected by either spec @@ -140,43 +144,49 @@ pub(crate) fn find_highest_version( packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, + match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, ) -> Option<(Version, bool)> { - let candidates = find_candidates( - match_spec_id, - match_specs, - interned_strings, - packages_by_name, - solvables, - match_spec_to_candidates, - ); - - candidates - .iter() - .map(|id| &solvables[*id].package().record) - .fold(None, |init, record| { - Some(init.map_or_else( - || { - ( - record.version.version().clone(), - !record.track_features.is_empty(), - ) - }, - |(version, has_tracked_features)| { - ( - version.max(record.version.version().clone()), - has_tracked_features && record.track_features.is_empty(), - ) - }, - )) + match_spec_highest_version[match_spec_id] + .get_or_init(|| { + let candidates = find_candidates( + match_spec_id, + match_specs, + interned_strings, + packages_by_name, + solvables, + match_spec_to_candidates, + ); + + candidates + .iter() + .map(|id| &solvables[*id].package().record) + .fold(None, |init, record| { + Some(init.map_or_else( + || { + ( + record.version.version().clone(), + !record.track_features.is_empty(), + ) + }, + |(version, has_tracked_features)| { + ( + version.max(record.version.version().clone()), + has_tracked_features && record.track_features.is_empty(), + ) + }, + )) + }) }) + .as_ref() + .map(|(version, has_tracked_features)| (version.clone(), *has_tracked_features)) } -pub(crate) fn find_candidates<'a, 'b>( +pub(crate) fn find_candidates<'b>( match_spec_id: MatchSpecId, match_specs: &Arena<MatchSpecId, MatchSpec>, names_to_ids: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, - solvables: &Arena<SolvableId, Solvable<'a>>, + solvables: &Arena<SolvableId, Solvable>, match_spec_to_candidates: &'b Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, ) -> &'b Vec<SolvableId> { match_spec_to_candidates[match_spec_id].get_or_init(|| { diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index 9c72e66c1..dfef7ed61 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -3,7 +3,7 @@ use crate::conda_util; use crate::id::{MatchSpecId, NameId, RepoId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; -use rattler_conda_types::{MatchSpec, PackageRecord}; +use rattler_conda_types::{MatchSpec, PackageRecord, Version}; use std::cell::OnceCell; use std::collections::hash_map::Entry; use std::collections::HashMap; @@ -130,6 +130,7 @@ impl<'a> Pool<'a> { favored_map: &HashMap<NameId, SolvableId>, match_spec_to_sorted_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, + match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec @@ -160,6 +161,7 @@ impl<'a> Pool<'a> { &self.packages_by_name, &self.match_specs, match_spec_to_candidates, + match_spec_highest_version, ) }); diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index 684a136bc..fd601ca76 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -169,6 +169,8 @@ impl<'a> Solver<'a> { Mapping::new(vec![Vec::new(); self.pool.match_specs.len()]); let match_spec_to_candidates = Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); + let match_spec_to_highest_version = + Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); let mut seen_requires = HashSet::new(); let mut seen_forbidden = HashSet::new(); let empty_vec = Vec::new(); @@ -187,6 +189,7 @@ impl<'a> Solver<'a> { favored_map, &mut match_spec_to_sorted_candidates, &match_spec_to_candidates, + &match_spec_to_highest_version, ); } From 60640ea253c3fcb0c83d69556b9076bce538c19a Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 17:32:48 +0200 Subject: [PATCH 4/7] fix: cache ordering of solvables --- crates/libsolv_rs/src/pool.rs | 25 +++++++++++++++---------- crates/libsolv_rs/src/solver/mod.rs | 2 ++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index dfef7ed61..4f56e2c3e 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -5,6 +5,7 @@ use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; use rattler_conda_types::{MatchSpec, PackageRecord, Version}; use std::cell::OnceCell; +use std::cmp::Ordering; use std::collections::hash_map::Entry; use std::collections::HashMap; use std::str::FromStr; @@ -131,6 +132,7 @@ impl<'a> Pool<'a> { match_spec_to_sorted_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, + solvable_order: &mut HashMap<(SolvableId, SolvableId), Ordering>, ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec @@ -153,16 +155,19 @@ impl<'a> Pool<'a> { .clone(); pkgs.sort_by(|&p1, &p2| { - conda_util::compare_candidates( - p1, - p2, - &self.solvables, - &self.names_to_ids, - &self.packages_by_name, - &self.match_specs, - match_spec_to_candidates, - match_spec_highest_version, - ) + let key = (p1, p2); + *solvable_order.entry(key).or_insert_with(|| { + conda_util::compare_candidates( + p1, + p2, + &self.solvables, + &self.names_to_ids, + &self.packages_by_name, + &self.match_specs, + match_spec_to_candidates, + match_spec_highest_version, + ) + }) }); if let Some(&favored_id) = favored_map.get(&name_id) { diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index fd601ca76..146256f34 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -171,6 +171,7 @@ impl<'a> Solver<'a> { Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); let match_spec_to_highest_version = Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); + let mut sorting_cache = HashMap::new(); let mut seen_requires = HashSet::new(); let mut seen_forbidden = HashSet::new(); let empty_vec = Vec::new(); @@ -190,6 +191,7 @@ impl<'a> Solver<'a> { &mut match_spec_to_sorted_candidates, &match_spec_to_candidates, &match_spec_to_highest_version, + &mut sorting_cache, ); } From b17b9469e36acbfecf6ae892d057ced0e915fd5c Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 18:09:48 +0200 Subject: [PATCH 5/7] perf: small improvements --- crates/libsolv_rs/Cargo.toml | 1 + crates/libsolv_rs/src/conda_util.rs | 7 ++++--- crates/libsolv_rs/src/id.rs | 6 ++++++ crates/libsolv_rs/src/pool.rs | 15 ++++++++------- crates/libsolv_rs/src/solver/clause.rs | 1 + crates/libsolv_rs/src/solver/mod.rs | 7 ++++--- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/crates/libsolv_rs/Cargo.toml b/crates/libsolv_rs/Cargo.toml index ac644c1b9..514873d12 100644 --- a/crates/libsolv_rs/Cargo.toml +++ b/crates/libsolv_rs/Cargo.toml @@ -11,6 +11,7 @@ license.workspace = true readme.workspace = true [dependencies] +ahash = "0.8.3" itertools = "0.11.0" petgraph = "0.6.3" rattler_conda_types = { version = "0.5.0", path = "../rattler_conda_types" } diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index 860e3362d..dcd32fff6 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -7,6 +7,7 @@ use rattler_conda_types::{MatchSpec, Version}; use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::HashMap; +use ahash::AHashMap; /// Returns the order of two candidates based on the order used by conda. #[allow(clippy::too_many_arguments)] @@ -14,7 +15,7 @@ pub(crate) fn compare_candidates( a: SolvableId, b: SolvableId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &HashMap<String, NameId>, + interned_strings: &AHashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -140,7 +141,7 @@ pub(crate) fn compare_candidates( pub(crate) fn find_highest_version( match_spec_id: MatchSpecId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &HashMap<String, NameId>, + interned_strings: &AHashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -184,7 +185,7 @@ pub(crate) fn find_highest_version( pub(crate) fn find_candidates<'b>( match_spec_id: MatchSpecId, match_specs: &Arena<MatchSpecId, MatchSpec>, - names_to_ids: &HashMap<String, NameId>, + names_to_ids: &AHashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, solvables: &Arena<SolvableId, Solvable>, match_spec_to_candidates: &'b Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, diff --git a/crates/libsolv_rs/src/id.rs b/crates/libsolv_rs/src/id.rs index 904808dda..14fef9287 100644 --- a/crates/libsolv_rs/src/id.rs +++ b/crates/libsolv_rs/src/id.rs @@ -74,6 +74,12 @@ impl ArenaId for SolvableId { } } +impl From<SolvableId> for u32 { + fn from(value: SolvableId) -> Self { + value.0 + } +} + #[repr(transparent)] #[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Debug, Hash)] pub(crate) struct ClauseId(u32); diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index 4f56e2c3e..4417b5ca7 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -7,8 +7,9 @@ use rattler_conda_types::{MatchSpec, PackageRecord, Version}; use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::hash_map::Entry; -use std::collections::HashMap; +use std::collections::{HashMap}; use std::str::FromStr; +use ahash::AHashMap; /// A pool that stores data related to the available packages /// @@ -25,7 +26,7 @@ pub struct Pool<'a> { package_names: Arena<NameId, String>, /// Map from package names to the id of their interned counterpart - pub(crate) names_to_ids: HashMap<String, NameId>, + pub(crate) names_to_ids: AHashMap<String, NameId>, /// Map from interned package names to the solvables that have that name pub(crate) packages_by_name: Mapping<NameId, Vec<SolvableId>>, @@ -34,7 +35,7 @@ pub struct Pool<'a> { pub(crate) match_specs: Arena<MatchSpecId, MatchSpec>, /// Map from match spec strings to the id of their interned counterpart - match_specs_to_ids: HashMap<String, MatchSpecId>, + match_specs_to_ids: AHashMap<String, MatchSpecId>, /// Cached candidates for each match spec pub(crate) match_spec_to_sorted_candidates: Mapping<MatchSpecId, Vec<SolvableId>>, @@ -52,11 +53,11 @@ impl<'a> Default for Pool<'a> { solvables, total_repos: 0, - names_to_ids: HashMap::new(), + names_to_ids: Default::default(), package_names: Arena::new(), packages_by_name: Mapping::empty(), - match_specs_to_ids: HashMap::default(), + match_specs_to_ids: Default::default(), match_specs: Arena::new(), match_spec_to_sorted_candidates: Mapping::empty(), match_spec_to_forbidden: Mapping::empty(), @@ -132,7 +133,7 @@ impl<'a> Pool<'a> { match_spec_to_sorted_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, - solvable_order: &mut HashMap<(SolvableId, SolvableId), Ordering>, + solvable_order: &mut AHashMap<u64, Ordering>, ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec @@ -155,7 +156,7 @@ impl<'a> Pool<'a> { .clone(); pkgs.sort_by(|&p1, &p2| { - let key = (p1, p2); + let key = u32::from(p1) as u64 | ((u32::from(p2) as u64) << 32); *solvable_order.entry(key).or_insert_with(|| { conda_util::compare_candidates( p1, diff --git a/crates/libsolv_rs/src/solver/clause.rs b/crates/libsolv_rs/src/solver/clause.rs index e1e2b9990..7b862656e 100644 --- a/crates/libsolv_rs/src/solver/clause.rs +++ b/crates/libsolv_rs/src/solver/clause.rs @@ -230,6 +230,7 @@ impl ClauseState { } } + #[inline] pub fn next_watched_clause(&self, solvable_id: SolvableId) -> ClauseId { if solvable_id == self.watched_literals[0] { self.next_watches[0] diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index 146256f34..c21a25c51 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -12,6 +12,7 @@ use std::cell::OnceCell; use itertools::Itertools; use rattler_conda_types::MatchSpec; use std::collections::{HashMap, HashSet}; +use ahash::{AHashMap, AHashSet}; use clause::{Clause, ClauseState, Literal}; use decision::Decision; @@ -171,9 +172,9 @@ impl<'a> Solver<'a> { Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); let match_spec_to_highest_version = Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); - let mut sorting_cache = HashMap::new(); - let mut seen_requires = HashSet::new(); - let mut seen_forbidden = HashSet::new(); + let mut sorting_cache = AHashMap::new(); + let mut seen_requires = AHashSet::new(); + let mut seen_forbidden = AHashSet::new(); let empty_vec = Vec::new(); while let Some(solvable_id) = stack.pop() { From aace58abf9fd6b989584b41100c5b912c91e8aa5 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <zalmstra.bas@gmail.com> Date: Thu, 6 Jul 2023 18:10:22 +0200 Subject: [PATCH 6/7] perf: small improvements --- crates/libsolv_rs/src/conda_util.rs | 2 +- crates/libsolv_rs/src/pool.rs | 4 ++-- crates/libsolv_rs/src/solver/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index dcd32fff6..9cc5e3e4a 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -3,11 +3,11 @@ use crate::id::{NameId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::Solvable; use crate::MatchSpecId; +use ahash::AHashMap; use rattler_conda_types::{MatchSpec, Version}; use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::HashMap; -use ahash::AHashMap; /// Returns the order of two candidates based on the order used by conda. #[allow(clippy::too_many_arguments)] diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index 4417b5ca7..96fcdec8a 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -3,13 +3,13 @@ use crate::conda_util; use crate::id::{MatchSpecId, NameId, RepoId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; +use ahash::AHashMap; use rattler_conda_types::{MatchSpec, PackageRecord, Version}; use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::hash_map::Entry; -use std::collections::{HashMap}; +use std::collections::HashMap; use std::str::FromStr; -use ahash::AHashMap; /// A pool that stores data related to the available packages /// diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index c21a25c51..02abbf687 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -9,10 +9,10 @@ use crate::solve_jobs::SolveJobs; use crate::transaction::Transaction; use std::cell::OnceCell; +use ahash::{AHashMap, AHashSet}; use itertools::Itertools; use rattler_conda_types::MatchSpec; use std::collections::{HashMap, HashSet}; -use ahash::{AHashMap, AHashSet}; use clause::{Clause, ClauseState, Literal}; use decision::Decision; From dc193ad0c02c6a5097429522f928c61f9707802c Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <bas@prefix.dev> Date: Fri, 7 Jul 2023 14:19:42 +0200 Subject: [PATCH 7/7] fix: remove ahash --- crates/libsolv_rs/Cargo.toml | 1 - crates/libsolv_rs/src/conda_util.rs | 7 +++---- crates/libsolv_rs/src/pool.rs | 7 +++---- crates/libsolv_rs/src/solver/mod.rs | 7 +++---- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/libsolv_rs/Cargo.toml b/crates/libsolv_rs/Cargo.toml index 514873d12..ac644c1b9 100644 --- a/crates/libsolv_rs/Cargo.toml +++ b/crates/libsolv_rs/Cargo.toml @@ -11,7 +11,6 @@ license.workspace = true readme.workspace = true [dependencies] -ahash = "0.8.3" itertools = "0.11.0" petgraph = "0.6.3" rattler_conda_types = { version = "0.5.0", path = "../rattler_conda_types" } diff --git a/crates/libsolv_rs/src/conda_util.rs b/crates/libsolv_rs/src/conda_util.rs index 9cc5e3e4a..860e3362d 100644 --- a/crates/libsolv_rs/src/conda_util.rs +++ b/crates/libsolv_rs/src/conda_util.rs @@ -3,7 +3,6 @@ use crate::id::{NameId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::Solvable; use crate::MatchSpecId; -use ahash::AHashMap; use rattler_conda_types::{MatchSpec, Version}; use std::cell::OnceCell; use std::cmp::Ordering; @@ -15,7 +14,7 @@ pub(crate) fn compare_candidates( a: SolvableId, b: SolvableId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &AHashMap<String, NameId>, + interned_strings: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -141,7 +140,7 @@ pub(crate) fn compare_candidates( pub(crate) fn find_highest_version( match_spec_id: MatchSpecId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &AHashMap<String, NameId>, + interned_strings: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -185,7 +184,7 @@ pub(crate) fn find_highest_version( pub(crate) fn find_candidates<'b>( match_spec_id: MatchSpecId, match_specs: &Arena<MatchSpecId, MatchSpec>, - names_to_ids: &AHashMap<String, NameId>, + names_to_ids: &HashMap<String, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, solvables: &Arena<SolvableId, Solvable>, match_spec_to_candidates: &'b Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, diff --git a/crates/libsolv_rs/src/pool.rs b/crates/libsolv_rs/src/pool.rs index 96fcdec8a..b357d09ae 100644 --- a/crates/libsolv_rs/src/pool.rs +++ b/crates/libsolv_rs/src/pool.rs @@ -3,7 +3,6 @@ use crate::conda_util; use crate::id::{MatchSpecId, NameId, RepoId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::{PackageSolvable, Solvable}; -use ahash::AHashMap; use rattler_conda_types::{MatchSpec, PackageRecord, Version}; use std::cell::OnceCell; use std::cmp::Ordering; @@ -26,7 +25,7 @@ pub struct Pool<'a> { package_names: Arena<NameId, String>, /// Map from package names to the id of their interned counterpart - pub(crate) names_to_ids: AHashMap<String, NameId>, + pub(crate) names_to_ids: HashMap<String, NameId>, /// Map from interned package names to the solvables that have that name pub(crate) packages_by_name: Mapping<NameId, Vec<SolvableId>>, @@ -35,7 +34,7 @@ pub struct Pool<'a> { pub(crate) match_specs: Arena<MatchSpecId, MatchSpec>, /// Map from match spec strings to the id of their interned counterpart - match_specs_to_ids: AHashMap<String, MatchSpecId>, + match_specs_to_ids: HashMap<String, MatchSpecId>, /// Cached candidates for each match spec pub(crate) match_spec_to_sorted_candidates: Mapping<MatchSpecId, Vec<SolvableId>>, @@ -133,7 +132,7 @@ impl<'a> Pool<'a> { match_spec_to_sorted_candidates: &mut Mapping<MatchSpecId, Vec<SolvableId>>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, match_spec_highest_version: &Mapping<MatchSpecId, OnceCell<Option<(Version, bool)>>>, - solvable_order: &mut AHashMap<u64, Ordering>, + solvable_order: &mut HashMap<u64, Ordering>, ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec diff --git a/crates/libsolv_rs/src/solver/mod.rs b/crates/libsolv_rs/src/solver/mod.rs index 02abbf687..146256f34 100644 --- a/crates/libsolv_rs/src/solver/mod.rs +++ b/crates/libsolv_rs/src/solver/mod.rs @@ -9,7 +9,6 @@ use crate::solve_jobs::SolveJobs; use crate::transaction::Transaction; use std::cell::OnceCell; -use ahash::{AHashMap, AHashSet}; use itertools::Itertools; use rattler_conda_types::MatchSpec; use std::collections::{HashMap, HashSet}; @@ -172,9 +171,9 @@ impl<'a> Solver<'a> { Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); let match_spec_to_highest_version = Mapping::new(vec![OnceCell::new(); self.pool.match_specs.len()]); - let mut sorting_cache = AHashMap::new(); - let mut seen_requires = AHashSet::new(); - let mut seen_forbidden = AHashSet::new(); + let mut sorting_cache = HashMap::new(); + let mut seen_requires = HashSet::new(); + let mut seen_forbidden = HashSet::new(); let empty_vec = Vec::new(); while let Some(solvable_id) = stack.pop() {