OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Only visit types once when walking the type tree

Browse Source 
This fixes #72408.

Nested closures were resulting in exponential compilation time.

As a performance optimization this change introduces MiniSet,
which is a simple small storage optimized set.
This commit is contained in:
Valerii Lashmanov 2020-09-15 09:37:19 -05:00
parent 255ceeb5ff
commit 2f3296192b
10 changed files with 193 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock
View File

@ -3733,6 +3733,7 @@ dependencies = [
name = "rustc_middle"
version = "0.0.0"
dependencies = [
"arrayvec",
"bitflags",
"chalk-ir",
"measureme",

40
compiler/rustc_infer/src/infer/outlives/verify.rs
View File

@ -3,6 +3,7 @@ use crate::infer::{GenericKind, VerifyBound};
use rustc_data_structures::captures::Captures;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst};
use rustc_middle::ty::walk::MiniSet;
use rustc_middle::ty::{self, Ty, TyCtxt};
/// The `TypeOutlives` struct has the job of "lowering" a `T: 'a`
@ -31,16 +32,23 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
/// Returns a "verify bound" that encodes what we know about
/// `generic` and the regions it outlives.
pub fn generic_bound(&self, generic: GenericKind<'tcx>) -> VerifyBound<'tcx> {
let mut visited = MiniSet::new();
match generic {
GenericKind::Param(param_ty) => self.param_bound(param_ty),
GenericKind::Projection(projection_ty) => self.projection_bound(projection_ty),
GenericKind::Projection(projection_ty) => {
self.projection_bound(projection_ty, &mut visited)
}
}
}
fn type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> {
fn type_bound(
&self,
ty: Ty<'tcx>,
visited: &mut MiniSet<GenericArg<'tcx>>,
) -> VerifyBound<'tcx> {
match *ty.kind() {
ty::Param(p) => self.param_bound(p),
ty::Projection(data) => self.projection_bound(data),
ty::Projection(data) => self.projection_bound(data, visited),
ty::FnDef(_, substs) => {
// HACK(eddyb) ignore lifetimes found shallowly in `substs`.
// This is inconsistent with `ty::Adt` (including all substs),
@ -50,9 +58,9 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
let mut bounds = substs
.iter()
.filter_map(|child| match child.unpack() {
GenericArgKind::Type(ty) => Some(self.type_bound(ty)),
GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)),
GenericArgKind::Lifetime(_) => None,
GenericArgKind::Const(_) => Some(self.recursive_bound(child)),
GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)),
})
.filter(|bound| {
// Remove bounds that must hold, since they are not interesting.
@ -66,7 +74,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
),
}
}
_ => self.recursive_bound(ty.into()),
_ => self.recursive_bound(ty.into(), visited),
}
}
@ -137,7 +145,11 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, '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::ProjectionTy<'tcx>,
visited: &mut MiniSet<GenericArg<'tcx>>,
) -> VerifyBound<'tcx> {
debug!("projection_bound(projection_ty={:?})", projection_ty);
let projection_ty_as_ty =
@ -166,21 +178,25 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
// see the extensive comment in projection_must_outlive
let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs);
let recursive_bound = self.recursive_bound(ty.into());
let recursive_bound = self.recursive_bound(ty.into(), visited);
VerifyBound::AnyBound(env_bounds.chain(trait_bounds).collect()).or(recursive_bound)
}
fn recursive_bound(&self, parent: GenericArg<'tcx>) -> VerifyBound<'tcx> {
fn recursive_bound(
&self,
parent: GenericArg<'tcx>,
visited: &mut MiniSet<GenericArg<'tcx>>,
) -> VerifyBound<'tcx> {
let mut bounds = parent
.walk_shallow()
.walk_shallow(visited)
.filter_map(|child| match child.unpack() {
GenericArgKind::Type(ty) => Some(self.type_bound(ty)),
GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)),
GenericArgKind::Lifetime(lt) => {
// Ignore late-bound regions.
if !lt.is_late_bound() { Some(VerifyBound::OutlivedBy(lt)) } else { None }
}
GenericArgKind::Const(_) => Some(self.recursive_bound(child)),
GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)),
})
.filter(|bound| {
// Remove bounds that must hold, since they are not interesting.

1
compiler/rustc_middle/Cargo.toml
View File

@ -28,5 +28,6 @@ rustc_ast = { path = "../rustc_ast" }
rustc_span = { path = "../rustc_span" }
chalk-ir = "0.21.0"
smallvec = { version = "1.0", features = ["union", "may_dangle"] }
arrayvec = { version = "0.5.1", default-features = false }
measureme = "0.7.1"
rustc_session = { path = "../rustc_session" }

33
compiler/rustc_middle/src/ty/outlives.rs
View File

@ -3,6 +3,7 @@
// RFC for reference.
use crate::ty::subst::{GenericArg, GenericArgKind};
use crate::ty::walk::MiniSet;
use crate::ty::{self, Ty, TyCtxt, TypeFoldable};
use smallvec::SmallVec;
@ -50,12 +51,18 @@ impl<'tcx> TyCtxt<'tcx> {
/// Push onto `out` all the things that must outlive `'a` for the condition
/// `ty0: 'a` to hold. Note that `ty0` must be a **fully resolved type**.
pub fn push_outlives_components(self, ty0: Ty<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>) {
compute_components(self, ty0, out);
let mut visited = MiniSet::new();
compute_components(self, ty0, out, &mut visited);
debug!("components({:?}) = {:?}", ty0, out);
}
}
fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>) {
fn compute_components(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
out: &mut SmallVec<[Component<'tcx>; 4]>,
visited: &mut MiniSet<GenericArg<'tcx>>,
) {
// Descend through the types, looking for the various "base"
// components and collecting them into `out`. This is not written
// with `collect()` because of the need to sometimes skip subtrees
@ -73,11 +80,11 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo
for child in substs {
match child.unpack() {
GenericArgKind::Type(ty) => {
compute_components(tcx, ty, out);
compute_components(tcx, ty, out, visited);
}
GenericArgKind::Lifetime(_) => {}
GenericArgKind::Const(_) => {
compute_components_recursive(tcx, child, out);
compute_components_recursive(tcx, child, out, visited);
}
}
}
@ -85,19 +92,19 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo
ty::Array(element, _) => {
// Don't look into the len const as it doesn't affect regions
compute_components(tcx, element, out);
compute_components(tcx, element, out, visited);
}
ty::Closure(_, ref substs) => {
for upvar_ty in substs.as_closure().upvar_tys() {
compute_components(tcx, upvar_ty, out);
compute_components(tcx, upvar_ty, out, visited);
}
}
ty::Generator(_, ref substs, _) => {
// Same as the closure case
for upvar_ty in substs.as_generator().upvar_tys() {
compute_components(tcx, upvar_ty, out);
compute_components(tcx, upvar_ty, out, visited);
}
// We ignore regions in the generator interior as we don't
@ -135,7 +142,8 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo
// OutlivesProjectionComponents. Continue walking
// through and constrain Pi.
let mut subcomponents = smallvec![];
compute_components_recursive(tcx, ty.into(), &mut subcomponents);
let mut subvisited = MiniSet::new();
compute_components_recursive(tcx, ty.into(), &mut subcomponents, &mut subvisited);
out.push(Component::EscapingProjection(subcomponents.into_iter().collect()));
}
}
@ -177,7 +185,7 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo
// the "bound regions list". In our representation, no such
// list is maintained explicitly, because bound regions
// themselves can be readily identified.
compute_components_recursive(tcx, ty.into(), out);
compute_components_recursive(tcx, ty.into(), out, visited);
}
}
}
@ -186,11 +194,12 @@ fn compute_components_recursive(
tcx: TyCtxt<'tcx>,
parent: GenericArg<'tcx>,
out: &mut SmallVec<[Component<'tcx>; 4]>,
visited: &mut MiniSet<GenericArg<'tcx>>,
) {
for child in parent.walk_shallow() {
for child in parent.walk_shallow(visited) {
match child.unpack() {
GenericArgKind::Type(ty) => {
compute_components(tcx, ty, out);
compute_components(tcx, ty, out, visited);
}
GenericArgKind::Lifetime(lt) => {
// Ignore late-bound regions.
@ -199,7 +208,7 @@ fn compute_components_recursive(
}
}
GenericArgKind::Const(_) => {
compute_components_recursive(tcx, child, out);
compute_components_recursive(tcx, child, out, visited);
}
}
}

80
compiler/rustc_middle/src/ty/walk.rs
View File

@ -3,7 +3,50 @@
use crate::ty;
use crate::ty::subst::{GenericArg, GenericArgKind};
use arrayvec::ArrayVec;
use rustc_data_structures::fx::FxHashSet;
use smallvec::{self, SmallVec};
use std::hash::Hash;
/// Small-storage-optimized implementation of a set
/// made specifically for walking type tree.
///
/// Stores elements in a small array up to a certain length
/// and switches to `HashSet` when that length is exceeded.
pub enum MiniSet<T> {
Array(ArrayVec<[T; 8]>),
Set(FxHashSet<T>),
}
impl<T: Eq + Hash + Copy> MiniSet<T> {
/// Creates an empty `MiniSet`.
pub fn new() -> Self {
MiniSet::Array(ArrayVec::new())
}
/// Adds a value to the set.
///
/// If the set did not have this value present, true is returned.
///
/// If the set did have this value present, false is returned.
pub fn insert(&mut self, elem: T) -> bool {
match self {
MiniSet::Array(array) => {
if array.iter().any(|e| *e == elem) {
false
} else {
if array.try_push(elem).is_err() {
let mut set: FxHashSet<T> = array.iter().copied().collect();
set.insert(elem);
*self = MiniSet::Set(set);
}
true
}
}
MiniSet::Set(set) => set.insert(elem),
}
}
}
// The TypeWalker's stack is hot enough that it's worth going to some effort to
// avoid heap allocations.
@ -12,11 +55,20 @@ type TypeWalkerStack<'tcx> = SmallVec<[GenericArg<'tcx>; 8]>;
pub struct TypeWalker<'tcx> {
stack: TypeWalkerStack<'tcx>,
last_subtree: usize,
visited: MiniSet<GenericArg<'tcx>>,
}
/// An iterator for walking the type tree.
///
/// It's very easy to produce a deeply
/// nested type tree with a lot of
/// identical subtrees. In order to work efficiently
/// in this situation walker only visits each type once.
/// It maintains a set of visited types and
/// skips any types that are already there.
impl<'tcx> TypeWalker<'tcx> {
pub fn new(root: GenericArg<'tcx>) -> TypeWalker<'tcx> {
TypeWalker { stack: smallvec![root], last_subtree: 1 }
pub fn new(root: GenericArg<'tcx>) -> Self {
Self { stack: smallvec![root], last_subtree: 1, visited: MiniSet::new() }
}
/// Skips the subtree corresponding to the last type
@ -41,11 +93,15 @@ impl<'tcx> Iterator for TypeWalker<'tcx> {
fn next(&mut self) -> Option<GenericArg<'tcx>> {
debug!("next(): stack={:?}", self.stack);
let next = self.stack.pop()?;
self.last_subtree = self.stack.len();
push_inner(&mut self.stack, next);
debug!("next: stack={:?}", self.stack);
Some(next)
loop {
let next = self.stack.pop()?;
self.last_subtree = self.stack.len();
if self.visited.insert(next) {
push_inner(&mut self.stack, next);
debug!("next: stack={:?}", self.stack);
return Some(next);
}
}
}
}
@ -67,9 +123,17 @@ impl GenericArg<'tcx> {
/// Iterator that walks the immediate children of `self`. Hence
/// `Foo<Bar<i32>, u32>` yields the sequence `[Bar<i32>, u32]`
/// (but not `i32`, like `walk`).
pub fn walk_shallow(self) -> impl Iterator<Item = GenericArg<'tcx>> {
///
/// Iterator only walks items once.
/// It accepts visited set, updates it with all visited types
/// and skips any types that are already there.
pub fn walk_shallow(
self,
visited: &mut MiniSet<GenericArg<'tcx>>,
) -> impl Iterator<Item = GenericArg<'tcx>> {
let mut stack = SmallVec::new();
push_inner(&mut stack, self);
stack.retain(|a| visited.insert(*a));
stack.into_iter()
}
}

59
src/test/ui/closures/issue-72408-nested-closures-exponential.rs Normal file
View File

@ -0,0 +1,59 @@
// build-pass
// Closures include captured types twice in a type tree.
//
// Wrapping one closure with another leads to doubling
// the amount of types in the type tree.
//
// This test ensures that rust can handle
// deeply nested type trees with a lot
// of duplicated subtrees.
fn dup(f: impl Fn(i32) -> i32) -> impl Fn(i32) -> i32 {
move |a| f(a * 2)
}
fn main() {
let f = |a| a;
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
// Compiler dies around here if it tries
// to walk the tree exhaustively.
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
let f = dup(f);
println!("Type size was at least {}", f(1));
}

2
src/test/ui/issues/issue-22638.rs
View File

@ -51,9 +51,9 @@ struct D (Box<A>);
impl D {
pub fn matches<F: Fn()>(&self, f: &F) {
//~^ ERROR reached the type-length limit while instantiating `D::matches::<[closure
let &D(ref a) = self;
a.matches(f)
//~^ ERROR reached the recursion limit while instantiating `A::matches::<[closure
}
}

12
src/test/ui/issues/issue-22638.stderr
View File

@ -1,10 +1,14 @@
error: reached the type-length limit while instantiating `D::matches::$CLOSURE`
--> $DIR/issue-22638.rs:53:5
error: reached the recursion limit while instantiating `A::matches::$CLOSURE`
--> $DIR/issue-22638.rs:55:9
|
LL | a.matches(f)
| ^^^^^^^^^^^^
|
note: `A::matches` defined here
--> $DIR/issue-22638.rs:14:5
|
LL | pub fn matches<F: Fn()>(&self, f: &F) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: consider adding a `#![type_length_limit="30408681"]` attribute to your crate
error: aborting due to previous error

2
src/test/ui/type_length_limit.rs
View File

@ -4,7 +4,7 @@
// Test that the type length limit can be changed.
#![allow(dead_code)]
#![type_length_limit="256"]
#![type_length_limit="4"]
macro_rules! link {
($id:ident, $t:ty) => {

2
src/test/ui/type_length_limit.stderr
View File

@ -4,7 +4,7 @@ error: reached the type-length limit while instantiating `std::mem::drop::<Optio
LL | pub fn drop<T>(_x: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: consider adding a `#![type_length_limit="1094"]` attribute to your crate
= note: consider adding a `#![type_length_limit="8"]` attribute to your crate
error: aborting due to previous error