Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up opt_normalize_projection_type #50818

Merged
merged 2 commits into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,19 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
obligation.cause.span,
infer::LateBoundRegionConversionTime::HigherRankedType,
data);
let normalized = super::normalize_projection_type(
let mut obligations = vec![];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this specifically is hot, it'd be interesting to see if we could make this an Option argument since obligations appear to be unused in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular callsite is not hot. The hot callsite is the one within <TypeFolder for AssociatedTypeNormalizer>::fold_ty in librustc/traits/project.rs, which passes an existing vector.

let normalized_ty = super::normalize_projection_type(
&mut selcx,
obligation.param_env,
data.projection_ty,
obligation.cause.clone(),
0
0,
&mut obligations
);
if let Err(error) = self.at(&obligation.cause, obligation.param_env)
.eq(normalized.value, data.ty) {
.eq(normalized_ty, data.ty) {
values = Some(infer::ValuePairs::Types(ExpectedFound {
expected: normalized.value,
expected: normalized_ty,
found: data.ty,
}));
err_buf = error;
Expand Down
25 changes: 12 additions & 13 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,18 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
// FIXME(#20304) -- cache

let mut selcx = SelectionContext::new(infcx);
let normalized = project::normalize_projection_type(&mut selcx,
param_env,
projection_ty,
cause,
0);

for obligation in normalized.obligations {
self.register_predicate_obligation(infcx, obligation);
}

debug!("normalize_projection_type: result={:?}", normalized.value);

normalized.value
let mut obligations = vec![];
let normalized_ty = project::normalize_projection_type(&mut selcx,
param_env,
projection_ty,
cause,
0,
&mut obligations);
self.register_predicate_obligations(infcx, obligations);

debug!("normalize_projection_type: result={:?}", normalized_ty);

normalized_ty
}

/// Requires that `ty` must implement the trait with `def_id` in
Expand Down
145 changes: 90 additions & 55 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,14 @@ fn project_and_unify_type<'cx, 'gcx, 'tcx>(
debug!("project_and_unify_type(obligation={:?})",
obligation);

let Normalized { value: normalized_ty, mut obligations } =
let mut obligations = vec![];
let normalized_ty =
match opt_normalize_projection_type(selcx,
obligation.param_env,
obligation.predicate.projection_ty,
obligation.cause.clone(),
obligation.recursion_depth) {
obligation.recursion_depth,
&mut obligations) {
Some(n) => n,
None => return Ok(None),
};
Expand Down Expand Up @@ -386,16 +388,15 @@ impl<'a, 'b, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for AssociatedTypeNormalizer<'a,
// binder). It would be better to normalize in a
// binding-aware fashion.

let Normalized { value: normalized_ty, obligations } =
normalize_projection_type(self.selcx,
self.param_env,
data.clone(),
self.cause.clone(),
self.depth);
debug!("AssociatedTypeNormalizer: depth={} normalized {:?} to {:?} \
with {} add'l obligations",
self.depth, ty, normalized_ty, obligations.len());
self.obligations.extend(obligations);
let normalized_ty = normalize_projection_type(self.selcx,
self.param_env,
data.clone(),
self.cause.clone(),
self.depth,
&mut self.obligations);
debug!("AssociatedTypeNormalizer: depth={} normalized {:?} to {:?}, \
now with {} obligations",
self.depth, ty, normalized_ty, self.obligations.len());
normalized_ty
}

Expand Down Expand Up @@ -471,10 +472,12 @@ pub fn normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
param_env: ty::ParamEnv<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize)
-> NormalizedTy<'tcx>
depth: usize,
obligations: &mut Vec<PredicateObligation<'tcx>>)
-> Ty<'tcx>
{
opt_normalize_projection_type(selcx, param_env, projection_ty.clone(), cause.clone(), depth)
opt_normalize_projection_type(selcx, param_env, projection_ty.clone(), cause.clone(), depth,
obligations)
.unwrap_or_else(move || {
// if we bottom out in ambiguity, create a type variable
// and a deferred predicate to resolve this when more type
Expand All @@ -490,24 +493,29 @@ pub fn normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
});
let obligation = Obligation::with_depth(
cause, depth + 1, param_env, projection.to_predicate());
Normalized {
value: ty_var,
obligations: vec![obligation]
}
obligations.push(obligation);
ty_var
})
}

/// The guts of `normalize`: normalize a specific projection like `<T
/// as Trait>::Item`. The result is always a type (and possibly
/// additional obligations). Returns `None` in the case of ambiguity,
/// which indicates that there are unbound type variables.
///
/// This function used to return `Option<NormalizedTy<'tcx>>`, which contains a
/// `Ty<'tcx>` and an obligations vector. But that obligation vector was very
/// often immediately appended to another obligations vector. So now this
/// function takes an obligations vector and appends to it directly, which is
/// slightly uglier but avoids the need for an extra short-lived allocation.
fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
selcx: &'a mut SelectionContext<'b, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize)
-> Option<NormalizedTy<'tcx>>
depth: usize,
obligations: &mut Vec<PredicateObligation<'tcx>>)
-> Option<Ty<'tcx>>
{
let infcx = selcx.infcx();

Expand Down Expand Up @@ -579,7 +587,9 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
projection_ty);
selcx.infcx().report_overflow_error(&obligation, false);
}
Err(ProjectionCacheEntry::NormalizedTy(mut ty)) => {
Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
// This is the hottest path in this function.
//
// If we find the value in the cache, then return it along
// with the obligations that went along with it. Note
// that, when using a fulfillment context, these
Expand All @@ -596,29 +606,32 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
// Once we have inferred everything we need to know, we
// can ignore the `obligations` from that point on.
if !infcx.any_unresolved_type_vars(&ty.value) {
infcx.projection_cache.borrow_mut().complete(cache_key);
ty.obligations = vec![];
infcx.projection_cache.borrow_mut().complete_normalized(cache_key, &ty);
// No need to extend `obligations`.
} else {
obligations.extend(ty.obligations);
}

push_paranoid_cache_value_obligation(infcx,
param_env,
projection_ty,
cause,
depth,
&mut ty);

return Some(ty);
obligations.push(get_paranoid_cache_value_obligation(infcx,
param_env,
projection_ty,
cause,
depth));
return Some(ty.value);
}
Err(ProjectionCacheEntry::Error) => {
debug!("opt_normalize_projection_type: \
found error");
return Some(normalize_to_error(selcx, param_env, projection_ty, cause, depth));
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
obligations.extend(result.obligations);
return Some(result.value)
}
}

let obligation = Obligation::with_depth(cause.clone(), depth, param_env, projection_ty);
match project_type(selcx, &obligation) {
Ok(ProjectedTy::Progress(Progress { ty: projected_ty, mut obligations })) => {
Ok(ProjectedTy::Progress(Progress { ty: projected_ty,
obligations: mut projected_obligations })) => {
// if projection succeeded, then what we get out of this
// is also non-normalized (consider: it was derived from
// an impl, where-clause etc) and hence we must
Expand All @@ -627,10 +640,10 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
debug!("opt_normalize_projection_type: \
projected_ty={:?} \
depth={} \
obligations={:?}",
projected_obligations={:?}",
projected_ty,
depth,
obligations);
projected_obligations);

let result = if projected_ty.has_projections() {
let mut normalizer = AssociatedTypeNormalizer::new(selcx,
Expand All @@ -644,22 +657,22 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
normalized_ty,
depth);

obligations.extend(normalizer.obligations);
projected_obligations.extend(normalizer.obligations);
Normalized {
value: normalized_ty,
obligations,
obligations: projected_obligations,
}
} else {
Normalized {
value: projected_ty,
obligations,
obligations: projected_obligations,
}
};

let cache_value = prune_cache_value_obligations(infcx, &result);
infcx.projection_cache.borrow_mut().insert_ty(cache_key, cache_value);

Some(result)
obligations.extend(result.obligations);
Some(result.value)
}
Ok(ProjectedTy::NoProgress(projected_ty)) => {
debug!("opt_normalize_projection_type: \
Expand All @@ -670,7 +683,8 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
obligations: vec![]
};
infcx.projection_cache.borrow_mut().insert_ty(cache_key, result.clone());
Some(result)
// No need to extend `obligations`.
Some(result.value)
}
Err(ProjectionTyError::TooManyCandidates) => {
debug!("opt_normalize_projection_type: \
Expand All @@ -688,7 +702,9 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(

infcx.projection_cache.borrow_mut()
.error(cache_key);
Some(normalize_to_error(selcx, param_env, projection_ty, cause, depth))
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
obligations.extend(result.obligations);
Some(result.value)
}
}
}
Expand Down Expand Up @@ -737,7 +753,7 @@ fn prune_cache_value_obligations<'a, 'gcx, 'tcx>(infcx: &'a InferCtxt<'a, 'gcx,
/// may or may not be necessary -- in principle, all the obligations
/// that must be proven to show that `T: Trait` were also returned
/// when the cache was first populated. But there are some vague concerns,
/// and so we take the precatuionary measure of including `T: Trait` in
/// and so we take the precautionary measure of including `T: Trait` in
/// the result:
///
/// Concern #1. The current setup is fragile. Perhaps someone could
Expand All @@ -754,19 +770,21 @@ fn prune_cache_value_obligations<'a, 'gcx, 'tcx>(infcx: &'a InferCtxt<'a, 'gcx,
/// that may yet turn out to be wrong. This *may* lead to some sort
/// of trouble, though we don't have a concrete example of how that
/// can occur yet. But it seems risky at best.
fn push_paranoid_cache_value_obligation<'a, 'gcx, 'tcx>(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize,
result: &mut NormalizedTy<'tcx>)
fn get_paranoid_cache_value_obligation<'a, 'gcx, 'tcx>(
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize)
-> PredicateObligation<'tcx>
{
let trait_ref = projection_ty.trait_ref(infcx.tcx).to_poly_trait_ref();
let trait_obligation = Obligation { cause,
recursion_depth: depth,
param_env,
predicate: trait_ref.to_predicate() };
result.obligations.push(trait_obligation);
Obligation {
cause,
recursion_depth: depth,
param_env,
predicate: trait_ref.to_predicate(),
}
}

/// If we are projecting `<T as Trait>::Item`, but `T: Trait` does not
Expand Down Expand Up @@ -1682,6 +1700,23 @@ impl<'tcx> ProjectionCache<'tcx> {
}));
}

/// A specialized version of `complete` for when the key's value is known
/// to be a NormalizedTy.
pub fn complete_normalized(&mut self, key: ProjectionCacheKey<'tcx>, ty: &NormalizedTy<'tcx>) {
// We want to insert `ty` with no obligations. If the existing value
// already has no obligations (as is common) we can use `insert_noop`
// to do a minimal amount of work -- the HashMap insertion is skipped,
// and minimal changes are made to the undo log.
if ty.obligations.is_empty() {
self.map.insert_noop();
} else {
self.map.insert(key, ProjectionCacheEntry::NormalizedTy(Normalized {
value: ty.value,
obligations: vec![]
}));
}
}

/// Indicates that trying to normalize `key` resulted in
/// ambiguity. No point in trying it again then until we gain more
/// type information (in which case, the "fully resolved" key will
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_data_structures/snapshot_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ impl<K, V> SnapshotMap<K, V>
}
}

pub fn insert_noop(&mut self) {
if !self.undo_log.is_empty() {
self.undo_log.push(UndoLog::Noop);
}
}

pub fn remove(&mut self, key: K) -> bool {
match self.map.remove(&key) {
Some(old_value) => {
Expand Down
10 changes: 4 additions & 6 deletions src/librustc_traits/normalize_projection_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
// except according to those terms.

use rustc::infer::canonical::{Canonical, QueryResult};
use rustc::traits::{self, FulfillmentContext, Normalized, ObligationCause,
SelectionContext};
use rustc::traits::{self, FulfillmentContext, ObligationCause, SelectionContext};
use rustc::traits::query::{CanonicalProjectionGoal, NoSolution, normalize::NormalizationResult};
use rustc::ty::{ParamEnvAnd, TyCtxt};
use rustc_data_structures::sync::Lrc;
Expand All @@ -37,10 +36,9 @@ crate fn normalize_projection_ty<'tcx>(
let fulfill_cx = &mut FulfillmentContext::new();
let selcx = &mut SelectionContext::new(infcx);
let cause = ObligationCause::misc(DUMMY_SP, DUMMY_NODE_ID);
let Normalized {
value: answer,
obligations,
} = traits::normalize_projection_type(selcx, param_env, goal, cause, 0);
let mut obligations = vec![];
let answer =
traits::normalize_projection_type(selcx, param_env, goal, cause, 0, &mut obligations);
fulfill_cx.register_predicate_obligations(infcx, obligations);

// Now that we have fulfilled as much as we can, create a solution
Expand Down
Loading