Skip to content

rework TraitSelect to avoid a vec and just use two def-ids #39912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,37 @@ pub enum DepNode<D: Clone + Debug> {
// which would yield an overly conservative dep-graph.
TraitItems(D),
ReprHints(D),
TraitSelect(Vec<D>),

// Trait selection cache is a little funny. Given a trait
// reference like `Foo: SomeTrait<Bar>`, there could be
// arbitrarily many def-ids to map on in there (e.g., `Foo`,
// `SomeTrait`, `Bar`). We could have a vector of them, but it
// requires heap-allocation, and trait sel in general can be a
// surprisingly hot path. So instead we pick two def-ids: the
// trait def-id, and the first def-id in the input types. If there
// is no def-id in the input types, then we use the trait def-id
// again. So for example:
//
// - `i32: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `u32: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `Clone: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `Vec<i32>: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Vec }`
// - `String: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: String }`
// - `Foo: Trait<Bar>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `Foo: Trait<i32>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `(Foo, Bar): Trait` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `i32: Trait<Foo>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
//
// You can see that we map many trait refs to the same
// trait-select node. This is not a problem, it just means
// imprecision in our dep-graph tracking. The important thing is
// that for any given trait-ref, we always map to the **same**
// trait-select node.
TraitSelect { trait_def_id: D, input_def_id: D },

// For proj. cache, we just keep a list of all def-ids, since it is
// not a hotspot.
ProjectionCache { def_ids: Vec<D> },
}

impl<D: Clone + Debug> DepNode<D> {
Expand Down Expand Up @@ -236,9 +266,17 @@ impl<D: Clone + Debug> DepNode<D> {
TraitImpls(ref d) => op(d).map(TraitImpls),
TraitItems(ref d) => op(d).map(TraitItems),
ReprHints(ref d) => op(d).map(ReprHints),
TraitSelect(ref type_ds) => {
let type_ds = try_opt!(type_ds.iter().map(|d| op(d)).collect());
Some(TraitSelect(type_ds))
TraitSelect { ref trait_def_id, ref input_def_id } => {
op(trait_def_id).and_then(|trait_def_id| {
op(input_def_id).and_then(|input_def_id| {
Some(TraitSelect { trait_def_id: trait_def_id,
input_def_id: input_def_id })
})
})
}
ProjectionCache { ref def_ids } => {
let def_ids: Option<Vec<E>> = def_ids.iter().map(op).collect();
def_ids.map(|d| ProjectionCache { def_ids: d })
}
}
}
Expand Down
30 changes: 12 additions & 18 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use serialize::{self, Encodable, Encoder};
use std::borrow::Cow;
use std::cell::{Cell, RefCell, Ref};
use std::hash::{Hash, Hasher};
use std::iter;
use std::ops::Deref;
use std::rc::Rc;
use std::slice;
Expand Down Expand Up @@ -843,27 +842,22 @@ impl<'tcx> TraitPredicate<'tcx> {

/// Creates the dep-node for selecting/evaluating this trait reference.
fn dep_node(&self) -> DepNode<DefId> {
// Ideally, the dep-node would just have all the input types
// in it. But they are limited to including def-ids. So as an
// approximation we include the def-ids for all nominal types
// found somewhere. This means that we will e.g. conflate the
// dep-nodes for `u32: SomeTrait` and `u64: SomeTrait`, but we
// would have distinct dep-nodes for `Vec<u32>: SomeTrait`,
// `Rc<u32>: SomeTrait`, and `(Vec<u32>, Rc<u32>): SomeTrait`.
// Note that it's always sound to conflate dep-nodes, it just
// leads to more recompilation.
let def_ids: Vec<_> =
// Extact the trait-def and first def-id from inputs. See the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Extract"

// docs for `DepNode::TraitSelect` for more information.
let trait_def_id = self.def_id();
let input_def_id =
self.input_types()
.flat_map(|t| t.walk())
.filter_map(|t| match t.sty {
ty::TyAdt(adt_def, _) =>
Some(adt_def.did),
_ =>
None
ty::TyAdt(adt_def, _) => Some(adt_def.did),
_ => None
})
.chain(iter::once(self.def_id()))
.collect();
DepNode::TraitSelect(def_ids)
.next()
.unwrap_or(trait_def_id);
DepNode::TraitSelect {
trait_def_id: trait_def_id,
input_def_id: input_def_id
}
}

pub fn input_types<'a>(&'a self) -> impl DoubleEndedIterator<Item=Ty<'tcx>> + 'a {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ impl<'gcx> DepTrackingMapConfig for ProjectionCache<'gcx> {
_ => None,
})
.collect();
DepNode::TraitSelect(def_ids)

DepNode::ProjectionCache { def_ids: def_ids }
}
}

Expand Down