Skip to content

Cache the query result. #5112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 3, 2018
Merged

Cache the query result. #5112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10db1be
Cache the query result.
Eh2406 Mar 2, 2018
a7846b8
Make a `RegistryQueryer` to own the cache
Eh2406 Mar 3, 2018
a81e08b
`Context` no longer needs replacements nor its lifetime
Eh2406 Mar 3, 2018
cb0df8e
Use `HashMap` insted of `BTreeMap`
Eh2406 Mar 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/cargo/core/dependency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ use util::errors::{CargoResult, CargoResultExt, CargoError};

/// Information about a dependency requested by a Cargo manifest.
/// Cheap to copy.
#[derive(PartialEq, Clone, Debug)]
#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug)]
pub struct Dependency {
inner: Rc<Inner>,
}

/// The data underlying a Dependency.
#[derive(PartialEq, Clone, Debug)]
#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug)]
struct Inner {
name: String,
source_id: SourceId,
Expand All @@ -38,7 +38,7 @@ struct Inner {
platform: Option<Platform>,
}

#[derive(Clone, Debug, PartialEq)]
#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)]
pub enum Platform {
Name(String),
Cfg(CfgExpr),
Expand Down Expand Up @@ -76,7 +76,7 @@ impl ser::Serialize for Dependency {
}
}

#[derive(PartialEq, Clone, Debug, Copy)]
#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug, Copy)]
pub enum Kind {
Normal,
Development,
Expand Down
216 changes: 119 additions & 97 deletions src/cargo/core/resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ enum GraphNode {
// risk of being cloned *a lot* so we want to make this as cheap to clone as
// possible.
#[derive(Clone)]
struct Context<'a> {
struct Context {
// TODO: Both this and the two maps below are super expensive to clone. We should
// switch to persistent hash maps if we can at some point or otherwise
// make these much cheaper to clone in general.
Expand All @@ -340,8 +340,6 @@ struct Context<'a> {
resolve_graph: RcList<GraphNode>,
resolve_replacements: RcList<(PackageId, PackageId)>,

replacements: &'a [(PackageIdSpec, Dependency)],

// These warnings are printed after resolution.
warnings: RcList<String>,
}
Expand All @@ -360,11 +358,10 @@ pub fn resolve(summaries: &[(Summary, Method)],
links: HashMap::new(),
resolve_replacements: RcList::new(),
activations: HashMap::new(),
replacements,
warnings: RcList::new(),
};
let _p = profile::start("resolving");
let cx = activate_deps_loop(cx, registry, summaries, config)?;
let cx = activate_deps_loop(cx, &mut RegistryQueryer::new(registry, replacements), summaries, config)?;

let mut resolve = Resolve {
graph: cx.graph(),
Expand Down Expand Up @@ -410,7 +407,7 @@ pub fn resolve(summaries: &[(Summary, Method)],
/// If `candidate` was activated, this function returns the dependency frame to
/// iterate through next.
fn activate(cx: &mut Context,
registry: &mut Registry,
registry: &mut RegistryQueryer,
parent: Option<&Summary>,
candidate: Candidate,
method: &Method)
Expand Down Expand Up @@ -573,10 +570,112 @@ impl ConflictReason {
}
}

struct RegistryQueryer<'a> {
registry: &'a mut (Registry + 'a),
replacements: &'a [(PackageIdSpec, Dependency)],
// TODO: with nll the Rc can be removed
cache: HashMap<Dependency, Rc<Vec<Candidate>>>,
}

impl<'a> RegistryQueryer<'a> {
fn new(registry: &'a mut Registry, replacements: &'a [(PackageIdSpec, Dependency)],) -> Self {
RegistryQueryer {
registry,
replacements,
cache: HashMap::new(),
}
}

/// Queries the `registry` to return a list of candidates for `dep`.
///
/// This method is the location where overrides are taken into account. If
/// any candidates are returned which match an override then the override is
/// applied by performing a second query for what the override should
/// return.
fn query(&mut self, dep: &Dependency) -> CargoResult<Rc<Vec<Candidate>>> {
if let Some(out) = self.cache.get(dep).cloned() {
return Ok(out);
}

let mut ret = Vec::new();
self.registry.query(dep, &mut |s| {
ret.push(Candidate { summary: s, replace: None });
})?;
for candidate in ret.iter_mut() {
let summary = &candidate.summary;

let mut potential_matches = self.replacements.iter()
.filter(|&&(ref spec, _)| spec.matches(summary.package_id()));

let &(ref spec, ref dep) = match potential_matches.next() {
None => continue,
Some(replacement) => replacement,
};
debug!("found an override for {} {}", dep.name(), dep.version_req());

let mut summaries = self.registry.query_vec(dep)?.into_iter();
let s = summaries.next().ok_or_else(|| {
format_err!("no matching package for override `{}` found\n\
location searched: {}\n\
version required: {}",
spec, dep.source_id(), dep.version_req())
})?;
let summaries = summaries.collect::<Vec<_>>();
if !summaries.is_empty() {
let bullets = summaries.iter().map(|s| {
format!(" * {}", s.package_id())
}).collect::<Vec<_>>();
bail!("the replacement specification `{}` matched \
multiple packages:\n * {}\n{}", spec, s.package_id(),
bullets.join("\n"));
}

// The dependency should be hard-coded to have the same name and an
// exact version requirement, so both of these assertions should
// never fail.
assert_eq!(s.version(), summary.version());
assert_eq!(s.name(), summary.name());

let replace = if s.source_id() == summary.source_id() {
debug!("Preventing\n{:?}\nfrom replacing\n{:?}", summary, s);
None
} else {
Some(s)
};
let matched_spec = spec.clone();

// Make sure no duplicates
if let Some(&(ref spec, _)) = potential_matches.next() {
bail!("overlapping replacement specifications found:\n\n \
* {}\n * {}\n\nboth specifications match: {}",
matched_spec, spec, summary.package_id());
}

for dep in summary.dependencies() {
debug!("\t{} => {}", dep.name(), dep.version_req());
}

candidate.replace = replace;
}

// When we attempt versions for a package, we'll want to start at
// the maximum version and work our way down.
ret.sort_unstable_by(|a, b| {
b.summary.version().cmp(a.summary.version())
});

let out = Rc::new(ret);

self.cache.insert(dep.clone(), out.clone());

Ok(out)
}
}

#[derive(Clone)]
struct BacktrackFrame<'a> {
struct BacktrackFrame {
cur: usize,
context_backup: Context<'a>,
context_backup: Context,
deps_backup: BinaryHeap<DepsFrame>,
remaining_candidates: RemainingCandidates,
parent: Summary,
Expand Down Expand Up @@ -658,12 +757,12 @@ impl RemainingCandidates {
///
/// If all dependencies can be activated and resolved to a version in the
/// dependency graph, cx.resolve is returned.
fn activate_deps_loop<'a>(
mut cx: Context<'a>,
registry: &mut Registry,
fn activate_deps_loop(
mut cx: Context,
registry: &mut RegistryQueryer,
summaries: &[(Summary, Method)],
config: Option<&Config>,
) -> CargoResult<Context<'a>> {
) -> CargoResult<Context> {
// Note that a `BinaryHeap` is used for the remaining dependencies that need
// activation. This heap is sorted such that the "largest value" is the most
// constrained dependency, or the one with the least candidates.
Expand Down Expand Up @@ -780,7 +879,7 @@ fn activate_deps_loop<'a>(
).ok_or_else(|| {
activation_error(
&cx,
registry,
registry.registry,
&parent,
&dep,
&conflicting_activations,
Expand Down Expand Up @@ -850,9 +949,9 @@ fn activate_deps_loop<'a>(
/// If the outcome could differ, resets `cx` and `remaining_deps` to that
/// level and returns the next candidate.
/// If all candidates have been exhausted, returns None.
fn find_candidate<'a>(
backtrack_stack: &mut Vec<BacktrackFrame<'a>>,
cx: &mut Context<'a>,
fn find_candidate(
backtrack_stack: &mut Vec<BacktrackFrame>,
cx: &mut Context,
remaining_deps: &mut BinaryHeap<DepsFrame>,
parent: &mut Summary,
cur: &mut usize,
Expand Down Expand Up @@ -1176,7 +1275,7 @@ fn build_requirements<'a, 'b: 'a>(s: &'a Summary, method: &'b Method)
Ok(reqs)
}

impl<'a> Context<'a> {
impl Context {
/// Activate this summary by inserting it into our list of known activations.
///
/// Returns true if this summary with the given method is already activated.
Expand Down Expand Up @@ -1219,7 +1318,7 @@ impl<'a> Context<'a> {
}

fn build_deps(&mut self,
registry: &mut Registry,
registry: &mut RegistryQueryer,
parent: Option<&Summary>,
candidate: &Summary,
method: &Method) -> ActivateResult<Vec<DepInfo>> {
Expand All @@ -1231,13 +1330,8 @@ impl<'a> Context<'a> {
// Next, transform all dependencies into a list of possible candidates
// which can satisfy that dependency.
let mut deps = deps.into_iter().map(|(dep, features)| {
let mut candidates = self.query(registry, &dep)?;
// When we attempt versions for a package, we'll want to start at
// the maximum version and work our way down.
candidates.sort_by(|a, b| {
b.summary.version().cmp(a.summary.version())
});
Ok((dep, Rc::new(candidates), Rc::new(features)))
let candidates = registry.query(&dep)?;
Ok((dep, candidates, Rc::new(features)))
}).collect::<CargoResult<Vec<DepInfo>>>()?;

// Attempt to resolve dependencies with fewer candidates before trying
Expand All @@ -1249,78 +1343,6 @@ impl<'a> Context<'a> {
Ok(deps)
}

/// Queries the `registry` to return a list of candidates for `dep`.
///
/// This method is the location where overrides are taken into account. If
/// any candidates are returned which match an override then the override is
/// applied by performing a second query for what the override should
/// return.
fn query(&self,
registry: &mut Registry,
dep: &Dependency) -> CargoResult<Vec<Candidate>> {
let mut ret = Vec::new();
registry.query(dep, &mut |s| {
ret.push(Candidate { summary: s, replace: None });
})?;
for candidate in ret.iter_mut() {
let summary = &candidate.summary;

let mut potential_matches = self.replacements.iter()
.filter(|&&(ref spec, _)| spec.matches(summary.package_id()));

let &(ref spec, ref dep) = match potential_matches.next() {
None => continue,
Some(replacement) => replacement,
};
debug!("found an override for {} {}", dep.name(), dep.version_req());

let mut summaries = registry.query_vec(dep)?.into_iter();
let s = summaries.next().ok_or_else(|| {
format_err!("no matching package for override `{}` found\n\
location searched: {}\n\
version required: {}",
spec, dep.source_id(), dep.version_req())
})?;
let summaries = summaries.collect::<Vec<_>>();
if !summaries.is_empty() {
let bullets = summaries.iter().map(|s| {
format!(" * {}", s.package_id())
}).collect::<Vec<_>>();
bail!("the replacement specification `{}` matched \
multiple packages:\n * {}\n{}", spec, s.package_id(),
bullets.join("\n"));
}

// The dependency should be hard-coded to have the same name and an
// exact version requirement, so both of these assertions should
// never fail.
assert_eq!(s.version(), summary.version());
assert_eq!(s.name(), summary.name());

let replace = if s.source_id() == summary.source_id() {
debug!("Preventing\n{:?}\nfrom replacing\n{:?}", summary, s);
None
} else {
Some(s)
};
let matched_spec = spec.clone();

// Make sure no duplicates
if let Some(&(ref spec, _)) = potential_matches.next() {
bail!("overlapping replacement specifications found:\n\n \
* {}\n * {}\n\nboth specifications match: {}",
matched_spec, spec, summary.package_id());
}

for dep in summary.dependencies() {
debug!("\t{} => {}", dep.name(), dep.version_req());
}

candidate.replace = replace;
}
Ok(ret)
}

fn prev_active(&self, dep: &Dependency) -> &[Summary] {
self.activations.get(dep.name())
.and_then(|v| v.get(dep.source_id()))
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/cfg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ use std::fmt;

use util::{CargoError, CargoResult};

#[derive(Clone, PartialEq, Debug)]
#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)]
pub enum Cfg {
Name(String),
KeyPair(String, String),
}

#[derive(Clone, PartialEq, Debug)]
#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)]
pub enum CfgExpr {
Not(Box<CfgExpr>),
All(Vec<CfgExpr>),
Expand Down