Skip to content

Commit

Permalink
Auto merge of #67899 - Marwes:view, r=<try>
Browse files Browse the repository at this point in the history
perf: Avoid re-interning types in outlives checking

In profiling `intern_ty` is a very hot function (9% in the test I used). While there does not seem to be a way to reduce the cost of calling we can avoid the call in some cases.

In outlives checking `ParamTy` and `ProjectionTy` are extracted from the `Ty` value that contains them only to later be passed as an argument to `intern_ty` again later. This seems to be happening a lot in my test with `intern_ty` called from outlives is at ~6%.

Since all `ParamTy` and `ProjectionTy` are already stored in a `Ty` I had an idea to pass around a `View` type which provides direct access to the specific, inner type without losing the original `Ty` pointer. While the current implementation does so with some unsafe to let the branch be elided on `Deref`, it could be done entirely in safe code as well, either by accepting the (predictable) branch in `Deref` or by storing the inner type in `View` as well as the `Ty`. But considering that the unsafe is trivial to prove and the call sites seem quite hot I opted to show the unsafe approach first.

Based on #67840 (since it touches the same file/lines)

Commits without #67840 https://github.com/rust-lang/rust/pull/67899/files/77ddc3540e52be4b5bd75cf082c621392acaf81b..b55bab206096c27533120921f6b0c273f115e34a
  • Loading branch information
bors committed Jan 5, 2020
2 parents 7785834 + b55bab2 commit 4218803
Show file tree
Hide file tree
Showing 15 changed files with 472 additions and 219 deletions.
2 changes: 1 addition & 1 deletion src/librustc/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
continue;
}

let verify_kind_ty = verify.kind.to_ty(self.tcx());
let verify_kind_ty = verify.kind.as_ty();
if self.bound_is_met(&verify.bound, var_data, verify_kind_ty, sub) {
continue;
}
Expand Down
49 changes: 31 additions & 18 deletions src/librustc/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ use crate::hir;
use crate::infer::outlives::env::RegionBoundPairs;
use crate::infer::outlives::verify::VerifyBoundCx;
use crate::infer::{self, GenericKind, InferCtxt, RegionObligation, SubregionOrigin, VerifyBound};
use crate::traits;
use crate::traits::ObligationCause;
use crate::ty::outlives::Component;
use crate::ty::subst::GenericArgKind;
Expand Down Expand Up @@ -154,6 +155,8 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {

let my_region_obligations = self.take_registered_region_obligations();

let mut elaborator = traits::Elaborator::new(self.tcx);

for (body_id, RegionObligation { sup_type, sub_region, origin }) in my_region_obligations {
debug!(
"process_registered_region_obligations: sup_type={:?} sub_region={:?} origin={:?}",
Expand All @@ -169,6 +172,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
&region_bound_pairs,
implicit_region_bound,
param_env,
&mut elaborator,
);
outlives.type_must_outlive(origin, sup_type, sub_region);
} else {
Expand All @@ -191,15 +195,16 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
ty: Ty<'tcx>,
region: ty::Region<'tcx>,
) {
let outlives = &mut TypeOutlives::new(
let ty = self.resolve_vars_if_possible(&ty);
TypeOutlives::new(
self,
self.tcx,
region_bound_pairs,
implicit_region_bound,
param_env,
);
let ty = self.resolve_vars_if_possible(&ty);
outlives.type_must_outlive(origin, ty, region);
&mut traits::Elaborator::new(self.tcx),
)
.type_must_outlive(origin, ty, region);
}
}

Expand All @@ -209,15 +214,15 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
/// via a "delegate" of type `D` -- this is usually the `infcx`, which
/// accrues them into the `region_obligations` code, but for NLL we
/// use something else.
pub struct TypeOutlives<'cx, 'tcx, D>
pub struct TypeOutlives<'cx, 'tcx, 'e, D>
where
D: TypeOutlivesDelegate<'tcx>,
{
// See the comments on `process_registered_region_obligations` for the meaning
// of these fields.
delegate: D,
tcx: TyCtxt<'tcx>,
verify_bound: VerifyBoundCx<'cx, 'tcx>,
verify_bound: VerifyBoundCx<'cx, 'tcx, 'e>,
}

pub trait TypeOutlivesDelegate<'tcx> {
Expand All @@ -237,7 +242,7 @@ pub trait TypeOutlivesDelegate<'tcx> {
);
}

impl<'cx, 'tcx, D> TypeOutlives<'cx, 'tcx, D>
impl<'cx, 'tcx, 'e, D> TypeOutlives<'cx, 'tcx, 'e, D>
where
D: TypeOutlivesDelegate<'tcx>,
{
Expand All @@ -247,6 +252,7 @@ where
region_bound_pairs: &'cx RegionBoundPairs<'tcx>,
implicit_region_bound: Option<ty::Region<'tcx>>,
param_env: ty::ParamEnv<'tcx>,
elaborator: &'e mut traits::Elaborator<'tcx>,
) -> Self {
Self {
delegate,
Expand All @@ -256,6 +262,7 @@ where
region_bound_pairs,
implicit_region_bound,
param_env,
elaborator,
),
}
}
Expand All @@ -273,7 +280,9 @@ where
origin: infer::SubregionOrigin<'tcx>,
ty: Ty<'tcx>,
region: ty::Region<'tcx>,
) {
) where
'tcx: 'e,
{
debug!("type_must_outlive(ty={:?}, region={:?}, origin={:?})", ty, region, origin);

assert!(!ty.has_escaping_bound_vars());
Expand All @@ -288,8 +297,10 @@ where
origin: infer::SubregionOrigin<'tcx>,
components: &[Component<'tcx>],
region: ty::Region<'tcx>,
) {
for component in components.iter() {
) where
'tcx: 'e,
{
for component in components {
let origin = origin.clone();
match component {
Component::Region(region1) => {
Expand Down Expand Up @@ -321,7 +332,7 @@ where
&mut self,
origin: infer::SubregionOrigin<'tcx>,
region: ty::Region<'tcx>,
param_ty: ty::ParamTy,
param_ty: ty::View<'tcx, ty::ParamTy>,
) {
debug!(
"param_ty_must_outlive(region={:?}, param_ty={:?}, origin={:?})",
Expand All @@ -337,8 +348,10 @@ where
&mut self,
origin: infer::SubregionOrigin<'tcx>,
region: ty::Region<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
) {
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) where
'tcx: 'e,
{
debug!(
"projection_must_outlive(region={:?}, projection_ty={:?}, origin={:?})",
region, projection_ty, origin
Expand Down Expand Up @@ -367,17 +380,17 @@ where
// Compute the bounds we can derive from the environment. This
// is an "approximate" match -- in some cases, these bounds
// may not apply.
let mut approx_env_bounds =
self.verify_bound.projection_approx_declared_bounds_from_env(projection_ty);
let mut approx_env_bounds: Vec<_> =
self.verify_bound.projection_approx_declared_bounds_from_env(projection_ty).collect();
debug!("projection_must_outlive: approx_env_bounds={:?}", approx_env_bounds);

// Remove outlives bounds that we get from the environment but
// which are also deducable from the trait. This arises (cc
// #55756) in cases where you have e.g., `<T as Foo<'a>>::Item:
// 'a` in the environment but `trait Foo<'b> { type Item: 'b
// }` in the trait definition.
approx_env_bounds.retain(|bound| match bound.0.kind {
ty::Projection(projection_ty) => self
approx_env_bounds.retain(|bound| match bound.0.into() {
ty::view::Projection(projection_ty) => self
.verify_bound
.projection_declared_bounds_from_trait(projection_ty)
.all(|r| r != bound.1),
Expand Down Expand Up @@ -454,7 +467,7 @@ where
}
}

impl<'cx, 'tcx> TypeOutlivesDelegate<'tcx> for &'cx InferCtxt<'cx, 'tcx> {
impl<'cx, 'tcx> TypeOutlivesDelegate<'tcx> for &'_ InferCtxt<'cx, 'tcx> {
fn push_sub_region_constraint(
&mut self,
origin: SubregionOrigin<'tcx>,
Expand Down
Loading

0 comments on commit 4218803

Please sign in to comment.