diff --git a/Cargo.toml b/Cargo.toml index 9b20fb766fc..aae7be46cea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ hex = "0.4" home = "0.5" humantime = "1.2.0" ignore = "0.4.7" +indexmap = "1.0.2" lazy_static = "1.2.0" jobserver = "0.1.13" lazycell = "1.2.0" diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index c77a050758e..58a47f0e26c 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -70,7 +70,7 @@ pub fn resolve_and_validated( let mut self_pub_dep = HashSet::new(); self_pub_dep.insert(p); for (dp, deps) in resolve.deps(p) { - if deps.iter().any(|d| d.is_public()) { + if deps.clone().any(|d| d.is_public()) { self_pub_dep.extend(pub_deps[&dp].iter().cloned()) } } diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 085c41e97cc..25430426eaa 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -225,9 +225,9 @@ fn compute_deps<'a, 'cfg>( let bcx = state.bcx; let id = unit.pkg.package_id(); - let deps = state.resolve().deps(id).filter(|&(_id, deps)| { + let deps = state.resolve().deps(id).filter(|(_id, deps)| { assert!(!deps.is_empty()); - deps.iter().any(|dep| { + (*deps).clone().any(|dep| { // If this target is a build command, then we only want build // dependencies, otherwise we want everything *other than* build // dependencies. @@ -391,8 +391,8 @@ fn compute_deps_doc<'a, 'cfg>( let deps = state .resolve() .deps(unit.pkg.package_id()) - .filter(|&(_id, deps)| { - deps.iter().any(|dep| match dep.kind() { + .filter(|(_id, deps)| { + (*deps).clone().any(|dep| match dep.kind() { DepKind::Normal => bcx.dep_platform_activated(dep, unit.kind), _ => false, }) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 991b746e3c2..60ded0809e0 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -1,13 +1,12 @@ use std::collections::HashMap; use std::num::NonZeroU64; -use std::rc::Rc; use failure::format_err; use log::debug; use crate::core::interning::InternedString; use crate::core::{Dependency, PackageId, SourceId, Summary}; -use crate::util::Graph; +use crate::util::{Graph, StackGraph}; use super::dep_cache::RegistryQueryer; use super::errors::ActivateResult; @@ -35,7 +34,7 @@ pub struct Context { /// a way to look up for a package in activations what packages required it /// and all of the exact deps that it fulfilled. - pub parents: Graph>>, + pub parents: StackGraph, } /// When backtracking it can be useful to know how far back to go. @@ -90,7 +89,7 @@ impl Context { } else { None }, - parents: Graph::new(), + parents: StackGraph::new(), activations: im_rc::HashMap::new(), } } @@ -224,17 +223,17 @@ impl Context { .collect() } - pub fn graph(&self) -> Graph> { - let mut graph: Graph> = Graph::new(); + pub fn graph(&self) -> Graph { + let mut graph: Graph = Graph::new(); self.activations .values() .for_each(|(r, _)| graph.add(r.package_id())); - for i in self.parents.iter() { + for i in self.parents.borrow().iter() { graph.add(*i); - for (o, e) in self.parents.edges(i) { - let old_link = graph.link(*o, *i); - assert!(old_link.is_empty()); - *old_link = e.to_vec(); + for (o, edges) in self.parents.borrow().edges(i) { + for e in edges { + graph.add_edge(*o, *i, e.clone()) + } } } graph diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 823120e167b..94762aa0120 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -77,10 +77,11 @@ pub(super) fn activation_error( candidates: &[Summary], config: Option<&Config>, ) -> ResolveError { + let parents = cx.parents.clone_into_graph(); let to_resolve_err = |err| { ResolveError::new( err, - cx.parents + parents .path_to_bottom(&parent.package_id()) .into_iter() .cloned() @@ -92,7 +93,7 @@ pub(super) fn activation_error( let mut msg = format!("failed to select a version for `{}`.", dep.package_name()); msg.push_str("\n ... required by "); msg.push_str(&describe_path( - &cx.parents.path_to_bottom(&parent.package_id()), + &parents.path_to_bottom(&parent.package_id()), )); msg.push_str("\nversions that meet the requirements `"); @@ -124,7 +125,7 @@ pub(super) fn activation_error( msg.push_str(link); msg.push_str("` as well:\n"); } - msg.push_str(&describe_path(&cx.parents.path_to_bottom(p))); + msg.push_str(&describe_path(&parents.path_to_bottom(p))); } let (features_errors, mut other_errors): (Vec<_>, Vec<_>) = other_errors @@ -178,7 +179,7 @@ pub(super) fn activation_error( for &(p, _) in other_errors.iter() { msg.push_str("\n\n previously selected "); - msg.push_str(&describe_path(&cx.parents.path_to_bottom(p))); + msg.push_str(&describe_path(&parents.path_to_bottom(p))); } msg.push_str("\n\nfailed to select a version for `"); @@ -230,7 +231,7 @@ pub(super) fn activation_error( ); msg.push_str("required by "); msg.push_str(&describe_path( - &cx.parents.path_to_bottom(&parent.package_id()), + &parents.path_to_bottom(&parent.package_id()), )); // If we have a path dependency with a locked version, then this may @@ -307,7 +308,7 @@ pub(super) fn activation_error( } msg.push_str("required by "); msg.push_str(&describe_path( - &cx.parents.path_to_bottom(&parent.package_id()), + &parents.path_to_bottom(&parent.package_id()), )); msg diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 4aaa7eeafed..a53fdaf38e0 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -596,12 +596,8 @@ fn activate( let candidate_pid = candidate.package_id(); if let Some((parent, dep)) = parent { let parent_pid = parent.package_id(); - Rc::make_mut( - // add a edge from candidate to parent in the parents graph - cx.parents.link(candidate_pid, parent_pid), - ) - // and associate dep with that edge - .push(dep.clone()); + // add a edge from candidate to parent in the parents graph and associate dep with that edge + cx.parents.add_edge(candidate_pid, parent_pid, dep.clone()); if let Some(public_dependency) = cx.public_dependency.as_mut() { // one tricky part is that `candidate_pid` may already be active and // have public dependencies of its own. So we not only need to mark @@ -643,8 +639,8 @@ fn activate( // if `candidate_pid` was a private dependency of `p` then `p` parents can't see `c` thru `p` if public { // if it was public, then we add all of `p`s parents to be checked - for &(grand, ref d) in cx.parents.edges(&p) { - stack.push((grand, d.iter().any(|x| x.is_public()))); + for (&grand, mut d) in cx.parents.borrow().edges(&p) { + stack.push((grand, d.any(|x| x.is_public()))); } } } @@ -825,8 +821,8 @@ impl RemainingCandidates { // if `b` was a private dependency of `p` then `p` parents can't see `t` thru `p` if public { // if it was public, then we add all of `p`s parents to be checked - for &(grand, ref d) in cx.parents.edges(&p) { - stack.push((grand, d.iter().any(|x| x.is_public()))); + for (&grand, mut d) in cx.parents.borrow().edges(&p) { + stack.push((grand, d.any(|x| x.is_public()))); } } } @@ -884,13 +880,16 @@ fn generalize_conflicting( return None; } // What parents does that critical activation have - for (critical_parent, critical_parents_deps) in - cx.parents.edges(&backtrack_critical_id).filter(|(p, _)| { + for (critical_parent, critical_parents_deps) in cx + .parents + .borrow() + .edges(&backtrack_critical_id) + .filter(|(&p, _)| { // it will only help backjump further if it is older then the critical_age - cx.is_active(*p).expect("parent not currently active!?") < backtrack_critical_age + cx.is_active(p).expect("parent not currently active!?") < backtrack_critical_age }) { - for critical_parents_dep in critical_parents_deps.iter() { + for critical_parents_dep in critical_parents_deps { // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. @@ -1083,8 +1082,8 @@ fn check_cycles(resolve: &Resolve) -> CargoResult<()> { // visitation list as we can't induce a cycle through transitive // dependencies. if checked.insert(id) { - for (dep, listings) in resolve.deps_not_replaced(id) { - let is_transitive = listings.iter().any(|d| d.is_transitive()); + for (dep, mut listings) in resolve.deps_not_replaced(id) { + let is_transitive = listings.any(|d| d.is_transitive()); let mut empty = HashSet::new(); let visited = if is_transitive { &mut *visited diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index dcb7cd44361..176df30e3a8 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,12 +1,13 @@ use std::borrow::Borrow; use std::collections::{HashMap, HashSet}; use std::fmt; +use std::hash::Hash; use std::iter::FromIterator; use crate::core::dependency::Kind; use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; use crate::util::errors::CargoResult; -use crate::util::Graph; +use crate::util::graph::{self, Graph}; use super::encode::Metadata; @@ -18,9 +19,9 @@ use super::encode::Metadata; #[derive(PartialEq)] pub struct Resolve { /// A graph, whose vertices are packages and edges are dependency specifications - /// from `Cargo.toml`. We need a `Vec` here because the same package + /// from `Cargo.toml`. We may have more then one here because the same package /// might be present in both `[dependencies]` and `[build-dependencies]`. - graph: Graph>, + graph: Graph, /// Replacements from the `[replace]` table. replacements: HashMap, /// Inverted version of `replacements`. @@ -67,7 +68,7 @@ pub enum ResolveVersion { impl Resolve { pub fn new( - graph: Graph>, + graph: Graph, replacements: HashMap, features: HashMap>, checksums: HashMap>, @@ -82,7 +83,8 @@ impl Resolve { let public_deps = graph .edges(p) .filter(|(_, deps)| { - deps.iter() + (*deps) + .clone() .any(|d| d.kind() == Kind::Normal && d.is_public()) }) .map(|(dep_package, _)| *dep_package) @@ -232,7 +234,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated pub fn contains(&self, k: &Q) -> bool where PackageId: Borrow, - Q: Ord + Eq, + Q: Hash + Eq, { self.graph.contains(k) } @@ -245,7 +247,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated self.graph.iter().cloned() } - pub fn deps(&self, pkg: PackageId) -> impl Iterator { + pub fn deps( + &self, + pkg: PackageId, + ) -> impl Iterator)> { self.deps_not_replaced(pkg) .map(move |(id, deps)| (self.replacement(id).unwrap_or(id), deps)) } @@ -253,10 +258,8 @@ unable to verify that `{0}` is the same as when the lockfile was generated pub fn deps_not_replaced( &self, pkg: PackageId, - ) -> impl Iterator { - self.graph - .edges(&pkg) - .map(|(id, deps)| (*id, deps.as_slice())) + ) -> impl Iterator)> { + self.graph.edges(&pkg).map(|(id, deps)| (*id, deps)) } pub fn replacement(&self, pkg: PackageId) -> Option { @@ -306,14 +309,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated to: PackageId, to_target: &Target, ) -> CargoResult { - let deps = if from == to { - &[] - } else { - self.dependencies_listed(from, to) - }; + let deps = self.dependencies_listed(from, to); let crate_name = to_target.crate_name(); - let mut names = deps.iter().map(|d| { + let mut names = deps.map(|d| { d.explicit_name_in_toml() .map(|s| s.as_str().replace("-", "_")) .unwrap_or_else(|| crate_name.clone()) @@ -330,7 +329,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated Ok(name) } - fn dependencies_listed(&self, from: PackageId, to: PackageId) -> &[Dependency] { + fn dependencies_listed(&self, from: PackageId, to: PackageId) -> graph::Edges<'_, Dependency> { // We've got a dependency on `from` to `to`, but this dependency edge // may be affected by [replace]. If the `to` package is listed as the // target of a replacement (aka the key of a reverse replacement map) @@ -341,14 +340,16 @@ unable to verify that `{0}` is the same as when the lockfile was generated // that's where the dependency originates from, and we only replace // targets of dependencies not the originator. if let Some(replace) = self.reverse_replacements.get(&to) { - if let Some(deps) = self.graph.edge(&from, replace) { + let deps = self.graph.edge(&from, replace); + if !deps.is_empty() { return deps; } } - match self.graph.edge(&from, &to) { - Some(ret) => ret, - None => panic!("no Dependency listed for `{}` => `{}`", from, to), + let deps = self.graph.edge(&from, &to); + if deps.is_empty() && from != to { + panic!("no Dependency listed for `{}` => `{}`", from, to); } + deps } /// Returns the version of the encoding that's being used for this lock diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs index 43835374ab8..1b2023550ec 100644 --- a/src/cargo/ops/cargo_fetch.rs +++ b/src/cargo/ops/cargo_fetch.rs @@ -41,8 +41,8 @@ pub fn fetch<'a>( to_download.push(id); let deps = resolve .deps(id) - .filter(|&(_id, deps)| { - deps.iter().any(|d| { + .filter(|(_id, deps)| { + (*deps).clone().any(|d| { // If no target was specified then all dependencies can // be fetched. let target = match options.target { diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs index 00f58a292fb..492191ee97a 100644 --- a/src/cargo/util/graph.rs +++ b/src/cargo/util/graph.rs @@ -1,52 +1,307 @@ +//! This module implements some Graph data structures optimized for Cargo's uses. +//! The module documentation here will try to justify why Cargo needs the odd set of requirements +//! that the structures provide, but the how is documented on each type. +//! +//! Cargo uses these types for several things in the codebase, but by far the most important is +//! the "dependency graph". It is a graph that records for each package all the packages that it +//! depends on. It is used to decide what order to build the packages, for what targets, and when +//! they need to be rebuilt (among many other things). One small complication is that we may have +//! more then one edge between the same two packages because the same package might be present +//! in both `[dependencies]` and `[build-dependencies]`. +//! This module must provide a way to read it efficiently with no fuss nor complications. The +//! "dependency graph" is constructed in two ways, when reading a lockfile and by running the resolver. +//! When reading a lockfile the only complication is that the lockfile has what is depended on but +//! not why. This module must provide a way to link two packages with out providing an example +//! of the type of the edge. +//! Running the resolver is the source of our most unusual requirements. +//! The fundamental pattern of the resolver is like: +//! +//! ```ignore +//! fn resolve_next(dependency_graph_so_far: _, mut unresolved_dependencies: _) -> Result { +//! if let Some(unresolved) = unresolved_dependencies.pop() { +//! let combined_problem = Problem::new(); +//! for candidate in unresolved { +//! let mut dgsf = dependency_graph_so_far.clone(); +//! let mut ud = unresolved_dependencies.clone(); +//! if let Err(prob) = activate(&mut dgsf, &mut ud, candidate) { +//! combined_problem.extend(prob); +//! continue +//! }; +//! match resolve_next(dgsf, ud) { +//! Ok(dg) => return Ok(dg), +//! Err(prob) => { +//! if dependency_graph_so_far.has(prob) { +//! return Err(prob) +//! } +//! } +//! } +//! } +//! Err(combined_problem) +//! } else { +//! Ok(finalize(dependency_graph_so_far)) +//! } +//! } +//! +//! fn activate(&mut dependency_graph: _, &mut unresolved_dependencies: _, candidate:_) -> Result<(), Problem> { +//! // check that we can use candidate given the other selected packages +//! if semver conflict { return Err(Problem::Semver); }; +//! if links conflict { return Err(Problem::Links); }; +//! ... +//! // update other state +//! if used a [replace] section { update that lookup table }; +//! if used a [patch] section { update that lookup table }; +//! update activated features +//! ... +//! // add candidate to dependency_graph +//! dependency_graph.add_edge(candidate.parent, candidate.id, candidate.dependency); +//! // add candidate's dependencies to the list to be resolved +//! unresolved_dependencies.extend(candidate.dependencies); +//! Ok(()) +//! } +//! ``` +//! +//! The real resolver is not recursive to avoid blowing the stack. The most expensive +//! (non recursive) call in this algorithm is the `dependency_graph_so_far.clone();`. +//! To make this more annoying the first thing we try will probably work, and any work we do to +//! prepare for the next iteration is wasted. If we had a `undo_activate` we could be much more efficient, +//! completely remove the `.clone()` and just `undo_activate` if things tern out to not work. +//! Unfortunately, making sure `undo_activate` handles all the corner cases correctly is not +//! practical for the resolver. However, this is possible for a graph like thing, a `clone` means +//! record the size of the graph. a `&mut self` method means undo everything newer and do the +//! mutation, a `&self` method means only look at the older part. This module provides a +//! `StackGraph` type to encapsulate this pattern. + +use indexmap::IndexMap; use std::borrow::Borrow; -use std::collections::BTreeSet; +use std::cell::{Ref, RefCell, RefMut}; +use std::collections::{BTreeMap, HashSet}; use std::fmt; +use std::hash::Hash; +use std::rc::Rc; + +type EdgeIndex = usize; -use im_rc; +/// This stores one Edge or Link element. If the two Nodes have data to connect them then we say it +/// is an Edge and the `value` is `Some(E)`, if the two Nodes did not have data then we say it is a +/// Link and the `value` is `None`. If there are other `EdgeLink`s associated with the same two Nodes, +/// then we make a index based doubly linked list using the `next` and `previous` fields. +#[derive(Clone, Debug)] +struct EdgeLink { + value: Option, + /// the index into the edge list of the next edge related to the same (from, to) nodes + next: Option, // can be `NonZeroUsize` but not worth the boilerplate + /// the index into the edge list of the previous edge related to the same (from, to) nodes + previous: Option, +} +/// This is a directed Graph structure. Each edge can have an `E` associated with it, +/// but may have more then one or none. Furthermore, it is designed to be "append only" so that +/// it can be queried as it would have been when it was smaller. This allows a `reset_to` method +/// that efficiently undoes the most reason modifications. +#[derive(Clone)] pub struct Graph { - nodes: im_rc::OrdMap>, + /// An index based linked list of the edge data. This maintains insertion order. + /// If the connection has data associated with it then that Edge is stored here. + /// If the connection has no data associated with it then that Link is represented with a dummy here. + edges: Vec>, + /// A map from a "from" node to (a map of "to" node to the starting index in the `edges`). + /// The inner map may be empty to represent a Node that was `add` but not connected to anything. + /// The maps are `IndexMap`s so they maintains insertion order. + nodes: indexmap::IndexMap>, +} + +/// All the data needed to query the prefix of a `Graph`. The only way for eny of the `len_` in +/// this to decrease is to call `reset_to`. All other modifications of a graph will increase at +/// least one of the `len_`. +#[derive(Copy, Clone, PartialEq, Debug, Ord, PartialOrd, Eq)] +struct GraphAge { + /// The number of stored edges, increased when `add_edge` or `link` is called. + len_edges: usize, + /// The number of stored nodes, increased when `add` is called or + /// if `add_edge` or `link` is called with a previously unseen node. + len_nodes: usize, +} + +impl Graph { + fn len(&self) -> GraphAge { + GraphAge { + len_edges: self.edges.len(), + len_nodes: self.nodes.len(), + } + } } -impl Graph { +impl Graph { pub fn new() -> Graph { Graph { - nodes: im_rc::OrdMap::new(), + edges: vec![], + nodes: Default::default(), + } + } + + /// This resets a Graph to the same state as it was when the passed `age` was made. + /// All the other `&mut` methods are guaranteed to increase the `len` of the Graph. + /// So the reset can be accomplished by throwing out all newer items and fixing internal pointers. + fn reset_to(&mut self, age: GraphAge) { + // the prefix we are resetting to had `age.len_nodes`, so remove all newer nodes + assert!(self.nodes.len() >= age.len_nodes); + // IndexMap dose not have a `truncate` so we roll our own + while self.nodes.len() > age.len_nodes { + self.nodes.pop(); + } + + // the prefix we are resetting to had `age.len_edges`, so remove all links pointing to newer edges + for (_, lookup) in self.nodes.iter_mut() { + // IndexMap dose not have a `last` so we roll our own + while lookup.len() >= 1 + && lookup + .get_index(lookup.len() - 1) + .filter(|(_, idx)| idx >= &&age.len_edges) + .is_some() + { + lookup.pop(); + } + } + + // the prefix we are resetting to had `age.len_edges`, so + assert!(self.edges.len() >= age.len_edges); + // remove all newer edges and record the references that need to be fixed + let to_fix: Vec = self + .edges + .drain(age.len_edges..) + .filter_map(|e| e.previous) + .filter(|idx| idx < &age.len_edges) + .collect(); + + // fix references into the newer edges we remove + for idx in to_fix { + self.edges[idx].next = None; } + assert_eq!(self.len(), age); } pub fn add(&mut self, node: N) { - self.nodes.entry(node).or_insert_with(im_rc::OrdMap::new); + // IndexMap happens to do exactly what we need to keep the ordering correct. + // This can be undone as it will increase the `len_nodes` if `node` is new. + self.nodes.entry(node).or_insert_with(IndexMap::new); } - pub fn link(&mut self, node: N, child: N) -> &mut E { - self.nodes + /// connect `node`to `child` with out associating any data. + pub fn link(&mut self, node: N, child: N) { + use indexmap::map::Entry; + // IndexMap happens to do exactly what we need to keep the ordering correct. + // This can be undone as it will increase the `len_nodes` if `node` is new. + match self + .nodes .entry(node) - .or_insert_with(im_rc::OrdMap::new) + .or_insert_with(IndexMap::new) .entry(child) - .or_insert_with(Default::default) + { + Entry::Vacant(entry) => { + // add the new edge and link + // This can be undone as it will increase the `len_edges`. + self.edges.push(EdgeLink { + value: None, + next: None, + previous: None, + }); + let edge_index: EdgeIndex = self.edges.len() - 1; + entry.insert(edge_index); + } + Entry::Occupied(_) => { + // this pair is already linked + } + }; + } + + /// connect `node`to `child` associating it with `edge`. + pub fn add_edge(&mut self, node: N, child: N, edge: E) { + use indexmap::map::Entry; + let edge = Some(edge); + // IndexMap happens to do exactly what we need to keep the ordering correct. + // This can be undone as it will increase the `len_nodes` if `node` is new. + match self + .nodes + .entry(node) + .or_insert_with(IndexMap::new) + .entry(child) + { + Entry::Vacant(entry) => { + // add the new edge and link + // This can be undone as it will increase the `len_edges`. + self.edges.push(EdgeLink { + value: edge, + next: None, + previous: None, + }); + let edge_index: EdgeIndex = self.edges.len() - 1; + entry.insert(edge_index); + } + Entry::Occupied(entry) => { + // this pare is already linked + let mut edge_index = *entry.get(); + loop { + // follow the linked list + if self.edges[edge_index].value == edge { + return; + } + if self.edges[edge_index].next.is_none() { + // we found the end, add the new edge + // This can be undone as it will increase the `len_edges`. + self.edges.push(EdgeLink { + value: edge, + next: None, + previous: Some(edge_index), + }); + let new_index: EdgeIndex = self.edges.len() - 1; + // make the list point to the new edge + self.edges[edge_index].next = Some(new_index); + return; + } + edge_index = self.edges[edge_index].next.unwrap(); + } + } + } } pub fn contains(&self, k: &Q) -> bool where N: Borrow, - Q: Ord + Eq, + Q: Hash + Eq, { self.nodes.contains_key(k) } - pub fn edge(&self, from: &N, to: &N) -> Option<&E> { - self.nodes.get(from)?.get(to) + pub fn iter(&self) -> impl Iterator { + self.borrow().nodes.keys() + } + + pub fn edge(&self, from: &N, to: &N) -> Edges<'_, E> { + Edges { + graph: &self.edges, + index: self.nodes.get(from).and_then(|x| x.get(to).copied()), + } } - pub fn edges(&self, from: &N) -> impl Iterator { - self.nodes.get(from).into_iter().flat_map(|x| x.iter()) + pub fn edges(&self, from: &N) -> impl Iterator)> { + let edges = &self.edges; + self.nodes.get(from).into_iter().flat_map(move |x| { + x.iter().map(move |(to, idx)| { + ( + to, + Edges { + graph: edges, + index: Some(*idx), + }, + ) + }) + }) } /// A topological sort of the `Graph` pub fn sort(&self) -> Vec { let mut ret = Vec::new(); - let mut marks = BTreeSet::new(); + let mut marks = HashSet::new(); for node in self.nodes.keys() { self.sort_inner_visit(node, &mut ret, &mut marks); @@ -55,26 +310,24 @@ impl Graph { ret } - fn sort_inner_visit(&self, node: &N, dst: &mut Vec, marks: &mut BTreeSet) { + fn sort_inner_visit(&self, node: &N, dst: &mut Vec, marks: &mut HashSet) { if !marks.insert(node.clone()) { return; } - for child in self.nodes[node].keys() { - self.sort_inner_visit(child, dst, marks); + if let Some(nodes) = self.nodes.get(node) { + for child in nodes.keys().rev() { + self.sort_inner_visit(child, dst, marks); + } } dst.push(node.clone()); } - pub fn iter(&self) -> impl Iterator { - self.nodes.keys() - } - /// Checks if there is a path from `from` to `to`. pub fn is_path_from_to<'a>(&'a self, from: &'a N, to: &'a N) -> bool { let mut stack = vec![from]; - let mut seen = BTreeSet::new(); + let mut seen = HashSet::new(); seen.insert(from); while let Some(iter) = stack.pop().and_then(|p| self.nodes.get(p)) { for p in iter.keys() { @@ -94,11 +347,10 @@ impl Graph { pub fn path_to_bottom<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> { let mut result = vec![pkg]; while let Some(p) = self.nodes.get(pkg).and_then(|p| { - p.iter() + p.keys() // Note that we can have "cycles" introduced through dev-dependency // edges, so make sure we don't loop infinitely. - .find(|&(node, _)| !result.contains(&node)) - .map(|(ref p, _)| p) + .find(|node| !result.contains(node)) }) { result.push(p); pkg = p; @@ -114,13 +366,14 @@ impl Graph { // it's used for! let mut result = vec![pkg]; let first_pkg_depending_on = |pkg: &N, res: &[&N]| { - self.nodes + self.borrow() + .nodes .iter() .filter(|&(_, adjacent)| adjacent.contains_key(pkg)) // Note that we can have "cycles" introduced through dev-dependency // edges, so make sure we don't loop infinitely. .find(|&(node, _)| !res.contains(&node)) - .map(|(ref p, _)| p) + .map(|(p, _)| p) }; while let Some(p) = first_pkg_depending_on(pkg, &result) { result.push(p); @@ -130,21 +383,26 @@ impl Graph { } } -impl Default for Graph { +impl Default for Graph { fn default() -> Graph { Graph::new() } } -impl fmt::Debug for Graph { +impl fmt::Debug + for Graph +{ fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(fmt, "Graph {{")?; - for (n, e) in &self.nodes { - writeln!(fmt, " - {}", n)?; + for (from, e) in self.nodes.iter() { + writeln!(fmt, " - {}", from)?; - for n in e.keys() { - writeln!(fmt, " - {}", n)?; + for to in e.keys() { + writeln!(fmt, " - {}", to)?; + for edge in self.edge(from, to) { + writeln!(fmt, " - {:?}", edge)?; + } } } @@ -154,17 +412,333 @@ impl fmt::Debug for Graph { } } -impl PartialEq for Graph { +impl PartialEq for Graph { fn eq(&self, other: &Graph) -> bool { self.nodes.eq(&other.nodes) } } -impl Eq for Graph {} +impl Eq for Graph {} -impl Clone for Graph { - fn clone(&self) -> Graph { - Graph { - nodes: self.nodes.clone(), +/// This is a directed Graph structure, that builds on the `Graph`'s "append only" internals +/// to provide: +/// - `O(ln(number of clones))` clone, no dependence on the size of the graph +/// - the clone has no overhead to read the `Graph` as it was +/// - no overhead over using the `Graph` directly when modifying the biggest clone +/// Is this too good to be true? +/// - If a modification (`&mut` method) is done to a clone that is not the largest then a full `O(N)` +/// deep clone will happen internally. +pub struct StackGraph { + /// The `Graph` shared by all clones, this `StackGraph` refers to only the prefix of size `age` + inner: Rc>>, + /// The shared list of all extant references to the same `Graph`. + /// The largest one is allowed to reset and expend the `Graph` without doing a deep clone. + /// There can be more then one clone at the same age, so each age is associated with a count. + other_refs: Rc>>, + /// The size of the `Graph` that this `StackGraph` refers to. + age: GraphAge, +} + +#[test] +fn demonstrate_stack_graph_can_read_all_clones() { + let mut graph = StackGraph::new(); + let mut stack = Vec::new(); + graph.add_edge(1, 2, 1); + stack.push(graph.clone()); + graph.add_edge(2, 3, 2); + stack.push(graph.clone()); + graph.add_edge(2, 3, 3); + stack.push(graph.clone()); + // violate stack discipline, so a deep clone is needed + graph = stack[0].clone(); + graph.add_edge(2, 3, 4); + assert_eq!( + stack.iter().map(|g| g.contains(&2)).collect::>(), + [false, true, true] + ); + assert_eq!(stack[1].borrow().edge(&2, &3).collect::>(), [&2]); + assert_eq!(stack[2].borrow().edge(&2, &3).collect::>(), [&2, &3]); + assert_eq!(graph.borrow().edge(&2, &3).collect::>(), [&4]); +} + +impl Clone for StackGraph { + fn clone(&self) -> Self { + *RefCell::borrow_mut(&self.other_refs) + .entry(self.age) + .or_insert(0) += 1; + StackGraph { + inner: Rc::clone(&self.inner), + other_refs: Rc::clone(&self.other_refs), + age: self.age, } } } + +fn remove_from_other_refs(borrow: &mut RefMut<'_, BTreeMap>, age: GraphAge) { + if let std::collections::btree_map::Entry::Occupied(mut val) = borrow.entry(age) { + *val.get_mut() -= 1; + if val.get() == &0 { + val.remove(); + } + } +} + +impl Drop for StackGraph { + fn drop(&mut self) { + remove_from_other_refs(&mut RefCell::borrow_mut(&self.other_refs), self.age); + } +} + +impl StackGraph { + pub fn new() -> StackGraph { + let inner = Graph::new(); + let age = inner.len(); + let mut other_refs = BTreeMap::new(); + other_refs.insert(age, 1); + StackGraph { + inner: Rc::new(RefCell::new(inner)), + other_refs: Rc::new(RefCell::new(other_refs)), + age, + } + } + + pub fn borrow(&self) -> StackGraphView<'_, N, E> { + let inner = RefCell::borrow(&self.inner); + assert!(self.age.len_edges <= inner.len().len_edges, "The internal Graph was reset to something smaller before you tried to read from the StackGraphView"); + assert!(self.age.len_nodes <= inner.len().len_nodes, "The internal Graph was reset to something smaller before you tried to read from the StackGraphView"); + StackGraphView { + inner, + age: self.age, + } + } + + /// Gets mutable access to the inner `Graph`. Uses `other_refs` to determine if a deep clone is + /// needed, and runs `reset_to` to get the `Graph` into the state it was in. + /// + /// It is the responsibility of the caller to set `self.age` and call `add_to_other_refs` after the new age is determined. + fn activate(&mut self) -> RefMut<'_, Graph> { + let inner = if { + let mut borrow = RefCell::borrow_mut(&self.other_refs); + remove_from_other_refs(&mut borrow, self.age); + borrow + .keys() + .next_back() + .map(|a| a <= &self.age) + .unwrap_or(true) + } { + // we are the biggest clone, so we can add to inner directly. + let mut inner = RefCell::borrow_mut(&mut self.inner); + if inner.len() != self.age { + // clean up after the larger clone that has since been dropped + inner.reset_to(self.age); + } + inner + } else { + // a bigger clone still exists so do a deep clone. + self.other_refs = Rc::new(RefCell::new(BTreeMap::new())); + let new = Rc::make_mut(&mut self.inner); + let mut inner = RefCell::borrow_mut(new); + inner.reset_to(self.age); + inner + }; + inner + } + + fn add_to_other_refs(&mut self) { + *RefCell::borrow_mut(&self.other_refs) + .entry(self.age) + .or_insert(0) += 1; + } + + pub fn clone_into_graph(&self) -> Graph { + self.clone().activate().clone() + } + + pub fn add(&mut self, node: N) { + self.age = { + let mut g = self.activate(); + g.add(node); + g.len() + }; + self.add_to_other_refs(); + } + + /// connect `node`to `child` with out associating any data. + pub fn link(&mut self, node: N, child: N) { + self.age = { + let mut g = self.activate(); + g.link(node, child); + g.len() + }; + self.add_to_other_refs(); + } + + /// connect `node`to `child` associating it with `edge`. + pub fn add_edge(&mut self, node: N, child: N, edge: E) { + self.age = { + let mut g = self.activate(); + g.add_edge(node, child, edge); + g.len() + }; + self.add_to_other_refs(); + } +} + +/// Methods for viewing the prefix of a `Graph` as stored in a `StackGraph`. +/// Other views of the inner `Graph` may have added things after this `StackGraph` was created. +/// So, we need to filter everything to only the prefix recorded by `self.age`. +impl StackGraph { + pub fn contains(&self, k: &Q) -> bool + where + N: Borrow, + Q: Hash + Eq, + { + self.borrow() + .inner + .nodes + .get_full(k) + .filter(|(i, _, _)| *i < self.age.len_nodes) + .is_some() + } + + /// Checks if there is a path from `from` to `to`. + pub fn is_path_from_to<'a>(&'a self, from: &'a N, to: &'a N) -> bool { + let mut stack = vec![from]; + let mut seen = HashSet::new(); + let inner = &self.borrow().inner; + seen.insert(from); + while let Some((_, _, iter)) = stack.pop().and_then(|p| { + inner + .nodes + .get_full(p) + .filter(|(i, _, _)| *i < self.age.len_nodes) + }) { + for (p, idx) in iter.iter() { + if *idx < self.age.len_edges { + if p == to { + return true; + } + if seen.insert(p) { + stack.push(p); + } + } + } + } + false + } +} + +impl Default for StackGraph { + fn default() -> StackGraph { + StackGraph::new() + } +} + +impl fmt::Debug + for StackGraph +{ + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(fmt, "Graph {{")?; + + for (from, e) in self.borrow().inner.nodes.iter().take(self.age.len_nodes) { + writeln!(fmt, " - {}", from)?; + + for (to, idx) in e.iter() { + if *idx < self.age.len_edges { + writeln!(fmt, " - {}", to)?; + for edge in self.borrow().edge(from, to) { + writeln!(fmt, " - {:?}", edge)?; + } + } + } + } + + write!(fmt, "}}")?; + + Ok(()) + } +} + +/// And Iterator of the edges related to one link in a `Graph` or a `StackGraphView`. +#[derive(Clone, Debug)] +pub struct Edges<'a, E: Clone> { + /// The arena the linked list is stored in. If we are used with a `StackGraphView` + /// then this is only the related prefix of the arena. So we need to filter out + /// pointers past the part we are given. + graph: &'a [EdgeLink], + index: Option, +} + +impl<'a, E: Clone> Edges<'a, E> { + pub fn is_empty(&self) -> bool { + self.index + .and_then(|idx| self.graph.get(idx)) + .and_then(|l| l.value.as_ref()) + .is_none() + } +} + +impl<'a, E: Clone> Iterator for Edges<'a, E> { + type Item = &'a E; + + fn next(&mut self) -> Option<&'a E> { + while let Some(edge_link) = self.index.and_then(|idx| { + // Check that the `idx` points to something in `self.graph`. It may not if we are + // looking at a smaller prefix of a larger graph. + self.graph.get(idx) + }) { + self.index = edge_link.next; + if let Some(value) = edge_link.value.as_ref() { + return Some(value); + } + } + self.index = None; + None + } +} + +/// A RAII gard that allows getting `&` references to the prefix of a `Graph` as stored in a `StackGraph`. +/// Other clones of the inner `StackGraph` may have added things after this `StackGraph` was created +/// and before this `StackGraphView` was `.borrow()`ed. +/// So, we need to filter everything to only the prefix recorded by `self.age`. +pub struct StackGraphView<'a, N: Clone, E: Clone> { + inner: Ref<'a, Graph>, + age: GraphAge, +} + +impl<'a, N: Eq + Hash + Clone, E: Clone + PartialEq> StackGraphView<'a, N, E> { + pub fn iter(&self) -> impl Iterator { + self.inner.nodes.keys().take(self.age.len_nodes) + } + + pub fn edge(&self, from: &N, to: &N) -> Edges<'_, E> { + Edges { + graph: &self.inner.edges[..self.age.len_edges], + index: self + .inner + .nodes + .get(from) + .and_then(|x| x.get(to).copied()) + .filter(|idx| idx < &self.age.len_edges), + } + } + + pub fn edges(&self, from: &N) -> impl Iterator)> { + let edges = &self.inner.edges[..self.age.len_edges]; + self.inner + .nodes + .get_full(from) + .filter(|(i, _, _)| *i < self.age.len_nodes) + .into_iter() + .flat_map(move |(_, _, x)| { + x.iter().map(move |(to, idx)| { + ( + to, + Edges { + graph: edges, + index: Some(*idx), + }, + ) + }) + }) + } +} diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 94946ae36de..20d2c581cf9 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -8,7 +8,7 @@ pub use self::errors::{internal, process_error}; pub use self::errors::{CargoResult, CargoResultExt, CliResult, Test}; pub use self::errors::{CargoTestError, CliError, ProcessError}; pub use self::flock::{FileLock, Filesystem}; -pub use self::graph::Graph; +pub use self::graph::{Graph, StackGraph}; pub use self::hex::{hash_u64, short_hash, to_hex}; pub use self::into_url::IntoUrl; pub use self::into_url_with_base::IntoUrlWithBase;