Skip to content

Commit d23e1a6

Browse files
committed
Auto merge of #117749 - aliemjay:perf-canon-cache, r=lcnr
cache param env canonicalization Canonicalize ParamEnv only once and store it. Then whenever we try to canonicalize `ParamEnvAnd<'tcx, T>` we only have to canonicalize `T` and then merge the results. Prelimiary results show ~3-4% savings in diesel and serde benchmarks. Best to review commits individually. Some commits have a short description. Initial implementation had a soundness bug (#117749 (comment)) due to cache invalidation: - When canonicalizing `Ty<'?0>` we first try to resolve region variables in the current InferCtxt which may have a constraint `?0 == 'static`. This means that we register `Ty<'?0> => Canonical<Ty<'static>>` in the cache, which is obviously incorrect in another inference context. - This is fixed by not doing region resolution when canonicalizing the query *input* (vs. response), which is the only place where ParamEnv is used, and then in a later commit we *statically* guard against any form of inference variable resolution of the cached canonical ParamEnv's. r? `@ghost`
2 parents e6d1b0e + aa36c35 commit d23e1a6

File tree

8 files changed

+199
-125
lines changed

8 files changed

+199
-125
lines changed

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

+122-114
Large diffs are not rendered by default.

compiler/rustc_infer/src/infer/combine.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ impl<'tcx> InferCtxt<'tcx> {
172172
// two const param's types are able to be equal has to go through a canonical query with the actual logic
173173
// in `rustc_trait_selection`.
174174
let canonical = self.canonicalize_query(
175-
(relation.param_env(), a.ty(), b.ty()),
175+
relation.param_env().and((a.ty(), b.ty())),
176176
&mut OriginalQueryValues::default(),
177177
);
178178
self.tcx.check_tys_might_be_eq(canonical).map_err(|_| {

compiler/rustc_middle/src/infer/canonical.rs

+63-1
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,20 @@
2121
//!
2222
//! [c]: https://rust-lang.github.io/chalk/book/canonical_queries/canonicalization.html
2323
24+
use rustc_data_structures::fx::FxHashMap;
25+
use rustc_data_structures::sync::Lock;
2426
use rustc_macros::HashStable;
2527
use rustc_type_ir::Canonical as IrCanonical;
2628
use rustc_type_ir::CanonicalVarInfo as IrCanonicalVarInfo;
2729
pub use rustc_type_ir::{CanonicalTyVarKind, CanonicalVarKind};
2830
use smallvec::SmallVec;
31+
use std::collections::hash_map::Entry;
2932
use std::ops::Index;
3033

3134
use crate::infer::MemberConstraint;
3235
use crate::mir::ConstraintCategory;
3336
use crate::ty::GenericArg;
34-
use crate::ty::{self, BoundVar, List, Region, Ty, TyCtxt};
37+
use crate::ty::{self, BoundVar, List, Region, Ty, TyCtxt, TypeFlags, TypeVisitableExt};
3538

3639
pub type Canonical<'tcx, V> = IrCanonical<TyCtxt<'tcx>, V>;
3740

@@ -291,3 +294,62 @@ impl<'tcx> Index<BoundVar> for CanonicalVarValues<'tcx> {
291294
&self.var_values[value.as_usize()]
292295
}
293296
}
297+
298+
#[derive(Default)]
299+
pub struct CanonicalParamEnvCache<'tcx> {
300+
map: Lock<
301+
FxHashMap<
302+
ty::ParamEnv<'tcx>,
303+
(Canonical<'tcx, ty::ParamEnv<'tcx>>, &'tcx [GenericArg<'tcx>]),
304+
>,
305+
>,
306+
}
307+
308+
impl<'tcx> CanonicalParamEnvCache<'tcx> {
309+
/// Gets the cached canonical form of `key` or executes
310+
/// `canonicalize_op` and caches the result if not present.
311+
///
312+
/// `canonicalize_op` is intentionally not allowed to be a closure to
313+
/// statically prevent it from capturing `InferCtxt` and resolving
314+
/// inference variables, which invalidates the cache.
315+
pub fn get_or_insert(
316+
&self,
317+
tcx: TyCtxt<'tcx>,
318+
key: ty::ParamEnv<'tcx>,
319+
state: &mut OriginalQueryValues<'tcx>,
320+
canonicalize_op: fn(
321+
TyCtxt<'tcx>,
322+
ty::ParamEnv<'tcx>,
323+
&mut OriginalQueryValues<'tcx>,
324+
) -> Canonical<'tcx, ty::ParamEnv<'tcx>>,
325+
) -> Canonical<'tcx, ty::ParamEnv<'tcx>> {
326+
if !key.has_type_flags(
327+
TypeFlags::HAS_INFER | TypeFlags::HAS_PLACEHOLDER | TypeFlags::HAS_FREE_REGIONS,
328+
) {
329+
return Canonical {
330+
max_universe: ty::UniverseIndex::ROOT,
331+
variables: List::empty(),
332+
value: key,
333+
};
334+
}
335+
336+
assert_eq!(state.var_values.len(), 0);
337+
assert_eq!(state.universe_map.len(), 1);
338+
debug_assert_eq!(&*state.universe_map, &[ty::UniverseIndex::ROOT]);
339+
340+
match self.map.borrow().entry(key) {
341+
Entry::Occupied(e) => {
342+
let (canonical, var_values) = e.get();
343+
state.var_values.extend_from_slice(var_values);
344+
canonical.clone()
345+
}
346+
Entry::Vacant(e) => {
347+
let canonical = canonicalize_op(tcx, key, state);
348+
let OriginalQueryValues { var_values, universe_map } = state;
349+
assert_eq!(universe_map.len(), 1);
350+
e.insert((canonical.clone(), tcx.arena.alloc_slice(var_values)));
351+
canonical
352+
}
353+
}
354+
}
355+
}

compiler/rustc_middle/src/query/mod.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -2177,7 +2177,9 @@ rustc_queries! {
21772177
/// Used in `super_combine_consts` to ICE if the type of the two consts are definitely not going to end up being
21782178
/// equal to eachother. This might return `Ok` even if the types are not equal, but will never return `Err` if
21792179
/// the types might be equal.
2180-
query check_tys_might_be_eq(arg: Canonical<'tcx, (ty::ParamEnv<'tcx>, Ty<'tcx>, Ty<'tcx>)>) -> Result<(), NoSolution> {
2180+
query check_tys_might_be_eq(
2181+
arg: Canonical<'tcx, ty::ParamEnvAnd<'tcx, (Ty<'tcx>, Ty<'tcx>)>>
2182+
) -> Result<(), NoSolution> {
21812183
desc { "check whether two const param are definitely not equal to eachother"}
21822184
}
21832185

compiler/rustc_middle/src/ty/context.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub mod tls;
66

77
use crate::arena::Arena;
88
use crate::dep_graph::{DepGraph, DepKindStruct};
9-
use crate::infer::canonical::{CanonicalVarInfo, CanonicalVarInfos};
9+
use crate::infer::canonical::{CanonicalParamEnvCache, CanonicalVarInfo, CanonicalVarInfos};
1010
use crate::lint::struct_lint_level;
1111
use crate::metadata::ModChild;
1212
use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
@@ -653,6 +653,8 @@ pub struct GlobalCtxt<'tcx> {
653653
pub new_solver_evaluation_cache: solve::EvaluationCache<'tcx>,
654654
pub new_solver_coherence_evaluation_cache: solve::EvaluationCache<'tcx>,
655655

656+
pub canonical_param_env_cache: CanonicalParamEnvCache<'tcx>,
657+
656658
/// Data layout specification for the current target.
657659
pub data_layout: TargetDataLayout,
658660

@@ -817,6 +819,7 @@ impl<'tcx> TyCtxt<'tcx> {
817819
evaluation_cache: Default::default(),
818820
new_solver_evaluation_cache: Default::default(),
819821
new_solver_coherence_evaluation_cache: Default::default(),
822+
canonical_param_env_cache: Default::default(),
820823
data_layout,
821824
alloc_map: Lock::new(interpret::AllocMap::new()),
822825
}

compiler/rustc_trait_selection/src/traits/misc.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use rustc_infer::infer::canonical::Canonical;
99
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
1010
use rustc_infer::traits::query::NoSolution;
1111
use rustc_infer::{infer::outlives::env::OutlivesEnvironment, traits::FulfillmentError};
12-
use rustc_middle::ty::{self, AdtDef, GenericArg, List, ParamEnv, Ty, TyCtxt, TypeVisitableExt};
12+
use rustc_middle::ty::{self, AdtDef, GenericArg, List, Ty, TyCtxt, TypeVisitableExt};
1313
use rustc_span::DUMMY_SP;
1414

1515
use super::outlives_bounds::InferCtxtExt;
@@ -209,10 +209,10 @@ pub fn all_fields_implement_trait<'tcx>(
209209

210210
pub fn check_tys_might_be_eq<'tcx>(
211211
tcx: TyCtxt<'tcx>,
212-
canonical: Canonical<'tcx, (ParamEnv<'tcx>, Ty<'tcx>, Ty<'tcx>)>,
212+
canonical: Canonical<'tcx, ty::ParamEnvAnd<'tcx, (Ty<'tcx>, Ty<'tcx>)>>,
213213
) -> Result<(), NoSolution> {
214-
let (infcx, (param_env, ty_a, ty_b), _) =
215-
tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical);
214+
let (infcx, key, _) = tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical);
215+
let (param_env, (ty_a, ty_b)) = key.into_parts();
216216
let ocx = ObligationCtxt::new(&infcx);
217217

218218
let result = ocx.eq(&ObligationCause::dummy(), param_env, ty_a, ty_b);

src/librustdoc/clean/blanket_impl.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ pub(crate) struct BlanketImplFinder<'a, 'tcx> {
1414
impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
1515
pub(crate) fn get_blanket_impls(&mut self, item_def_id: DefId) -> Vec<Item> {
1616
let cx = &mut self.cx;
17-
let param_env = cx.tcx.param_env(item_def_id);
1817
let ty = cx.tcx.type_of(item_def_id);
1918

2019
trace!("get_blanket_impls({ty:?})");
@@ -40,7 +39,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
4039
let infcx = cx.tcx.infer_ctxt().build();
4140
let args = infcx.fresh_args_for_item(DUMMY_SP, item_def_id);
4241
let impl_ty = ty.instantiate(infcx.tcx, args);
43-
let param_env = EarlyBinder::bind(param_env).instantiate(infcx.tcx, args);
42+
let param_env = ty::ParamEnv::empty();
4443

4544
let impl_args = infcx.fresh_args_for_item(DUMMY_SP, impl_def_id);
4645
let impl_trait_ref = trait_ref.instantiate(infcx.tcx, impl_args);

src/librustdoc/clean/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use rustc_middle::middle::resolve_bound_vars as rbv;
2626
use rustc_middle::ty::fold::TypeFolder;
2727
use rustc_middle::ty::GenericArgsRef;
2828
use rustc_middle::ty::TypeVisitableExt;
29-
use rustc_middle::ty::{self, AdtKind, EarlyBinder, Ty, TyCtxt};
29+
use rustc_middle::ty::{self, AdtKind, Ty, TyCtxt};
3030
use rustc_middle::{bug, span_bug};
3131
use rustc_span::hygiene::{AstPass, MacroKind};
3232
use rustc_span::symbol::{kw, sym, Ident, Symbol};

0 commit comments

Comments
 (0)