Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
This effectively reverts rust-lang#43546 as it seems that it does affect performance more than the PR has anticipated. Follow-up changes from rust-lang#44269 are also reverted.

This also removes the deduplication code from rust-lang#48296 as duplications were primarily coming from rust-lang#43546 and after removing that code it probably doesn't worth paying the cost of using a hash map.
  • Loading branch information
ishitatsuyuki committed May 21, 2021
1 parent fc81ad2 commit a1815d7
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 57 deletions.
29 changes: 5 additions & 24 deletions compiler/rustc_infer/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub enum ProjectionCacheEntry<'tcx> {
Ambiguous,
Recur,
Error,
NormalizedTy(NormalizedTy<'tcx>),
NormalizedTy(Ty<'tcx>),
}

impl<'tcx> ProjectionCacheStorage<'tcx> {
Expand Down Expand Up @@ -149,7 +149,7 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
debug!("Not overwriting Recur");
return;
}
let fresh_key = map.insert(key, ProjectionCacheEntry::NormalizedTy(value));
let fresh_key = map.insert(key, ProjectionCacheEntry::NormalizedTy(value.value));
assert!(!fresh_key, "never started projecting `{:?}`", key);
}

Expand All @@ -160,9 +160,9 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
pub fn complete(&mut self, key: ProjectionCacheKey<'tcx>) {
let mut map = self.map();
let ty = match map.get(&key) {
Some(&ProjectionCacheEntry::NormalizedTy(ref ty)) => {
Some(&ProjectionCacheEntry::NormalizedTy(ty)) => {
debug!("ProjectionCacheEntry::complete({:?}) - completing {:?}", key, ty);
ty.value
ty
}
ref value => {
// Type inference could "strand behind" old cache entries. Leave
Expand All @@ -172,26 +172,7 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
}
};

map.insert(
key,
ProjectionCacheEntry::NormalizedTy(Normalized { value: ty, obligations: vec![] }),
);
}

/// 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 don't insert anything.
if !ty.obligations.is_empty() {
self.map().insert(
key,
ProjectionCacheEntry::NormalizedTy(Normalized {
value: ty.value,
obligations: vec![],
}),
);
}
map.insert(key, ProjectionCacheEntry::NormalizedTy(ty));
}

/// Indicates that trying to normalize `key` resulted in
Expand Down
16 changes: 5 additions & 11 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,18 +519,12 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
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
// obligations could in principle be ignored: they have
// already been registered when the cache entry was
// created (and hence the new ones will quickly be
// discarded as duplicated). But when doing trait
// evaluation this is not the case, and dropping the trait
// evaluations can causes ICEs (e.g., #43132).
// If we find the value in the cache, then the obligations
// have already been returned from the previous entry (and
// should therefore have been honored).
debug!(?ty, "found normalized ty");
obligations.extend(ty.obligations);
return Ok(Some(ty.value));

return Ok(Some(ty));
}
Err(ProjectionCacheEntry::Error) => {
debug!("opt_normalize_projection_type: found error");
Expand Down
22 changes: 0 additions & 22 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2083,28 +2083,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
});
}

// We are performing deduplication here to avoid exponential blowups
// (#38528) from happening, but the real cause of the duplication is
// unknown. What we know is that the deduplication avoids exponential
// amount of predicates being propagated when processing deeply nested
// types.
//
// This code is hot enough that it's worth avoiding the allocation
// required for the FxHashSet when possible. Special-casing lengths 0,
// 1 and 2 covers roughly 75-80% of the cases.
if obligations.len() <= 1 {
// No possibility of duplicates.
} else if obligations.len() == 2 {
// Only two elements. Drop the second if they are equal.
if obligations[0] == obligations[1] {
obligations.truncate(1);
}
} else {
// Three or more elements. Use a general deduplication process.
let mut seen = FxHashSet::default();
obligations.retain(|i| seen.insert(i.clone()));
}

obligations
}
}
Expand Down

0 comments on commit a1815d7

Please sign in to comment.