Skip to content

Commit f988482

Browse files
borsbors
committed
Auto merge of #66408 - nnethercote:greedy-process_obligations, r=<try>
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. r? @nikomatsakis
2 parents 5e380b7 + 98a7454 commit f988482

File tree

3 files changed

+30
-11
lines changed

3 files changed

+30
-11
lines changed

‎src/librustc_data_structures/obligation_forest/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,10 @@ 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+
let mut index = 0;
401+
while index < self.nodes.len() {
399402
let node = &mut self.nodes[index];
400403

401404
debug!("process_obligations: node {} == {:?}", index, node);
@@ -406,6 +409,7 @@ impl<O: ForestObligation> ObligationForest<O> {
406409
// out of sync with `nodes`. It's not very common, but it does
407410
// happen, and code in `compress` has to allow for it.
408411
if node.state.get() != NodeState::Pending {
412+
index += 1;
409413
continue;
410414
}
411415
let result = processor.process_obligation(&mut node.obligation);
@@ -441,6 +445,7 @@ impl<O: ForestObligation> ObligationForest<O> {
441445
});
442446
}
443447
}
448+
index += 1;
444449
}
445450

446451
if stalled {

‎src/librustc_data_structures/obligation_forest/tests.rs

Lines changed: 21 additions & 9 deletions
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

Lines changed: 3 additions & 1 deletion
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)