Skip to content

Commit

Permalink
Auto merge of rust-lang#133566 - lcnr:fast-reject-perf, r=<try>
Browse files Browse the repository at this point in the history
fast-reject: add cache

slightly modified version of rust-lang#133524

I originally wanted to simply bail after recursion for a certain amount of times, however, looking at the number of steps taken while compiling different crates we get the following results[^1]:

typenum
```rust
1098842 counts
(  1)   670511 (61.0%, 61.0%): dropping after 1
(  2)   358785 (32.7%, 93.7%): dropping after 0
(  3)    25191 ( 2.3%, 96.0%): dropping after 2
(  4)    10912 ( 1.0%, 97.0%): dropping after 4
(  5)     6461 ( 0.6%, 97.5%): dropping after 3
(  6)     5239 ( 0.5%, 98.0%): dropping after 5
(  7)     2528 ( 0.2%, 98.3%): dropping after 8
(  8)     2188 ( 0.2%, 98.5%): dropping after 1094
(  9)     2097 ( 0.2%, 98.6%): dropping after 6
( 10)     1179 ( 0.1%, 98.7%): dropping after 34
( 11)     1148 ( 0.1%, 98.9%): dropping after 7
( 12)      822 ( 0.1%, 98.9%): dropping after 10
```
bitmaps
```rust
533346 counts
(  1)   526166 (98.7%, 98.7%): dropping after 1
(  2)     4562 ( 0.9%, 99.5%): dropping after 0
(  3)     2072 ( 0.4%, 99.9%): dropping after 1024
(  4)      305 ( 0.1%,100.0%): dropping after 2
(  5)      106 ( 0.0%,100.0%): dropping after 4
(  6)       30 ( 0.0%,100.0%): dropping after 8
(  7)       18 ( 0.0%,100.0%): dropping after 3
(  8)       17 ( 0.0%,100.0%): dropping after 44
(  9)       15 ( 0.0%,100.0%): dropping after 168
( 10)        8 ( 0.0%,100.0%): dropping after 14
( 11)        7 ( 0.0%,100.0%): dropping after 13
( 12)        7 ( 0.0%,100.0%): dropping after 24
```
stage 2 compiler is mostly trivial, but has a few cases where we get >5000
```rust
12987156 counts
(  1)  9280476 (71.5%, 71.5%): dropping after 0
(  2)  2277841 (17.5%, 89.0%): dropping after 1
(  3)   724888 ( 5.6%, 94.6%): dropping after 2
(  4)   204005 ( 1.6%, 96.2%): dropping after 4
(  5)   146537 ( 1.1%, 97.3%): dropping after 3
(  6)    64287 ( 0.5%, 97.8%): dropping after 5
(  7)    43938 ( 0.3%, 98.1%): dropping after 6
(  8)    43758 ( 0.3%, 98.4%): dropping after 8
(  9)    27220 ( 0.2%, 98.7%): dropping after 7
( 10)    17374 ( 0.1%, 98.8%): dropping after 9
( 11)    16015 ( 0.1%, 98.9%): dropping after 10
( 12)    12855 ( 0.1%, 99.0%): dropping after 12
( 13)    10494 ( 0.1%, 99.1%): dropping after 11
( 14)     7553 ( 0.1%, 99.2%): dropping after 14
```

Given that we have crates which frequently rely on fairly deep recursion, actually using a cache seems better than using an arbitrary cutoff here. Having an impl which is large enough to trigger a cutoff instead of getting rejected noticeably impacts perf, so just using a cache in these cases seems better to me. Does not matter too much in the end, we only have to make sure we don't regress crates which don't recurse deeply.

[^1]: i've incremented a counter in the place I now call `if cache.get(&(lhs, rhs))` and then printed it on drop
r? `@compiler-errors`
  • Loading branch information
bors committed Nov 29, 2024
2 parents cb2bd2b + f6f4396 commit 613c078
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 16 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub fn overlapping_impls(
// Before doing expensive operations like entering an inference context, do
// a quick check via fast_reject to tell if the impl headers could possibly
// unify.
let drcx = DeepRejectCtxt::relate_infer_infer(tcx);
let mut drcx = DeepRejectCtxt::relate_infer_infer(tcx);
let impl1_ref = tcx.impl_trait_ref(impl1_def_id);
let impl2_ref = tcx.impl_trait_ref(impl2_def_id);
let may_overlap = match (impl1_ref, impl2_ref) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn evaluate_host_effect_from_bounds<'tcx>(
obligation: &HostEffectObligation<'tcx>,
) -> Result<ThinVec<PredicateObligation<'tcx>>, EvaluationFailure> {
let infcx = selcx.infcx;
let drcx = DeepRejectCtxt::relate_rigid_rigid(selcx.tcx());
let mut drcx = DeepRejectCtxt::relate_rigid_rigid(selcx.tcx());
let mut candidate = None;

for predicate in obligation.param_env.caller_bounds() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ fn assemble_candidates_from_predicates<'cx, 'tcx>(
potentially_unnormalized_candidates: bool,
) {
let infcx = selcx.infcx;
let drcx = DeepRejectCtxt::relate_rigid_rigid(selcx.tcx());
let mut drcx = DeepRejectCtxt::relate_rigid_rigid(selcx.tcx());
for predicate in env_predicates {
let bound_predicate = predicate.kind();
if let ty::ClauseKind::Projection(data) = predicate.kind().skip_binder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.filter(|p| p.def_id() == stack.obligation.predicate.def_id())
.filter(|p| p.polarity() == stack.obligation.predicate.polarity());

let drcx = DeepRejectCtxt::relate_rigid_rigid(self.tcx());
let mut drcx = DeepRejectCtxt::relate_rigid_rigid(self.tcx());
let obligation_args = stack.obligation.predicate.skip_binder().trait_ref.args;
// Keep only those bounds which may apply, and propagate overflow if it occurs.
for bound in bounds {
Expand Down Expand Up @@ -548,7 +548,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation: &PolyTraitObligation<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>,
) {
let drcx = DeepRejectCtxt::relate_rigid_infer(self.tcx());
let mut drcx = DeepRejectCtxt::relate_rigid_infer(self.tcx());
let obligation_args = obligation.predicate.skip_binder().trait_ref.args;
self.tcx().for_each_relevant_impl(
obligation.predicate.def_id(),
Expand Down
32 changes: 23 additions & 9 deletions compiler/rustc_type_ir/src/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHas
#[cfg(feature = "nightly")]
use rustc_macros::{HashStable_NoContext, TyDecodable, TyEncodable};

use crate::data_structures::DelayedSet;
use crate::inherent::*;
use crate::visit::TypeVisitableExt as _;
use crate::{self as ty, Interner};
Expand Down Expand Up @@ -181,41 +182,44 @@ impl<DefId> SimplifiedType<DefId> {
/// We also use this function during coherence. For coherence the
/// impls only have to overlap for some value, so we treat parameters
/// on both sides like inference variables.
#[derive(Debug, Clone, Copy)]
#[derive(Debug)]
pub struct DeepRejectCtxt<
I: Interner,
const INSTANTIATE_LHS_WITH_INFER: bool,
const INSTANTIATE_RHS_WITH_INFER: bool,
> {
_interner: PhantomData<I>,
/// We use a cache here as exponentially large - but self-similar - types otherwise
/// cause hangs, e.g. when compiling itertools with the `-Znext-solver`.
cache: DelayedSet<(I::Ty, I::Ty)>,
}

impl<I: Interner> DeepRejectCtxt<I, false, false> {
/// Treat parameters in both the lhs and the rhs as rigid.
pub fn relate_rigid_rigid(_interner: I) -> DeepRejectCtxt<I, false, false> {
DeepRejectCtxt { _interner: PhantomData }
DeepRejectCtxt { _interner: PhantomData, cache: Default::default() }
}
}

impl<I: Interner> DeepRejectCtxt<I, true, true> {
/// Treat parameters in both the lhs and the rhs as infer vars.
pub fn relate_infer_infer(_interner: I) -> DeepRejectCtxt<I, true, true> {
DeepRejectCtxt { _interner: PhantomData }
DeepRejectCtxt { _interner: PhantomData, cache: Default::default() }
}
}

impl<I: Interner> DeepRejectCtxt<I, false, true> {
/// Treat parameters in the lhs as rigid, and in rhs as infer vars.
pub fn relate_rigid_infer(_interner: I) -> DeepRejectCtxt<I, false, true> {
DeepRejectCtxt { _interner: PhantomData }
DeepRejectCtxt { _interner: PhantomData, cache: Default::default() }
}
}

impl<I: Interner, const INSTANTIATE_LHS_WITH_INFER: bool, const INSTANTIATE_RHS_WITH_INFER: bool>
DeepRejectCtxt<I, INSTANTIATE_LHS_WITH_INFER, INSTANTIATE_RHS_WITH_INFER>
{
pub fn args_may_unify(
self,
&mut self,
obligation_args: I::GenericArgs,
impl_args: I::GenericArgs,
) -> bool {
Expand All @@ -234,7 +238,7 @@ impl<I: Interner, const INSTANTIATE_LHS_WITH_INFER: bool, const INSTANTIATE_RHS_
})
}

pub fn types_may_unify(self, lhs: I::Ty, rhs: I::Ty) -> bool {
pub fn types_may_unify(&mut self, lhs: I::Ty, rhs: I::Ty) -> bool {
match rhs.kind() {
// Start by checking whether the `rhs` type may unify with
// pretty much everything. Just return `true` in that case.
Expand Down Expand Up @@ -273,8 +277,12 @@ impl<I: Interner, const INSTANTIATE_LHS_WITH_INFER: bool, const INSTANTIATE_RHS_
| ty::Placeholder(_) => {}
};

if self.cache.contains(&(lhs, rhs)) {
return true;
}

// For purely rigid types, use structural equivalence.
match lhs.kind() {
let result = match lhs.kind() {
ty::Ref(_, lhs_ty, lhs_mutbl) => match rhs.kind() {
ty::Ref(_, rhs_ty, rhs_mutbl) => {
lhs_mutbl == rhs_mutbl && self.types_may_unify(lhs_ty, rhs_ty)
Expand Down Expand Up @@ -414,10 +422,16 @@ impl<I: Interner, const INSTANTIATE_LHS_WITH_INFER: bool, const INSTANTIATE_RHS_
}

ty::Error(..) => true,
};

if result {
self.cache.insert((lhs, rhs));
}

result
}

pub fn consts_may_unify(self, lhs: I::Const, rhs: I::Const) -> bool {
pub fn consts_may_unify(&mut self, lhs: I::Const, rhs: I::Const) -> bool {
match rhs.kind() {
ty::ConstKind::Param(_) => {
if INSTANTIATE_RHS_WITH_INFER {
Expand Down Expand Up @@ -465,7 +479,7 @@ impl<I: Interner, const INSTANTIATE_LHS_WITH_INFER: bool, const INSTANTIATE_RHS_
}
}

fn var_and_ty_may_unify(self, var: ty::InferTy, ty: I::Ty) -> bool {
fn var_and_ty_may_unify(&mut self, var: ty::InferTy, ty: I::Ty) -> bool {
if !ty.is_known_rigid() {
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/html/render/write_shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,8 +918,8 @@ impl<'item> DocVisitor<'item> for TypeImplCollector<'_, '_, 'item> {
// Be aware of `tests/rustdoc/type-alias/deeply-nested-112515.rs` which might regress.
let Some(impl_did) = impl_item_id.as_def_id() else { continue };
let for_ty = self.cx.tcx().type_of(impl_did).skip_binder();
let reject_cx = DeepRejectCtxt::relate_infer_infer(self.cx.tcx());
if !reject_cx.types_may_unify(aliased_ty, for_ty) {
let mut drcx = DeepRejectCtxt::relate_infer_infer(self.cx.tcx());
if !drcx.types_may_unify(aliased_ty, for_ty) {
continue;
}
// Avoid duplicates
Expand Down

0 comments on commit 613c078

Please sign in to comment.