From dbc3c6e56fe4cc7c4692b18067b335fc63ca1ec8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 30 Oct 2018 17:48:48 +1100 Subject: [PATCH] Make `process_obligations`' computation of `completed` optional. It's only used in tests. This reduces instruction counts on several benchmarks by 0.5--1%. --- src/librustc/traits/fulfill.rs | 7 +- .../obligation_forest/mod.rs | 54 ++++++---- .../obligation_forest/test.rs | 99 ++++++++++--------- 3 files changed, 88 insertions(+), 72 deletions(-) diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index cfa77b210e857..0c692dd9a0661 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -12,8 +12,9 @@ use infer::InferCtxt; use mir::interpret::{GlobalId, ErrorHandled}; use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, ToPredicate}; use ty::error::ExpectedFound; -use rustc_data_structures::obligation_forest::{Error, ForestObligation, ObligationForest}; -use rustc_data_structures::obligation_forest::{ObligationProcessor, ProcessResult}; +use rustc_data_structures::obligation_forest::{DoCompleted, Error, ForestObligation}; +use rustc_data_structures::obligation_forest::{ObligationForest, ObligationProcessor}; +use rustc_data_structures::obligation_forest::{ProcessResult}; use std::marker::PhantomData; use hir::def_id::DefId; @@ -98,7 +99,7 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> { let outcome = self.predicates.process_obligations(&mut FulfillProcessor { selcx, register_region_obligations: self.register_region_obligations - }); + }, DoCompleted::No); debug!("select: outcome={:#?}", outcome); // FIXME: if we kept the original cache key, we could mark projection diff --git a/src/librustc_data_structures/obligation_forest/mod.rs b/src/librustc_data_structures/obligation_forest/mod.rs index ccf2a7f81590e..c211d888df131 100644 --- a/src/librustc_data_structures/obligation_forest/mod.rs +++ b/src/librustc_data_structures/obligation_forest/mod.rs @@ -162,8 +162,8 @@ enum NodeState { #[derive(Debug)] pub struct Outcome { /// Obligations that were completely evaluated, including all - /// (transitive) subobligations. - pub completed: Vec, + /// (transitive) subobligations. Only computed if requested. + pub completed: Option>, /// Backtrace of obligations that were found to be in error. pub errors: Vec>, @@ -177,6 +177,14 @@ pub struct Outcome { pub stalled: bool, } +/// Should `process_obligations` compute the `Outcome::completed` field of its +/// result? +#[derive(PartialEq)] +pub enum DoCompleted { + No, + Yes, +} + #[derive(Debug, PartialEq, Eq)] pub struct Error { pub error: E, @@ -282,8 +290,8 @@ impl ObligationForest { }); } } - let successful_obligations = self.compress(); - assert!(successful_obligations.is_empty()); + let successful_obligations = self.compress(DoCompleted::Yes); + assert!(successful_obligations.unwrap().is_empty()); errors } @@ -311,7 +319,8 @@ impl ObligationForest { /// be called in a loop until `outcome.stalled` is false. /// /// This CANNOT be unrolled (presently, at least). - pub fn process_obligations

(&mut self, processor: &mut P) -> Outcome + pub fn process_obligations

(&mut self, processor: &mut P, do_completed: DoCompleted) + -> Outcome where P: ObligationProcessor { debug!("process_obligations(len={})", self.nodes.len()); @@ -366,7 +375,7 @@ impl ObligationForest { // There's no need to perform marking, cycle processing and compression when nothing // changed. return Outcome { - completed: vec![], + completed: if do_completed == DoCompleted::Yes { Some(vec![]) } else { None }, errors, stalled, }; @@ -376,12 +385,12 @@ impl ObligationForest { self.process_cycles(processor); // Now we have to compress the result - let completed_obligations = self.compress(); + let completed = self.compress(do_completed); debug!("process_obligations: complete"); Outcome { - completed: completed_obligations, + completed, errors, stalled, } @@ -524,7 +533,7 @@ impl ObligationForest { /// Beforehand, all nodes must be marked as `Done` and no cycles /// on these nodes may be present. This is done by e.g. `process_cycles`. #[inline(never)] - fn compress(&mut self) -> Vec { + fn compress(&mut self, do_completed: DoCompleted) -> Option> { let nodes_len = self.nodes.len(); let mut node_rewrites: Vec<_> = self.scratch.take().unwrap(); node_rewrites.extend(0..nodes_len); @@ -573,21 +582,26 @@ impl ObligationForest { if dead_nodes == 0 { node_rewrites.truncate(0); self.scratch = Some(node_rewrites); - return vec![]; + return if do_completed == DoCompleted::Yes { Some(vec![]) } else { None }; } // Pop off all the nodes we killed and extract the success // stories. - let successful = (0..dead_nodes) - .map(|_| self.nodes.pop().unwrap()) - .flat_map(|node| { - match node.state.get() { - NodeState::Error => None, - NodeState::Done => Some(node.obligation), - _ => unreachable!() - } - }) - .collect(); + let successful = if do_completed == DoCompleted::Yes { + Some((0..dead_nodes) + .map(|_| self.nodes.pop().unwrap()) + .flat_map(|node| { + match node.state.get() { + NodeState::Error => None, + NodeState::Done => Some(node.obligation), + _ => unreachable!() + } + }) + .collect()) + } else { + self.nodes.truncate(self.nodes.len() - dead_nodes); + None + }; self.apply_rewrites(&node_rewrites); node_rewrites.truncate(0); diff --git a/src/librustc_data_structures/obligation_forest/test.rs b/src/librustc_data_structures/obligation_forest/test.rs index c27a65e34310f..2a418973fbda2 100644 --- a/src/librustc_data_structures/obligation_forest/test.rs +++ b/src/librustc_data_structures/obligation_forest/test.rs @@ -10,7 +10,7 @@ #![cfg(test)] -use super::{Error, ObligationForest, ObligationProcessor, Outcome, ProcessResult}; +use super::{Error, DoCompleted, ObligationForest, ObligationProcessor, Outcome, ProcessResult}; use std::fmt; use std::marker::PhantomData; @@ -84,8 +84,8 @@ fn push_pop() { "C" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, vec!["C"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["C"]); assert_eq!(err, vec![Error { error: "B is for broken", @@ -108,8 +108,8 @@ fn push_pop() { "D" => ProcessResult::Changed(vec!["D.1", "D.2"]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, Vec::<&'static str>::new()); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), Vec::<&'static str>::new()); assert_eq!(err, Vec::new()); @@ -127,8 +127,8 @@ fn push_pop() { "D.2" => ProcessResult::Changed(vec!["D.2.i"]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, vec!["A.3", "A.1", "A.3.i"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["A.3", "A.1", "A.3.i"]); assert_eq!(err, vec![Error { error: "A is for apple", @@ -143,8 +143,8 @@ fn push_pop() { "D.2.i" => ProcessResult::Changed(vec![]), _ => panic!("unexpected obligation {:?}", obligation), } - }, |_| {})); - assert_eq!(ok, vec!["D.2.i", "D.2"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["D.2.i", "D.2"]); assert_eq!(err, vec![Error { error: "D is for dumb", @@ -171,8 +171,8 @@ fn success_in_grandchildren() { "A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]), _ => unreachable!(), } - }, |_| {})); - assert!(ok.is_empty()); + }, |_| {}), DoCompleted::Yes); + assert!(ok.unwrap().is_empty()); assert!(err.is_empty()); let Outcome { completed: ok, errors: err, .. } = @@ -183,8 +183,8 @@ fn success_in_grandchildren() { "A.3" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, vec!["A.3", "A.1"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["A.3", "A.1"]); assert!(err.is_empty()); let Outcome { completed: ok, errors: err, .. } = @@ -194,8 +194,8 @@ fn success_in_grandchildren() { "A.2.ii" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, vec!["A.2.ii"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["A.2.ii"]); assert!(err.is_empty()); let Outcome { completed: ok, errors: err, .. } = @@ -204,14 +204,15 @@ fn success_in_grandchildren() { "A.2.i.a" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_| {})); - assert_eq!(ok, vec!["A.2.i.a", "A.2.i", "A.2", "A"]); + }, |_| {}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["A.2.i.a", "A.2.i", "A.2", "A"]); assert!(err.is_empty()); let Outcome { completed: ok, errors: err, .. } = - forest.process_obligations(&mut C(|_| unreachable!(), |_| {})); + forest.process_obligations(&mut C(|_| unreachable!(), |_| {}), + DoCompleted::Yes); - assert!(ok.is_empty()); + assert!(ok.unwrap().is_empty()); assert!(err.is_empty()); } @@ -227,8 +228,8 @@ fn to_errors_no_throw() { "A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let errors = forest.to_errors(()); assert_eq!(errors[0].backtrace, vec!["A.1", "A"]); @@ -248,8 +249,8 @@ fn diamond() { "A" => ProcessResult::Changed(vec!["A.1", "A.2"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let Outcome { completed: ok, errors: err, .. } = @@ -259,8 +260,8 @@ fn diamond() { "A.2" => ProcessResult::Changed(vec!["D"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let mut d_count = 0; @@ -270,9 +271,9 @@ fn diamond() { "D" => { d_count += 1; ProcessResult::Changed(vec![]) }, _ => unreachable!(), } - }, |_|{})); + }, |_|{}), DoCompleted::Yes); assert_eq!(d_count, 1); - assert_eq!(ok, vec!["D", "A.2", "A.1", "A"]); + assert_eq!(ok.unwrap(), vec!["D", "A.2", "A.1", "A"]); assert_eq!(err.len(), 0); let errors = forest.to_errors(()); @@ -285,8 +286,8 @@ fn diamond() { "A'" => ProcessResult::Changed(vec!["A'.1", "A'.2"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let Outcome { completed: ok, errors: err, .. } = @@ -296,8 +297,8 @@ fn diamond() { "A'.2" => ProcessResult::Changed(vec!["D'"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let mut d_count = 0; @@ -307,9 +308,9 @@ fn diamond() { "D'" => { d_count += 1; ProcessResult::Error("operation failed") }, _ => unreachable!(), } - }, |_|{})); + }, |_|{}), DoCompleted::Yes); assert_eq!(d_count, 1); - assert_eq!(ok.len(), 0); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err, vec![super::Error { error: "operation failed", backtrace: vec!["D'", "A'.1", "A'"] @@ -333,8 +334,8 @@ fn done_dependency() { "A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok, vec!["C: Sized", "B: Sized", "A: Sized"]); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["C: Sized", "B: Sized", "A: Sized"]); assert_eq!(err.len(), 0); forest.register_obligation("(A,B,C): Sized"); @@ -348,8 +349,8 @@ fn done_dependency() { ]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok, vec!["(A,B,C): Sized"]); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["(A,B,C): Sized"]); assert_eq!(err.len(), 0); } @@ -371,8 +372,8 @@ fn orphan() { "C2" => ProcessResult::Changed(vec![]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok, vec!["C2", "C1"]); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap(), vec!["C2", "C1"]); assert_eq!(err.len(), 0); let Outcome { completed: ok, errors: err, .. } = @@ -382,8 +383,8 @@ fn orphan() { "B" => ProcessResult::Changed(vec!["D"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err.len(), 0); let Outcome { completed: ok, errors: err, .. } = @@ -393,8 +394,8 @@ fn orphan() { "E" => ProcessResult::Error("E is for error"), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err, vec![super::Error { error: "E is for error", backtrace: vec!["E", "A"] @@ -406,8 +407,8 @@ fn orphan() { "D" => ProcessResult::Error("D is dead"), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err, vec![super::Error { error: "D is dead", backtrace: vec!["D"] @@ -431,8 +432,8 @@ fn simultaneous_register_and_error() { "B" => ProcessResult::Changed(vec!["A"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err, vec![super::Error { error: "An error", backtrace: vec!["A"] @@ -449,8 +450,8 @@ fn simultaneous_register_and_error() { "B" => ProcessResult::Changed(vec!["A"]), _ => unreachable!(), } - }, |_|{})); - assert_eq!(ok.len(), 0); + }, |_|{}), DoCompleted::Yes); + assert_eq!(ok.unwrap().len(), 0); assert_eq!(err, vec![super::Error { error: "An error", backtrace: vec!["A"]