mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

auto merge of #18386 : nikomatsakis/rust/issue-18208, r=pnkfelix

Browse Source 
Avoid O(n^2) performance by reconsidering the full set of obligations only when we are about to report an error (#18208). I found it is still important to consider the full set in order to make tests like `let x: Vec<_> = obligations.iter().collect()` work.

I think we lack the infrastructure to write a regression test for this, but when I did manual testing I found a massive reduction in type-checking time for extreme examples like those found in #18208 vs stage0.

f? @dotdash
This commit is contained in:
bors 2014-10-28 12:26:50 +00:00
commit 58dc0a05ab
4 changed files with 128 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
src/librustc/middle/traits/fulfill.rs
View File

@ -35,12 +35,18 @@ pub struct FulfillmentContext {
// A list of all obligations that have been registered with this
// fulfillment context.
trait_obligations: Vec<Obligation>,
// Remembers the count of trait obligations that we have already
// attempted to select. This is used to avoid repeating work
// when `select_new_obligations` is called.
attempted_mark: uint,
}
impl FulfillmentContext {
pub fn new() -> FulfillmentContext {
FulfillmentContext {
trait_obligations: Vec::new(),
attempted_mark: 0,
}
}
@ -74,18 +80,49 @@ impl FulfillmentContext {
}
}
pub fn select_new_obligations<'a,'tcx>(&mut self,
infcx: &InferCtxt<'a,'tcx>,
param_env: &ty::ParameterEnvironment,
typer: &Typer<'tcx>)
-> Result<(),Vec<FulfillmentError>>
{
/*!
* Attempts to select obligations that were registered since
* the call to a selection routine. This is used by the type checker
* to eagerly attempt to resolve obligations in hopes of gaining
* type information. It'd be equally valid to use `select_where_possible`
* but it results in `O(n^2)` performance (#18208).
*/
let mut selcx = SelectionContext::new(infcx, param_env, typer);
self.select(&mut selcx, true)
}
pub fn select_where_possible<'a,'tcx>(&mut self,
infcx: &InferCtxt<'a,'tcx>,
param_env: &ty::ParameterEnvironment,
typer: &Typer<'tcx>)
-> Result<(),Vec<FulfillmentError>>
{
let tcx = infcx.tcx;
let mut selcx = SelectionContext::new(infcx, param_env, typer);
self.select(&mut selcx, false)
}
debug!("select_where_possible({} obligations) start",
self.trait_obligations.len());
fn select(&mut self,
selcx: &mut SelectionContext,
only_new_obligations: bool)
-> Result<(),Vec<FulfillmentError>>
{
/*!
* Attempts to select obligations using `selcx`. If
* `only_new_obligations` is true, then it only attempts to
* select obligations that haven't been seen before.
*/
debug!("select({} obligations, only_new_obligations={}) start",
self.trait_obligations.len(),
only_new_obligations);
let tcx = selcx.tcx();
let mut errors = Vec::new();
loop {
@ -96,30 +133,47 @@ impl FulfillmentContext {
let mut selections = Vec::new();
// If we are only attempting obligations we haven't seen yet,
// then set `skip` to the number of obligations we've already
// seen.
let mut skip = if only_new_obligations {
self.attempted_mark
} else {
0
};
// First pass: walk each obligation, retaining
// only those that we cannot yet process.
self.trait_obligations.retain(|obligation| {
match selcx.select(obligation) {
Ok(None) => {
true
}
Ok(Some(s)) => {
selections.push(s);
false
}
Err(selection_err) => {
debug!("obligation: {} error: {}",
obligation.repr(tcx),
selection_err.repr(tcx));
errors.push(FulfillmentError::new(
(*obligation).clone(),
CodeSelectionError(selection_err)));
false
// Hack: Retain does not pass in the index, but we want
// to avoid processing the first `start_count` entries.
if skip > 0 {
skip -= 1;
true
} else {
match selcx.select(obligation) {
Ok(None) => {
true
}
Ok(Some(s)) => {
selections.push(s);
false
}
Err(selection_err) => {
debug!("obligation: {} error: {}",
obligation.repr(tcx),
selection_err.repr(tcx));
errors.push(FulfillmentError::new(
(*obligation).clone(),
CodeSelectionError(selection_err)));
false
}
}
}
});
self.attempted_mark = self.trait_obligations.len();
if self.trait_obligations.len() == count {
// Nothing changed.
break;
@ -133,7 +187,7 @@ impl FulfillmentContext {
}
}
debug!("select_where_possible({} obligations, {} errors) done",
debug!("select({} obligations, {} errors) done",
self.trait_obligations.len(),
errors.len());

4
src/librustc/middle/typeck/check/method.rs
View File

@ -88,7 +88,7 @@ use middle::ty;
use middle::typeck::astconv::AstConv;
use middle::typeck::check::{FnCtxt, NoPreference, PreferMutLvalue};
use middle::typeck::check::{impl_self_ty};
use middle::typeck::check::vtable::select_fcx_obligations_where_possible;
use middle::typeck::check::vtable::select_new_fcx_obligations;
use middle::typeck::check;
use middle::typeck::infer;
use middle::typeck::{MethodCall, MethodCallee};
@ -1302,7 +1302,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
// the `Self` trait).
let callee = self.confirm_candidate(rcvr_ty, &candidate);
select_fcx_obligations_where_possible(self.fcx);
select_new_fcx_obligations(self.fcx);
Some(Ok(callee))
}

51
src/librustc/middle/typeck/check/mod.rs
View File

@ -101,7 +101,6 @@ use middle::typeck::check::method::{AutoderefReceiver};
use middle::typeck::check::method::{CheckTraitsAndInherentMethods};
use middle::typeck::check::regionmanip::replace_late_bound_regions;
use middle::typeck::CrateCtxt;
use middle::typeck::infer::{resolve_type, force_tvar};
use middle::typeck::infer;
use middle::typeck::rscope::RegionScope;
use middle::typeck::{lookup_def_ccx};
@ -1412,7 +1411,7 @@ fn check_cast(fcx: &FnCtxt,
}
// casts from C-like enums are allowed
} else if t_1_is_char {
let t_e = fcx.infcx().resolve_type_vars_if_possible(t_e);
let t_e = fcx.infcx().shallow_resolve(t_e);
if ty::get(t_e).sty != ty::ty_uint(ast::TyU8) {
fcx.type_error_message(span, |actual| {
format!("only `u8` can be cast as \
@ -2564,7 +2563,7 @@ fn check_argument_types<'a>(fcx: &FnCtxt,
// an "opportunistic" vtable resolution of any trait
// bounds on the call.
if check_blocks {
vtable::select_fcx_obligations_where_possible(fcx);
vtable::select_new_fcx_obligations(fcx);
}
// For variadic functions, we don't have a declared type for all of
@ -2988,9 +2987,11 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
// 'else' branch.
let expected = match expected.only_has_type() {
ExpectHasType(ety) => {
match infer::resolve_type(fcx.infcx(), Some(sp), ety, force_tvar) {
Ok(rty) if !ty::type_is_ty_var(rty) => ExpectHasType(rty),
_ => NoExpectation
let ety = fcx.infcx().shallow_resolve(ety);
if !ty::type_is_ty_var(ety) {
ExpectHasType(ety)
} else {
NoExpectation
}
}
_ => NoExpectation
@ -4037,7 +4038,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
ast::ExprForLoop(ref pat, ref head, ref block, _) => {
check_expr(fcx, &**head);
let typ = lookup_method_for_for_loop(fcx, &**head, expr.id);
vtable::select_fcx_obligations_where_possible(fcx);
vtable::select_new_fcx_obligations(fcx);
let pcx = pat_ctxt {
fcx: fcx,
@ -5393,18 +5394,32 @@ pub fn instantiate_path(fcx: &FnCtxt,
// Resolves `typ` by a single level if `typ` is a type variable. If no
// resolution is possible, then an error is reported.
pub fn structurally_resolved_type(fcx: &FnCtxt, sp: Span, tp: ty::t) -> ty::t {
match infer::resolve_type(fcx.infcx(), Some(sp), tp, force_tvar) {
Ok(t_s) if !ty::type_is_ty_var(t_s) => t_s,
_ => {
fcx.type_error_message(sp, |_actual| {
"the type of this value must be known in this \
context".to_string()
}, tp, None);
demand::suptype(fcx, sp, ty::mk_err(), tp);
tp
}
pub fn structurally_resolved_type(fcx: &FnCtxt, sp: Span, mut ty: ty::t) -> ty::t {
// If `ty` is a type variable, see whether we already know what it is.
ty = fcx.infcx().shallow_resolve(ty);
// If not, try resolve pending fcx obligations. Those can shed light.
//
// FIXME(#18391) -- This current strategy can lead to bad performance in
// extreme cases. We probably ought to smarter in general about
// only resolving when we need help and only resolving obligations
// will actually help.
if ty::type_is_ty_var(ty) {
vtable::select_fcx_obligations_where_possible(fcx);
ty = fcx.infcx().shallow_resolve(ty);
}
// If not, error.
if ty::type_is_ty_var(ty) {
fcx.type_error_message(sp, |_actual| {
"the type of this value must be known in this \
context".to_string()
}, ty, None);
demand::suptype(fcx, sp, ty::mk_err(), ty);
ty = ty::mk_err();
}
ty
}
// Returns the one-level-deep structure of the given type.

18
src/librustc/middle/typeck/check/vtable.rs
View File

@ -339,6 +339,24 @@ pub fn select_fcx_obligations_where_possible(fcx: &FnCtxt) {
}
}
pub fn select_new_fcx_obligations(fcx: &FnCtxt) {
/*!
* Try to select any fcx obligation that we haven't tried yet,
* in an effort to improve inference. You could just call
* `select_fcx_obligations_where_possible` except that it leads
* to repeated work.
*/
match
fcx.inh.fulfillment_cx
.borrow_mut()
.select_new_obligations(fcx.infcx(), &fcx.inh.param_env, fcx)
{
Ok(()) => { }
Err(errors) => { report_fulfillment_errors(fcx, &errors); }
}
}
fn note_obligation_cause(fcx: &FnCtxt,
obligation: &Obligation) {
let tcx = fcx.tcx();