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 10, 2020
2 parents 2d8d559 + c243723 commit 51adcde
Show file tree
Hide file tree
Showing 11 changed files with 309 additions and 126 deletions.
2 changes: 1 addition & 1 deletion src/librustc/infer/lexical_region_resolve/mod.rs
Expand Up @@ -602,7 +602,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
8 changes: 4 additions & 4 deletions src/librustc/infer/outlives/obligations.rs
Expand Up @@ -321,7 +321,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,7 +337,7 @@ where
&mut self,
origin: infer::SubregionOrigin<'tcx>,
region: ty::Region<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) {
debug!(
"projection_must_outlive(region={:?}, projection_ty={:?}, origin={:?})",
Expand Down Expand Up @@ -376,8 +376,8 @@ where
// #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
40 changes: 19 additions & 21 deletions src/librustc/infer/outlives/verify.rs
Expand Up @@ -2,7 +2,7 @@ use crate::infer::outlives::env::RegionBoundPairs;
use crate::infer::{GenericKind, VerifyBound};
use crate::traits;
use crate::ty::subst::{InternalSubsts, Subst};
use crate::ty::{self, Ty, TyCtxt};
use crate::ty::{self, Ty, TyCtxt, View};
use rustc_data_structures::captures::Captures;
use rustc_hir::def_id::DefId;

Expand Down Expand Up @@ -39,22 +39,20 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
}

fn type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> {
match ty.kind {
ty::Param(p) => self.param_bound(p),
ty::Projection(data) => self.projection_bound(data),
match ty.into() {
ty::view::Param(p) => self.param_bound(p),
ty::view::Projection(data) => self.projection_bound(data),
_ => self.recursive_type_bound(ty),
}
}

fn param_bound(&self, param_ty: ty::ParamTy) -> VerifyBound<'tcx> {
fn param_bound(&self, param_ty: View<'tcx, ty::ParamTy>) -> VerifyBound<'tcx> {
debug!("param_bound(param_ty={:?})", param_ty);

// Start with anything like `T: 'a` we can scrape from the
// environment
let param_bounds = self
.declared_generic_bounds_from_env(GenericKind::Param(param_ty))
.into_iter()
.map(|outlives| outlives.1);
let param_bounds =
self.declared_generic_bounds_from_env(param_ty).into_iter().map(|outlives| outlives.1);

// Add in the default bound of fn body that applies to all in
// scope type parameters:
Expand All @@ -78,9 +76,9 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
/// this list.
pub fn projection_approx_declared_bounds_from_env(
&self,
projection_ty: ty::ProjectionTy<'tcx>,
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) -> Vec<ty::OutlivesPredicate<Ty<'tcx>, ty::Region<'tcx>>> {
let projection_ty = GenericKind::Projection(projection_ty).to_ty(self.tcx);
let projection_ty = projection_ty.as_ty();
let erased_projection_ty = self.tcx.erase_regions(&projection_ty);
self.declared_generic_bounds_from_env_with_compare_fn(|ty| {
if let ty::Projection(..) = ty.kind {
Expand All @@ -97,16 +95,18 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
/// exact match.
pub fn projection_declared_bounds_from_trait(
&self,
projection_ty: ty::ProjectionTy<'tcx>,
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) -> impl Iterator<Item = ty::Region<'tcx>> + 'cx + Captures<'tcx> {
self.declared_projection_bounds_from_trait(projection_ty)
}

pub fn projection_bound(&self, projection_ty: ty::ProjectionTy<'tcx>) -> VerifyBound<'tcx> {
pub fn projection_bound(
&self,
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) -> VerifyBound<'tcx> {
debug!("projection_bound(projection_ty={:?})", projection_ty);

let projection_ty_as_ty =
self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs);
let projection_ty_as_ty = projection_ty.as_ty();

// Search the env for where clauses like `P: 'a`.
let env_bounds = self
Expand Down Expand Up @@ -161,18 +161,16 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
/// bounds, but all the bounds it returns can be relied upon.
fn declared_generic_bounds_from_env(
&self,
generic: GenericKind<'tcx>,
generic: View<'tcx, ty::ParamTy>,
) -> Vec<ty::OutlivesPredicate<Ty<'tcx>, ty::Region<'tcx>>> {
let generic_ty = generic.to_ty(self.tcx);
let generic_ty = generic.as_ty();
self.declared_generic_bounds_from_env_with_compare_fn(|ty| ty == generic_ty)
}

fn declared_generic_bounds_from_env_with_compare_fn(
&self,
compare_ty: impl Fn(Ty<'tcx>) -> bool,
) -> Vec<ty::OutlivesPredicate<Ty<'tcx>, ty::Region<'tcx>>> {
let tcx = self.tcx;

// To start, collect bounds from user environment. Note that
// parameter environments are already elaborated, so we don't
// have to worry about that. Comparing using `==` is a bit
Expand All @@ -198,7 +196,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
"declared_generic_bounds_from_env_with_compare_fn: region_bound_pair = {:?}",
(r, p)
);
let p_ty = p.to_ty(tcx);
let p_ty = p.as_ty();
compare_ty(p_ty).then_some(ty::OutlivesPredicate(p_ty, r))
});

Expand Down Expand Up @@ -227,7 +225,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
/// `region_bounds_declared_on_associated_item`.
fn declared_projection_bounds_from_trait(
&self,
projection_ty: ty::ProjectionTy<'tcx>,
projection_ty: ty::View<'tcx, ty::ProjectionTy<'tcx>>,
) -> impl Iterator<Item = ty::Region<'tcx>> + 'cx + Captures<'tcx> {
debug!("projection_bounds(projection_ty={:?})", projection_ty);
let tcx = self.tcx;
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/infer/region_constraints/mod.rs
Expand Up @@ -184,8 +184,8 @@ pub struct Verify<'tcx> {

#[derive(Copy, Clone, PartialEq, Eq, Hash, TypeFoldable)]
pub enum GenericKind<'tcx> {
Param(ty::ParamTy),
Projection(ty::ProjectionTy<'tcx>),
Param(ty::View<'tcx, ty::ParamTy>),
Projection(ty::View<'tcx, ty::ProjectionTy<'tcx>>),
}

/// Describes the things that some `GenericKind` value `G` is known to
Expand Down Expand Up @@ -875,10 +875,10 @@ impl<'tcx> fmt::Display for GenericKind<'tcx> {
}

impl<'tcx> GenericKind<'tcx> {
pub fn to_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
pub fn as_ty(&self) -> Ty<'tcx> {
match *self {
GenericKind::Param(ref p) => p.to_ty(tcx),
GenericKind::Projection(ref p) => tcx.mk_projection(p.item_def_id, p.substs),
GenericKind::Param(ref p) => p.as_ty(),
GenericKind::Projection(ref p) => p.as_ty(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/query/outlives_bounds.rs
Expand Up @@ -20,8 +20,8 @@ use std::mem;
#[derive(Clone, Debug, TypeFoldable, Lift)]
pub enum OutlivesBound<'tcx> {
RegionSubRegion(ty::Region<'tcx>, ty::Region<'tcx>),
RegionSubParam(ty::Region<'tcx>, ty::ParamTy),
RegionSubProjection(ty::Region<'tcx>, ty::ProjectionTy<'tcx>),
RegionSubParam(ty::Region<'tcx>, ty::View<'tcx, ty::ParamTy>),
RegionSubProjection(ty::Region<'tcx>, ty::View<'tcx, ty::ProjectionTy<'tcx>>),
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for OutlivesBound<'tcx> {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ty/mod.rs
Expand Up @@ -91,6 +91,8 @@ pub use self::trait_def::TraitDef;

pub use self::query::queries;

pub use self::view::View;

pub mod adjustment;
pub mod binding;
pub mod cast;
Expand All @@ -114,6 +116,7 @@ pub mod steal;
pub mod subst;
pub mod trait_def;
pub mod util;
pub mod view;
pub mod walk;

mod context;
Expand Down

0 comments on commit 51adcde

Please sign in to comment.