From 9b75a2bcd10918a8cde91962a1998615123a401e Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 17 Aug 2015 20:26:22 +0300 Subject: [PATCH 1/4] make trait matching smarter with projections also, use the right caching logic for type_moves_by_default (this was broken by @jroesch). before: 593.10user 5.21system 7:51.41elapsed 126%CPU (0avgtext+0avgdata 1150016maxresident)k llvm: 427.045 after: 577.76user 4.27system 7:36.13elapsed 127%CPU (0avgtext+0avgdata 1141608maxresident)k llvm: 431.001 --- src/librustc/middle/fast_reject.rs | 10 ++++++---- src/librustc/middle/infer/mod.rs | 12 +++++++++--- src/librustc/middle/ty.rs | 16 +++++++++++++++- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/librustc/middle/fast_reject.rs b/src/librustc/middle/fast_reject.rs index 8ff81635416a4..11d9512034440 100644 --- a/src/librustc/middle/fast_reject.rs +++ b/src/librustc/middle/fast_reject.rs @@ -85,11 +85,13 @@ pub fn simplify_type(tcx: &ty::ctxt, ty::TyBareFn(_, ref f) => { Some(FunctionSimplifiedType(f.sig.0.inputs.len())) } - ty::TyProjection(_) => { - None - } - ty::TyParam(_) => { + ty::TyProjection(_) | ty::TyParam(_) => { if can_simplify_params { + // In normalized types, projections don't unify with + // anything. when lazy normalization happens, this + // will change. It would still be nice to have a way + // to deal with known-not-to-unify-with-anything + // projections (e.g. the likes of <__S as Encoder>::Error). Some(ParameterSimplifiedType) } else { None diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index 578b39af88cb4..1b47ce75bc50d 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -1456,9 +1456,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { pub fn type_moves_by_default(&self, ty: Ty<'tcx>, span: Span) -> bool { let ty = self.resolve_type_vars_if_possible(&ty); - !traits::type_known_to_meet_builtin_bound(self, ty, ty::BoundCopy, span) - // FIXME(@jroesch): should be able to use: - // ty.moves_by_default(&self.parameter_environment, span) + if ty.needs_infer() { + // this can get called from typeck (by euv), and moves_by_default + // rightly refuses to work with inference variables, but + // moves_by_default has a cache, which we want to use in other + // cases. + !traits::type_known_to_meet_builtin_bound(self, ty, ty::BoundCopy, span) + } else { + ty.moves_by_default(&self.parameter_environment, span) + } } pub fn node_method_ty(&self, method_call: ty::MethodCall) diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 967eb3ff74f49..9f5b24c6b765b 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3192,6 +3192,8 @@ impl<'tcx> TraitDef<'tcx> { } } + /// Iterate over every impl that could possibly match the + /// self-type `self_ty`. pub fn for_each_relevant_impl(&self, tcx: &ctxt<'tcx>, self_ty: Ty<'tcx>, @@ -3203,7 +3205,19 @@ impl<'tcx> TraitDef<'tcx> { f(impl_def_id); } - if let Some(simp) = fast_reject::simplify_type(tcx, self_ty, false) { + // simplify_type(.., false) basically replaces type parameters and + // projections with infer-variables. This is, of course, done on + // the impl trait-ref when it is instantiated, but not on the + // predicate trait-ref which is passed here. + // + // for example, if we match `S: Copy` against an impl like + // `impl Copy for Option`, we replace the type variable + // in `Option` with an infer variable, to `Option<_>` (this + // doesn't actually change fast_reject output), but we don't + // replace `S` with anything - this impl of course can't be + // selected, and as there are hundreds of similar impls, + // considering them would significantly harm performance. + if let Some(simp) = fast_reject::simplify_type(tcx, self_ty, true) { if let Some(impls) = self.nonblanket_impls.borrow().get(&simp) { for &impl_def_id in impls { f(impl_def_id); From 96e6b2fef82c13aa542713e7923d7d0a76bd698b Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 17 Aug 2015 21:17:16 +0300 Subject: [PATCH 2/4] use an FnvHashSet instead of an HashSet in fulfill this doesn't cause a measurable perf increase, but it makes callgrind output cleaner. Anyway, rustc should be using FNV everywhere. --- src/librustc/middle/traits/fulfill.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index 1fca66f137918..99e12b7428d2a 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -12,11 +12,10 @@ use middle::infer::InferCtxt; use middle::ty::{self, RegionEscape, Ty, HasTypeFlags}; use middle::wf; -use std::collections::HashSet; use std::fmt; use syntax::ast; use util::common::ErrorReported; -use util::nodemap::NodeMap; +use util::nodemap::{FnvHashSet, NodeMap}; use super::CodeAmbiguity; use super::CodeProjectionError; @@ -33,7 +32,7 @@ use super::Unimplemented; use super::util::predicate_for_builtin_bound; pub struct FulfilledPredicates<'tcx> { - set: HashSet<(RFC1214Warning, ty::Predicate<'tcx>)> + set: FnvHashSet<(RFC1214Warning, ty::Predicate<'tcx>)> } /// The fulfillment context is used to drive trait resolution. It @@ -540,7 +539,7 @@ fn register_region_obligation<'tcx>(t_a: Ty<'tcx>, impl<'tcx> FulfilledPredicates<'tcx> { pub fn new() -> FulfilledPredicates<'tcx> { FulfilledPredicates { - set: HashSet::new() + set: FnvHashSet() } } From 8aeaaac6545565345f4df7d24d6421f93f28d698 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 17 Aug 2015 23:50:24 +0300 Subject: [PATCH 3/4] add a fast-path to resolve_type_vars_if_possible this avoids needless substituting before: 577.76user 4.27system 7:36.13elapsed 127%CPU (0avgtext+0avgdata 1141608maxresident)k after: 573.01user 4.04system 7:33.86elapsed 127%CPU (0avgtext+0avgdata 1141656maxresident)k --- .../middle/infer/higher_ranked/mod.rs | 2 +- src/librustc/middle/infer/mod.rs | 9 ++++-- src/librustc/middle/traits/error_reporting.rs | 2 +- src/librustc/middle/traits/project.rs | 7 +++++ src/librustc/middle/ty.rs | 30 +++++++++++++++++++ src/librustc/middle/ty_relate/mod.rs | 4 +-- 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/librustc/middle/infer/higher_ranked/mod.rs b/src/librustc/middle/infer/higher_ranked/mod.rs index 1919d8ffd294d..5f2f4df2f1601 100644 --- a/src/librustc/middle/infer/higher_ranked/mod.rs +++ b/src/librustc/middle/infer/higher_ranked/mod.rs @@ -614,7 +614,7 @@ pub fn plug_leaks<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>, snapshot: &CombinedSnapshot, value: &T) -> T - where T : TypeFoldable<'tcx> + where T : TypeFoldable<'tcx> + ty::HasTypeFlags { debug_assert!(leak_check(infcx, &skol_map, snapshot).is_ok()); diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index 1b47ce75bc50d..d56a73c36570c 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -985,7 +985,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { snapshot: &CombinedSnapshot, value: &T) -> T - where T : TypeFoldable<'tcx> + where T : TypeFoldable<'tcx> + HasTypeFlags { /*! See `higher_ranked::plug_leaks` */ @@ -1256,7 +1256,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } - pub fn resolve_type_vars_if_possible>(&self, value: &T) -> T { + pub fn resolve_type_vars_if_possible(&self, value: &T) -> T + where T: TypeFoldable<'tcx> + HasTypeFlags + { /*! * Where possible, replaces type/int/float variables in * `value` with their final value. Note that region variables @@ -1266,6 +1268,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { * at will. */ + if !value.needs_infer() { + return value.clone(); // avoid duplicated subst-folding + } let mut r = resolve::OpportunisticTypeResolver::new(self); value.fold_with(&mut r) } diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index 467c752499b36..3be05c45c4de2 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -160,7 +160,7 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, pub fn report_overflow_error<'a, 'tcx, T>(infcx: &InferCtxt<'a, 'tcx>, obligation: &Obligation<'tcx, T>) -> ! - where T: fmt::Display + TypeFoldable<'tcx> + where T: fmt::Display + TypeFoldable<'tcx> + HasTypeFlags { let predicate = infcx.resolve_type_vars_if_possible(&obligation.predicate); diff --git a/src/librustc/middle/traits/project.rs b/src/librustc/middle/traits/project.rs index ef3a217ecdbf2..cacefbb1a8551 100644 --- a/src/librustc/middle/traits/project.rs +++ b/src/librustc/middle/traits/project.rs @@ -927,6 +927,13 @@ impl<'tcx, T: TypeFoldable<'tcx>> TypeFoldable<'tcx> for Normalized<'tcx, T> { } } +impl<'tcx, T: HasTypeFlags> HasTypeFlags for Normalized<'tcx, T> { + fn has_type_flags(&self, flags: ty::TypeFlags) -> bool { + self.value.has_type_flags(flags) || + self.obligations.has_type_flags(flags) + } +} + impl<'tcx, T:fmt::Debug> fmt::Debug for Normalized<'tcx, T> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Normalized({:?},{:?})", diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 9f5b24c6b765b..f99760b6210c5 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -7280,6 +7280,24 @@ impl<'tcx,T:HasTypeFlags> HasTypeFlags for VecPerParamSpace { } } +impl HasTypeFlags for abi::Abi { + fn has_type_flags(&self, _flags: TypeFlags) -> bool { + false + } +} + +impl HasTypeFlags for ast::Unsafety { + fn has_type_flags(&self, _flags: TypeFlags) -> bool { + false + } +} + +impl HasTypeFlags for BuiltinBounds { + fn has_type_flags(&self, _flags: TypeFlags) -> bool { + false + } +} + impl<'tcx> HasTypeFlags for ClosureTy<'tcx> { fn has_type_flags(&self, flags: TypeFlags) -> bool { self.sig.has_type_flags(flags) @@ -7292,6 +7310,12 @@ impl<'tcx> HasTypeFlags for ClosureUpvar<'tcx> { } } +impl<'tcx> HasTypeFlags for ExistentialBounds<'tcx> { + fn has_type_flags(&self, flags: TypeFlags) -> bool { + self.projection_bounds.has_type_flags(flags) + } +} + impl<'tcx> HasTypeFlags for ty::InstantiatedPredicates<'tcx> { fn has_type_flags(&self, flags: TypeFlags) -> bool { self.predicates.has_type_flags(flags) @@ -7367,6 +7391,12 @@ impl<'tcx> HasTypeFlags for Ty<'tcx> { } } +impl<'tcx> HasTypeFlags for TypeAndMut<'tcx> { + fn has_type_flags(&self, flags: TypeFlags) -> bool { + self.ty.has_type_flags(flags) + } +} + impl<'tcx> HasTypeFlags for TraitRef<'tcx> { fn has_type_flags(&self, flags: TypeFlags) -> bool { self.substs.has_type_flags(flags) diff --git a/src/librustc/middle/ty_relate/mod.rs b/src/librustc/middle/ty_relate/mod.rs index 05b08b356efdf..f307f674732a1 100644 --- a/src/librustc/middle/ty_relate/mod.rs +++ b/src/librustc/middle/ty_relate/mod.rs @@ -14,7 +14,7 @@ //! type equality, etc. use middle::subst::{ErasedRegions, NonerasedRegions, ParamSpace, Substs}; -use middle::ty::{self, Ty, TypeError}; +use middle::ty::{self, HasTypeFlags, Ty, TypeError}; use middle::ty_fold::TypeFoldable; use std::rc::Rc; use syntax::abi; @@ -78,7 +78,7 @@ pub trait TypeRelation<'a,'tcx> : Sized { where T: Relate<'a,'tcx>; } -pub trait Relate<'a,'tcx>: TypeFoldable<'tcx> { +pub trait Relate<'a,'tcx>: TypeFoldable<'tcx> + HasTypeFlags { fn relate>(relation: &mut R, a: &Self, b: &Self) From 13809ffff7022a24b33d93f63a2cbdb6ecd20805 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 18 Aug 2015 00:17:02 +0300 Subject: [PATCH 4/4] don't iterate over all impls when none match before: 573.01user 4.04system 7:33.86elapsed 127%CPU (0avgtext+0avgdata 1141656maxresident)k after: 567.03user 4.00system 7:28.23elapsed 127%CPU (0avgtext+0avgdata 1133112maxresident)k an additional 1% improvement --- src/librustc/middle/ty.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index f99760b6210c5..69b83b24ffd21 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3222,13 +3222,12 @@ impl<'tcx> TraitDef<'tcx> { for &impl_def_id in impls { f(impl_def_id); } - return; // we don't need to process the other non-blanket impls } - } - - for v in self.nonblanket_impls.borrow().values() { - for &impl_def_id in v { - f(impl_def_id); + } else { + for v in self.nonblanket_impls.borrow().values() { + for &impl_def_id in v { + f(impl_def_id); + } } } }