Skip to content

Commit 05f13a8

Browse files
nnethercotennethercote
committed
Make process_obligations() greedier.
`process_obligations()` adds new nodes, but it does not process these new nodes until the next time it is called. This commit changes it so that it does process these new nodes within the same call. This change reduces the number of calls to `process_obligations()` required to complete processing, sometimes giving significant speed-ups. The change required some changes to tests. - The output of `cycle-cache-err-60010.rs` is slightly different. - The unit tests required extra cases to handle the earlier processing of the added nodes. I mostly did these in the simplest possible way, by making the added nodes be ignored, thus giving outcomes the same as with the old behaviour. But I changed `success_in_grandchildren()` more extensively so that some obligations are completed earlier than they used to be.
1 parent caf0187 commit 05f13a8

File tree

3 files changed

+36
-11
lines changed

3 files changed

+36
-11
lines changed

‎src/librustc_data_structures/obligation_forest/mod.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,16 @@ impl<O: ForestObligation> ObligationForest<O> {
395395
let mut errors = vec![];
396396
let mut stalled = true;
397397

398-
for index in 0..self.nodes.len() {
398+
// Note that the loop body can append new nodes, and those new nodes
399+
// will then be processed by subsequent iterations of the loop.
400+
//
401+
// We can't use an iterator for the loop because `self.nodes` is
402+
// appended to and the borrow checker would complain. We also can't use
403+
// `for index in 0..self.nodes.len() { ... }` because the range would
404+
// be computed with the initial length, and we would miss the appended
405+
// nodes. Therefore we use a `while` loop.
406+
let mut index = 0;
407+
while index < self.nodes.len() {
399408
let node = &mut self.nodes[index];
400409

401410
debug!("process_obligations: node {} == {:?}", index, node);
@@ -406,6 +415,7 @@ impl<O: ForestObligation> ObligationForest<O> {
406415
// out of sync with `nodes`. It's not very common, but it does
407416
// happen, and code in `compress` has to allow for it.
408417
if node.state.get() != NodeState::Pending {
418+
index += 1;
409419
continue;
410420
}
411421
let result = processor.process_obligation(&mut node.obligation);
@@ -441,6 +451,7 @@ impl<O: ForestObligation> ObligationForest<O> {
441451
});
442452
}
443453
}
454+
index += 1;
444455
}
445456

446457
if stalled {

‎src/librustc_data_structures/obligation_forest/tests.rs

+21-9
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ fn push_pop() {
7070
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
7171
"B" => ProcessResult::Error("B is for broken"),
7272
"C" => ProcessResult::Changed(vec![]),
73+
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
7374
_ => unreachable!(),
7475
}
7576
}, |_| {}), DoCompleted::Yes);
@@ -94,6 +95,7 @@ fn push_pop() {
9495
"A.2" => ProcessResult::Unchanged,
9596
"A.3" => ProcessResult::Changed(vec!["A.3.i"]),
9697
"D" => ProcessResult::Changed(vec!["D.1", "D.2"]),
98+
"A.3.i" | "D.1" | "D.2" => ProcessResult::Unchanged,
9799
_ => unreachable!(),
98100
}
99101
}, |_| {}), DoCompleted::Yes);
@@ -113,6 +115,7 @@ fn push_pop() {
113115
"A.3.i" => ProcessResult::Changed(vec![]),
114116
"D.1" => ProcessResult::Changed(vec!["D.1.i"]),
115117
"D.2" => ProcessResult::Changed(vec!["D.2.i"]),
118+
"D.1.i" | "D.2.i" => ProcessResult::Unchanged,
116119
_ => unreachable!(),
117120
}
118121
}, |_| {}), DoCompleted::Yes);
@@ -161,35 +164,38 @@ fn success_in_grandchildren() {
161164
forest.process_obligations(&mut C(|obligation| {
162165
match *obligation {
163166
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
167+
"A.1" => ProcessResult::Changed(vec![]),
168+
"A.2" => ProcessResult::Changed(vec!["A.2.i", "A.2.ii"]),
169+
"A.3" => ProcessResult::Changed(vec![]),
170+
"A.2.i" | "A.2.ii" => ProcessResult::Unchanged,
164171
_ => unreachable!(),
165172
}
166173
}, |_| {}), DoCompleted::Yes);
167-
assert!(ok.unwrap().is_empty());
174+
let mut ok = ok.unwrap();
175+
ok.sort();
176+
assert_eq!(ok, vec!["A.1", "A.3"]);
168177
assert!(err.is_empty());
169178

170179
let Outcome { completed: ok, errors: err, .. } =
171180
forest.process_obligations(&mut C(|obligation| {
172181
match *obligation {
173-
"A.1" => ProcessResult::Changed(vec![]),
174-
"A.2" => ProcessResult::Changed(vec!["A.2.i", "A.2.ii"]),
175-
"A.3" => ProcessResult::Changed(vec![]),
182+
"A.2.i" => ProcessResult::Unchanged,
183+
"A.2.ii" => ProcessResult::Changed(vec![]),
176184
_ => unreachable!(),
177185
}
178186
}, |_| {}), DoCompleted::Yes);
179-
let mut ok = ok.unwrap();
180-
ok.sort();
181-
assert_eq!(ok, vec!["A.1", "A.3"]);
187+
assert_eq!(ok.unwrap(), vec!["A.2.ii"]);
182188
assert!(err.is_empty());
183189

184190
let Outcome { completed: ok, errors: err, .. } =
185191
forest.process_obligations(&mut C(|obligation| {
186192
match *obligation {
187193
"A.2.i" => ProcessResult::Changed(vec!["A.2.i.a"]),
188-
"A.2.ii" => ProcessResult::Changed(vec![]),
194+
"A.2.i.a" => ProcessResult::Unchanged,
189195
_ => unreachable!(),
190196
}
191197
}, |_| {}), DoCompleted::Yes);
192-
assert_eq!(ok.unwrap(), vec!["A.2.ii"]);
198+
assert!(ok.unwrap().is_empty());
193199
assert!(err.is_empty());
194200

195201
let Outcome { completed: ok, errors: err, .. } =
@@ -222,6 +228,7 @@ fn to_errors_no_throw() {
222228
forest.process_obligations(&mut C(|obligation| {
223229
match *obligation {
224230
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
231+
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
225232
_ => unreachable!(),
226233
}
227234
}, |_|{}), DoCompleted::Yes);
@@ -243,6 +250,7 @@ fn diamond() {
243250
forest.process_obligations(&mut C(|obligation| {
244251
match *obligation {
245252
"A" => ProcessResult::Changed(vec!["A.1", "A.2"]),
253+
"A.1" | "A.2" => ProcessResult::Unchanged,
246254
_ => unreachable!(),
247255
}
248256
}, |_|{}), DoCompleted::Yes);
@@ -254,6 +262,7 @@ fn diamond() {
254262
match *obligation {
255263
"A.1" => ProcessResult::Changed(vec!["D"]),
256264
"A.2" => ProcessResult::Changed(vec!["D"]),
265+
"D" => ProcessResult::Unchanged,
257266
_ => unreachable!(),
258267
}
259268
}, |_|{}), DoCompleted::Yes);
@@ -282,6 +291,7 @@ fn diamond() {
282291
forest.process_obligations(&mut C(|obligation| {
283292
match *obligation {
284293
"A'" => ProcessResult::Changed(vec!["A'.1", "A'.2"]),
294+
"A'.1" | "A'.2" => ProcessResult::Unchanged,
285295
_ => unreachable!(),
286296
}
287297
}, |_|{}), DoCompleted::Yes);
@@ -293,6 +303,7 @@ fn diamond() {
293303
match *obligation {
294304
"A'.1" => ProcessResult::Changed(vec!["D'", "A'"]),
295305
"A'.2" => ProcessResult::Changed(vec!["D'"]),
306+
"D'" | "A'" => ProcessResult::Unchanged,
296307
_ => unreachable!(),
297308
}
298309
}, |_|{}), DoCompleted::Yes);
@@ -370,6 +381,7 @@ fn orphan() {
370381
"B" => ProcessResult::Unchanged,
371382
"C1" => ProcessResult::Changed(vec![]),
372383
"C2" => ProcessResult::Changed(vec![]),
384+
"D" | "E" => ProcessResult::Unchanged,
373385
_ => unreachable!(),
374386
}
375387
}, |_|{}), DoCompleted::Yes);

‎src/test/ui/traits/cycle-cache-err-60010.stderr

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | _parse: <ParseQuery as Query<RootDatabase>>::Data,
66
|
77
= note: required because of the requirements on the impl of `Query<RootDatabase>` for `ParseQuery`
88

9-
error[E0275]: overflow evaluating the requirement `RootDatabase: SourceDatabase`
9+
error[E0275]: overflow evaluating the requirement `Runtime<RootDatabase>: std::panic::RefUnwindSafe`
1010
--> $DIR/cycle-cache-err-60010.rs:31:5
1111
|
1212
LL | type Storage;
@@ -17,6 +17,8 @@ LL | impl Database for RootDatabase {
1717
LL | type Storage = SalsaStorage;
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1919
|
20+
= note: required because it appears within the type `RootDatabase`
21+
= note: required because of the requirements on the impl of `SourceDatabase` for `RootDatabase`
2022
= note: required because of the requirements on the impl of `Query<RootDatabase>` for `ParseQuery`
2123
= note: required because it appears within the type `SalsaStorage`
2224

0 commit comments

Comments
 (0)