Skip to content

Commit 980c163

Browse files
committed
Auto merge of #8908 - JoshMcguigan:dependency-queue-cost, r=alexcrichton
update dependency queue to consider cost for each node In support of #7437, this updates the dependency queue implementation to consider a non-fixed cost for each node. The behavior of this implementation should match the previous implementation if all units are assigned the same cost by the code which calls the `queue` method (which is what we do for now). In the future I can think of at least two ways these costs could be used: 1. Use some known constant value by default (say 100 as I've done in this PR), and allow the user to provide hints for certain units to influence compilation order (as requested in #7437). 2. Take timing data from a previous compilation (perhaps the json output of `cargo build -Ztimings=json`) and use the actual time required to compile a given unit (in milliseconds) as its cost. Any units not included in the provided timing data could perhaps have their cost set to the median cost of all crates included in the timing data.
2 parents 911595f + dd5aca3 commit 980c163

File tree

2 files changed

+67
-21
lines changed

2 files changed

+67
-21
lines changed

src/cargo/core/compiler/job_queue.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,11 @@ impl<'cfg> JobQueue<'cfg> {
370370
}
371371
}
372372

373-
self.queue.queue(unit.clone(), job, queue_deps);
373+
// For now we use a fixed placeholder value for the cost of each unit, but
374+
// in the future this could be used to allow users to provide hints about
375+
// relative expected costs of units, or this could be automatically set in
376+
// a smarter way using timing data from a previous compilation.
377+
self.queue.queue(unit.clone(), job, queue_deps, 100);
374378
*self.counts.entry(unit.pkg.package_id()).or_insert(0) += 1;
375379
Ok(())
376380
}

src/cargo/util/dependency_queue.rs

+62-20
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@ pub struct DependencyQueue<N: Hash + Eq, E: Hash + Eq, V> {
3131
/// easily indexable with just an `N`
3232
reverse_dep_map: HashMap<N, HashMap<E, HashSet<N>>>,
3333

34-
/// The total number of packages that are transitively waiting on this package
34+
/// The relative priority of this package. Higher values should be scheduled sooner.
3535
priority: HashMap<N, usize>,
36+
37+
/// An expected cost for building this package. Used to determine priority.
38+
cost: HashMap<N, usize>,
3639
}
3740

3841
impl<N: Hash + Eq, E: Hash + Eq, V> Default for DependencyQueue<N, E, V> {
@@ -48,6 +51,7 @@ impl<N: Hash + Eq, E: Hash + Eq, V> DependencyQueue<N, E, V> {
4851
dep_map: HashMap::new(),
4952
reverse_dep_map: HashMap::new(),
5053
priority: HashMap::new(),
54+
cost: HashMap::new(),
5155
}
5256
}
5357
}
@@ -63,7 +67,18 @@ impl<N: Hash + Eq + Clone, E: Eq + Hash + Clone, V> DependencyQueue<N, E, V> {
6367
///
6468
/// An optional `value` can also be associated with `key` which is reclaimed
6569
/// when the node is ready to go.
66-
pub fn queue(&mut self, key: N, value: V, dependencies: impl IntoIterator<Item = (N, E)>) {
70+
///
71+
/// The cost parameter can be used to hint at the relative cost of building
72+
/// this node. This implementation does not care about the units of this value, so
73+
/// the calling code is free to use whatever they'd like. In general, higher cost
74+
/// nodes are expected to take longer to build.
75+
pub fn queue(
76+
&mut self,
77+
key: N,
78+
value: V,
79+
dependencies: impl IntoIterator<Item = (N, E)>,
80+
cost: usize,
81+
) {
6782
assert!(!self.dep_map.contains_key(&key));
6883

6984
let mut my_dependencies = HashSet::new();
@@ -76,7 +91,8 @@ impl<N: Hash + Eq + Clone, E: Eq + Hash + Clone, V> DependencyQueue<N, E, V> {
7691
.or_insert_with(HashSet::new)
7792
.insert(key.clone());
7893
}
79-
self.dep_map.insert(key, (my_dependencies, value));
94+
self.dep_map.insert(key.clone(), (my_dependencies, value));
95+
self.cost.insert(key, cost);
8096
}
8197

8298
/// All nodes have been added, calculate some internal metadata and prepare
@@ -86,8 +102,19 @@ impl<N: Hash + Eq + Clone, E: Eq + Hash + Clone, V> DependencyQueue<N, E, V> {
86102
for key in self.dep_map.keys() {
87103
depth(key, &self.reverse_dep_map, &mut out);
88104
}
89-
self.priority = out.into_iter().map(|(n, set)| (n, set.len())).collect();
105+
self.priority = out
106+
.into_iter()
107+
.map(|(n, set)| {
108+
let total_cost =
109+
self.cost[&n] + set.iter().map(|key| self.cost[key]).sum::<usize>();
110+
(n, total_cost)
111+
})
112+
.collect();
90113

114+
/// Creates a flattened reverse dependency list. For a given key, finds the
115+
/// set of nodes which depend on it, including transitively. This is different
116+
/// from self.reverse_dep_map because self.reverse_dep_map only maps one level
117+
/// of reverse dependencies.
91118
fn depth<'a, N: Hash + Eq + Clone, E: Hash + Eq + Clone>(
92119
key: &N,
93120
map: &HashMap<N, HashMap<E, HashSet<N>>>,
@@ -123,16 +150,6 @@ impl<N: Hash + Eq + Clone, E: Eq + Hash + Clone, V> DependencyQueue<N, E, V> {
123150
/// A package is ready to be built when it has 0 un-built dependencies. If
124151
/// `None` is returned then no packages are ready to be built.
125152
pub fn dequeue(&mut self) -> Option<(N, V)> {
126-
// Look at all our crates and find everything that's ready to build (no
127-
// deps). After we've got that candidate set select the one which has
128-
// the maximum priority in the dependency graph. This way we should
129-
// hopefully keep CPUs hottest the longest by ensuring that long
130-
// dependency chains are scheduled early on in the build process and the
131-
// leafs higher in the tree can fill in the cracks later.
132-
//
133-
// TODO: it'd be best here to throw in a heuristic of crate size as
134-
// well. For example how long did this crate historically take to
135-
// compile? How large is its source code? etc.
136153
let next = self
137154
.dep_map
138155
.iter()
@@ -190,14 +207,14 @@ mod test {
190207
use super::DependencyQueue;
191208

192209
#[test]
193-
fn deep_first() {
210+
fn deep_first_equal_cost() {
194211
let mut q = DependencyQueue::new();
195212

196-
q.queue(1, (), vec![]);
197-
q.queue(2, (), vec![(1, ())]);
198-
q.queue(3, (), vec![]);
199-
q.queue(4, (), vec![(2, ()), (3, ())]);
200-
q.queue(5, (), vec![(4, ()), (3, ())]);
213+
q.queue(1, (), vec![], 1);
214+
q.queue(2, (), vec![(1, ())], 1);
215+
q.queue(3, (), vec![], 1);
216+
q.queue(4, (), vec![(2, ()), (3, ())], 1);
217+
q.queue(5, (), vec![(4, ()), (3, ())], 1);
201218
q.queue_finished();
202219

203220
assert_eq!(q.dequeue(), Some((1, ())));
@@ -214,4 +231,29 @@ mod test {
214231
q.finish(&4, &());
215232
assert_eq!(q.dequeue(), Some((5, ())));
216233
}
234+
235+
#[test]
236+
fn sort_by_highest_cost() {
237+
let mut q = DependencyQueue::new();
238+
239+
q.queue(1, (), vec![], 1);
240+
q.queue(2, (), vec![(1, ())], 1);
241+
q.queue(3, (), vec![], 4);
242+
q.queue(4, (), vec![(2, ()), (3, ())], 1);
243+
q.queue_finished();
244+
245+
assert_eq!(q.dequeue(), Some((3, ())));
246+
assert_eq!(q.dequeue(), Some((1, ())));
247+
assert_eq!(q.dequeue(), None);
248+
q.finish(&3, &());
249+
assert_eq!(q.dequeue(), None);
250+
q.finish(&1, &());
251+
assert_eq!(q.dequeue(), Some((2, ())));
252+
assert_eq!(q.dequeue(), None);
253+
q.finish(&2, &());
254+
assert_eq!(q.dequeue(), Some((4, ())));
255+
assert_eq!(q.dequeue(), None);
256+
q.finish(&4, &());
257+
assert_eq!(q.dequeue(), None);
258+
}
217259
}

0 commit comments

Comments
 (0)