Skip to content

Commit c260e83

Browse files
the8472the8472
committed
dedup obligations in opt_normalize_projection_type 
1 parent 7b3cd07 commit c260e83

File tree

5 files changed

+156
-59
lines changed

5 files changed

+156
-59
lines changed

‎compiler/rustc_infer/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#![feature(control_flow_enum)]
2424
#![feature(min_specialization)]
2525
#![feature(label_break_value)]
26+
#![feature(hash_raw_entry)]
2627
#![recursion_limit = "512"] // For rustdoc
2728

2829
#[macro_use]

‎compiler/rustc_infer/src/traits/mod.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ mod project;
88
mod structural_impls;
99
pub mod util;
1010

11+
use rustc_data_structures::fx::FxHashMap;
1112
use rustc_hir as hir;
1213
use rustc_middle::ty::error::{ExpectedFound, TypeError};
1314
use rustc_middle::ty::{self, Const, Ty, TyCtxt};
1415
use rustc_span::Span;
16+
use std::borrow::{Borrow, Cow};
17+
use std::collections::hash_map::RawEntryMut;
18+
use std::hash::Hash;
1519

1620
pub use self::FulfillmentErrorCode::*;
1721
pub use self::ImplSource::*;
@@ -95,6 +99,65 @@ pub enum FulfillmentErrorCode<'tcx> {
9599
CodeAmbiguity,
96100
}
97101

102+
pub struct ObligationsDedup<'a, 'tcx, T> {
103+
obligations: &'a mut Vec<Obligation<'tcx, T>>,
104+
}
105+
106+
impl<'a, 'tcx, T: 'tcx> ObligationsDedup<'a, 'tcx, T>
107+
where
108+
T: Clone + Hash + Eq,
109+
{
110+
pub fn from_vec(vec: &'a mut Vec<Obligation<'tcx, T>>) -> Self {
111+
ObligationsDedup { obligations: vec }
112+
}
113+
114+
pub fn extend<'b>(&mut self, iter: impl ExactSizeIterator<Item = Cow<'b, Obligation<'tcx, T>>>)
115+
where
116+
'tcx: 'b,
117+
{
118+
// obligation tracing has shown that initial batches added to an empty vec do not
119+
// contain any duplicates, so there's no need to attempt deduplication
120+
if self.obligations.is_empty() {
121+
*self.obligations = iter.into_iter().map(Cow::into_owned).collect();
122+
return;
123+
}
124+
125+
let initial_size = self.obligations.len();
126+
let iter = iter.into_iter();
127+
let expected_new = iter.len();
128+
let combined_size = initial_size + expected_new;
129+
130+
if combined_size <= 16 || combined_size < initial_size.next_power_of_two() {
131+
// small case/not crossing a power of two. don't bother with dedup
132+
self.obligations.extend(iter.map(Cow::into_owned));
133+
} else {
134+
// crossing power of two threshold. this would incur a vec growth anyway if we didn't do
135+
// anything. piggyback a dedup on that
136+
let obligations = std::mem::take(self.obligations);
137+
138+
let mut seen = FxHashMap::default();
139+
seen.reserve(initial_size);
140+
141+
*self.obligations = obligations
142+
.into_iter()
143+
.map(Cow::Owned)
144+
.chain(iter)
145+
.filter_map(|obligation| {
146+
match seen.raw_entry_mut().from_key(obligation.borrow()) {
147+
RawEntryMut::Occupied(..) => {
148+
return None;
149+
}
150+
RawEntryMut::Vacant(vacant) => {
151+
vacant.insert(obligation.clone().into_owned(), ());
152+
}
153+
}
154+
Some(obligation.into_owned())
155+
})
156+
.collect();
157+
}
158+
}
159+
}
160+
98161
impl<'tcx, O> Obligation<'tcx, O> {
99162
pub fn new(
100163
cause: ObligationCause<'tcx>,

‎compiler/rustc_infer/src/traits/project.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,20 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
138138
Ok(())
139139
}
140140

141+
pub fn try_start_borrowed<'a, T>(
142+
&'a mut self,
143+
key: ProjectionCacheKey<'tcx>,
144+
with: impl FnOnce(&'_ ProjectionCacheEntry<'tcx>) -> T + 'a,
145+
) -> Option<T> {
146+
let mut map = self.map();
147+
if let Some(entry) = map.get(&key) {
148+
return Some(with(entry));
149+
}
150+
151+
map.insert(key, ProjectionCacheEntry::InProgress);
152+
None
153+
}
154+
141155
/// Indicates that `key` was normalized to `value`.
142156
pub fn insert_ty(&mut self, key: ProjectionCacheKey<'tcx>, value: NormalizedTy<'tcx>) {
143157
debug!(

‎compiler/rustc_trait_selection/src/traits/project.rs‎

Lines changed: 74 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ use rustc_middle::ty::subst::Subst;
3030
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, WithConstness};
3131
use rustc_span::symbol::sym;
3232

33+
use std::borrow::Cow;
3334
use std::collections::BTreeMap;
3435

36+
use crate::traits::ObligationsDedup;
3537
pub use rustc_middle::traits::Reveal;
3638

3739
pub type PolyProjectionObligation<'tcx> = Obligation<'tcx, ty::PolyProjectionPredicate<'tcx>>;
@@ -839,6 +841,8 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
839841
// mode, which could lead to using incorrect cache results.
840842
let use_cache = !selcx.is_intercrate();
841843

844+
let mut obligations = ObligationsDedup::from_vec(obligations);
845+
842846
let projection_ty = infcx.resolve_vars_if_possible(projection_ty);
843847
let cache_key = ProjectionCacheKey::new(projection_ty);
844848

@@ -850,65 +854,76 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
850854
// bounds. It might be the case that we want two distinct caches,
851855
// or else another kind of cache entry.
852856

853-
let cache_result = if use_cache {
854-
infcx.inner.borrow_mut().projection_cache().try_start(cache_key)
855-
} else {
856-
Ok(())
857-
};
858-
match cache_result {
859-
Ok(()) => debug!("no cache"),
860-
Err(ProjectionCacheEntry::Ambiguous) => {
861-
// If we found ambiguity the last time, that means we will continue
862-
// to do so until some type in the key changes (and we know it
863-
// hasn't, because we just fully resolved it).
864-
debug!("found cache entry: ambiguous");
865-
return Ok(None);
866-
}
867-
Err(ProjectionCacheEntry::InProgress) => {
868-
// Under lazy normalization, this can arise when
869-
// bootstrapping. That is, imagine an environment with a
870-
// where-clause like `A::B == u32`. Now, if we are asked
871-
// to normalize `A::B`, we will want to check the
872-
// where-clauses in scope. So we will try to unify `A::B`
873-
// with `A::B`, which can trigger a recursive
874-
// normalization.
875-
876-
debug!("found cache entry: in-progress");
877-
878-
// Cache that normalizing this projection resulted in a cycle. This
879-
// should ensure that, unless this happens within a snapshot that's
880-
// rolled back, fulfillment or evaluation will notice the cycle.
857+
if use_cache {
858+
let result =
859+
infcx.inner.borrow_mut().projection_cache().try_start_borrowed(cache_key, |cached| {
860+
match cached {
861+
ProjectionCacheEntry::NormalizedTy(ty) => {
862+
// This is the hottest path in this function.
863+
//
864+
// If we find the value in the cache, then return it along
865+
// with the obligations that went along with it. Note
866+
// that, when using a fulfillment context, these
867+
// obligations could in principle be ignored: they have
868+
// already been registered when the cache entry was
869+
// created (and hence the new ones will quickly be
870+
// discarded as duplicated). But when doing trait
871+
// evaluation this is not the case, and dropping the trait
872+
// evaluations can causes ICEs (e.g., #43132).
873+
debug!(?ty, "found normalized ty");
874+
obligations.extend(ty.obligations.iter().map(Cow::Borrowed));
875+
Ok(Some(ty.value))
876+
}
877+
cached @ _ => Err(cached.clone()),
878+
}
879+
});
881880

882-
if use_cache {
883-
infcx.inner.borrow_mut().projection_cache().recur(cache_key);
881+
match result {
882+
Some(Ok(ret)) => return Ok(ret),
883+
Some(Err(cached)) => {
884+
return match cached {
885+
ProjectionCacheEntry::Ambiguous => {
886+
// If we found ambiguity the last time, that means we will continue
887+
// to do so until some type in the key changes (and we know it
888+
// hasn't, because we just fully resolved it).
889+
debug!("found cache entry: ambiguous");
890+
Ok(None)
891+
}
892+
ProjectionCacheEntry::InProgress => {
893+
// Under lazy normalization, this can arise when
894+
// bootstrapping. That is, imagine an environment with a
895+
// where-clause like `A::B == u32`. Now, if we are asked
896+
// to normalize `A::B`, we will want to check the
897+
// where-clauses in scope. So we will try to unify `A::B`
898+
// with `A::B`, which can trigger a recursive
899+
// normalization.
900+
901+
debug!("found cache entry: in-progress");
902+
903+
// Cache that normalizing this projection resulted in a cycle. This
904+
// should ensure that, unless this happens within a snapshot that's
905+
// rolled back, fulfillment or evaluation will notice the cycle.
906+
907+
if use_cache {
908+
infcx.inner.borrow_mut().projection_cache().recur(cache_key);
909+
}
910+
Err(InProgress)
911+
}
912+
ProjectionCacheEntry::Recur => {
913+
debug!("recur cache");
914+
Err(InProgress)
915+
}
916+
ProjectionCacheEntry::Error => {
917+
debug!("opt_normalize_projection_type: found error");
918+
let result =
919+
normalize_to_error(selcx, param_env, projection_ty, cause, depth);
920+
obligations.extend(result.obligations.into_iter().map(Cow::Owned));
921+
Ok(Some(result.value))
922+
}
923+
_ => unreachable!("unexpected variant"),
924+
};
884925
}
885-
return Err(InProgress);
886-
}
887-
Err(ProjectionCacheEntry::Recur) => {
888-
debug!("recur cache");
889-
return Err(InProgress);
890-
}
891-
Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
892-
// This is the hottest path in this function.
893-
//
894-
// If we find the value in the cache, then return it along
895-
// with the obligations that went along with it. Note
896-
// that, when using a fulfillment context, these
897-
// obligations could in principle be ignored: they have
898-
// already been registered when the cache entry was
899-
// created (and hence the new ones will quickly be
900-
// discarded as duplicated). But when doing trait
901-
// evaluation this is not the case, and dropping the trait
902-
// evaluations can causes ICEs (e.g., #43132).
903-
debug!(?ty, "found normalized ty");
904-
obligations.extend(ty.obligations);
905-
return Ok(Some(ty.value));
906-
}
907-
Err(ProjectionCacheEntry::Error) => {
908-
debug!("opt_normalize_projection_type: found error");
909-
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
910-
obligations.extend(result.obligations);
911-
return Ok(Some(result.value));
926+
_ => {}
912927
}
913928
}
914929

@@ -966,7 +981,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
966981
if use_cache {
967982
infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, result.clone());
968983
}
969-
obligations.extend(result.obligations);
984+
obligations.extend(result.obligations.into_iter().map(Cow::Owned));
970985
Ok(Some(result.value))
971986
}
972987
Ok(ProjectedTy::NoProgress(projected_ty)) => {
@@ -996,7 +1011,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
9961011
infcx.inner.borrow_mut().projection_cache().error(cache_key);
9971012
}
9981013
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
999-
obligations.extend(result.obligations);
1014+
obligations.extend(result.obligations.into_iter().map(Cow::Owned));
10001015
Ok(Some(result.value))
10011016
}
10021017
}

‎compiler/rustc_trait_selection/src/traits/select/mod.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,6 +2343,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
23432343
// This code is hot enough that it's worth avoiding the allocation
23442344
// required for the FxHashSet when possible. Special-casing lengths 0,
23452345
// 1 and 2 covers roughly 75-80% of the cases.
2346+
//
2347+
// Ideally we would perform deduplication incrementally in the predicates
2348+
// loop above to prevent excessive Vec growth but that would require
2349+
// a Vec::range_retain or similar method.
23462350
if obligations.len() <= 1 {
23472351
// No possibility of duplicates.
23482352
} else if obligations.len() == 2 {

0 commit comments

Comments
 (0)