From 4749175ced2eca288e5c0de57bb50daae406f8cd Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 4 Nov 2016 17:31:52 +1100 Subject: [PATCH] rework `TraitSelect` to avoid a vec and just use two def-ids --- src/librustc/dep_graph/dep_node.rs | 46 +++++++++++++++++++++++++++--- src/librustc/ty/mod.rs | 30 ++++++++----------- src/librustc_trans/context.rs | 3 +- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 006de1c06e2..96d1a925425 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -131,7 +131,37 @@ pub enum DepNode { // which would yield an overly conservative dep-graph. TraitItems(D), ReprHints(D), - TraitSelect(Vec), + + // Trait selection cache is a little funny. Given a trait + // reference like `Foo: SomeTrait`, 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: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Vec }` + // - `String: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: String }` + // - `Foo: Trait` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }` + // - `Foo: Trait` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }` + // - `(Foo, Bar): Trait` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }` + // - `i32: Trait` -> `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 }, } impl DepNode { @@ -236,9 +266,17 @@ pub fn map_def(&self, mut op: OP) -> Option> 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> = def_ids.iter().map(op).collect(); + def_ids.map(|d| ProjectionCache { def_ids: d }) } } } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 411e14531fa..7937d2ccfe4 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -34,7 +34,6 @@ 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; @@ -843,27 +842,22 @@ pub fn def_id(&self) -> DefId { /// Creates the dep-node for selecting/evaluating this trait reference. fn dep_node(&self) -> DepNode { - // 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: SomeTrait`, - // `Rc: SomeTrait`, and `(Vec, Rc): 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 + // 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> + 'a { diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 5efdd129a32..799f502aadb 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -208,7 +208,8 @@ fn to_dep_node(key: &Self::Key) -> DepNode { _ => None, }) .collect(); - DepNode::TraitSelect(def_ids) + + DepNode::ProjectionCache { def_ids: def_ids } } }