Auto merge of #41507 - arielb1:symbol-cache, r=nikomatsakis

cache symbol names in ty::maps

this fixes a performance regression introduced in commit 39a58c3.

r? @nikomatsakis

Author: bors

src/librustc/dep_graph/dep_node.rs

@@ -99,6 +99,7 @@ pub enum DepNode<D: Clone + Debug> {
     TypeckTables(D),
     UsedTraitImports(D),
     ConstEval(D),
+    SymbolName(D),
 
     // The set of impls for a given trait. Ultimately, it would be
     // nice to get more fine-grained here (e.g., to include a
@@ -236,6 +237,7 @@ impl<D: Clone + Debug> DepNode<D> {
             TypeckTables(ref d) => op(d).map(TypeckTables),
             UsedTraitImports(ref d) => op(d).map(UsedTraitImports),
             ConstEval(ref d) => op(d).map(ConstEval),
+            SymbolName(ref d) => op(d).map(SymbolName),
             TraitImpls(ref d) => op(d).map(TraitImpls),
             TraitItems(ref d) => op(d).map(TraitItems),
             ReprHints(ref d) => op(d).map(ReprHints),

src/librustc/ty/maps.rs

@@ -24,6 +24,7 @@ use std::cell::{RefCell, RefMut};
 use std::ops::Deref;
 use std::rc::Rc;
 use syntax_pos::{Span, DUMMY_SP};
+use syntax::symbol::Symbol;
 
 trait Key {
     fn map_crate(&self) -> CrateNum;
@@ -40,6 +41,16 @@ impl<'tcx> Key for ty::InstanceDef<'tcx> {
     }
 }
 
+impl<'tcx> Key for ty::Instance<'tcx> {
+    fn map_crate(&self) -> CrateNum {
+        LOCAL_CRATE
+    }
+
+    fn default_span(&self, tcx: TyCtxt) -> Span {
+        tcx.def_span(self.def_id())
+    }
+}
+
 impl Key for CrateNum {
     fn map_crate(&self) -> CrateNum {
         *self
@@ -108,13 +119,18 @@ impl<'tcx> Value<'tcx> for Ty<'tcx> {
     }
 }
 
-
 impl<'tcx> Value<'tcx> for ty::DtorckConstraint<'tcx> {
     fn from_cycle_error<'a>(_: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
         Self::empty()
     }
 }
 
+impl<'tcx> Value<'tcx> for ty::SymbolName {
+    fn from_cycle_error<'a>(_: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
+        ty::SymbolName { name: Symbol::intern("<error>").as_str() }
+    }
+}
+
 pub struct CycleError<'a, 'tcx: 'a> {
     span: Span,
     cycle: RefMut<'a, [(Span, Query<'tcx>)]>,
@@ -242,6 +258,12 @@ impl<'tcx> QueryDescription for queries::const_eval<'tcx> {
     }
 }
 
+impl<'tcx> QueryDescription for queries::symbol_name<'tcx> {
+    fn describe(_tcx: TyCtxt, instance: ty::Instance<'tcx>) -> String {
+        format!("computing the symbol for `{}`", instance)
+    }
+}
+
 macro_rules! define_maps {
     (<$tcx:tt>
      $($(#[$attr:meta])*
@@ -513,7 +535,10 @@ define_maps! { <'tcx>
 
     pub reachable_set: reachability_dep_node(CrateNum) -> Rc<NodeSet>,
 
-    pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>
+    pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>,
+
+    pub def_symbol_name: SymbolName(DefId) -> ty::SymbolName,
+    pub symbol_name: symbol_name_dep_node(ty::Instance<'tcx>) -> ty::SymbolName
 }
 
 fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode<DefId> {
@@ -532,6 +557,12 @@ fn mir_shim_dep_node(instance: ty::InstanceDef) -> DepNode<DefId> {
     instance.dep_node()
 }
 
+fn symbol_name_dep_node(instance: ty::Instance) -> DepNode<DefId> {
+    // symbol_name uses the substs only to traverse them to find the
+    // hash, and that does not create any new dep-nodes.
+    DepNode::SymbolName(instance.def.def_id())
+}
+
 fn typeck_item_bodies_dep_node(_: CrateNum) -> DepNode<DefId> {
     DepNode::TypeckBodiesKrate
 }

src/librustc/ty/mod.rs

@@ -38,6 +38,7 @@ use serialize::{self, Encodable, Encoder};
 use std::cell::{Cell, RefCell, Ref};
 use std::collections::BTreeMap;
 use std::cmp;
+use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::iter::FromIterator;
 use std::ops::Deref;
@@ -2745,3 +2746,22 @@ impl<'tcx> DtorckConstraint<'tcx> {
         self.dtorck_types.retain(|&val| dtorck_types.replace(val).is_none());
     }
 }
+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub struct SymbolName {
+    // FIXME: we don't rely on interning or equality here - better have
+    // this be a `&'tcx str`.
+    pub name: InternedString
+}
+
+impl Deref for SymbolName {
+    type Target = str;
+
+    fn deref(&self) -> &str { &self.name }
+}
+
+impl fmt::Display for SymbolName {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.name, fmt)
+    }
+}

src/librustc_driver/driver.rs

@@ -225,8 +225,6 @@ pub fn compile_input(sess: &Session,
         sess.code_stats.borrow().print_type_sizes();
     }
 
-    if ::std::env::var("SKIP_LLVM").is_ok() { ::std::process::exit(0); }
-
     let phase5_result = phase_5_run_llvm_passes(sess, &trans, &outputs);
 
     controller_entry_point!(after_llvm,
@@ -895,13 +893,15 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
     mir::provide(&mut local_providers);
     reachable::provide(&mut local_providers);
     rustc_privacy::provide(&mut local_providers);
+    trans::provide(&mut local_providers);
     typeck::provide(&mut local_providers);
     ty::provide(&mut local_providers);
     reachable::provide(&mut local_providers);
     rustc_const_eval::provide(&mut local_providers);
 
     let mut extern_providers = ty::maps::Providers::default();
     cstore::provide(&mut extern_providers);
+    trans::provide(&mut extern_providers);
     ty::provide_extern(&mut extern_providers);
     // FIXME(eddyb) get rid of this once we replace const_eval with miri.
     rustc_const_eval::provide(&mut extern_providers);

src/librustc_trans/back/symbol_export.rs

@@ -10,14 +10,11 @@
 
 use context::SharedCrateContext;
 use monomorphize::Instance;
-use symbol_map::SymbolMap;
-use back::symbol_names::symbol_name;
 use util::nodemap::FxHashMap;
 use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE};
 use rustc::session::config;
 use rustc::ty::TyCtxt;
 use syntax::attr;
-use trans_item::TransItem;
 
 /// The SymbolExportLevel of a symbols specifies from which kinds of crates
 /// the symbol will be exported. `C` symbols will be exported from any
@@ -36,27 +33,24 @@ pub struct ExportedSymbols {
 }
 
 impl ExportedSymbols {
-
     pub fn empty() -> ExportedSymbols {
         ExportedSymbols {
             exports: FxHashMap(),
         }
     }
 
-    pub fn compute_from<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
-                                  symbol_map: &SymbolMap<'tcx>)
-                                  -> ExportedSymbols {
+    pub fn compute<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) -> ExportedSymbols {
         let mut local_crate: Vec<_> = scx
             .exported_symbols()
             .iter()
             .map(|&node_id| {
                 scx.tcx().hir.local_def_id(node_id)
             })
             .map(|def_id| {
-                let name = symbol_for_def_id(scx.tcx(), def_id, symbol_map);
+                let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id));
                 let export_level = export_level(scx, def_id);
                 debug!("EXPORTED SYMBOL (local): {} ({:?})", name, export_level);
-                (name, export_level)
+                (str::to_owned(&name), export_level)
             })
             .collect();
 
@@ -108,7 +102,7 @@ impl ExportedSymbols {
                 .exported_symbols(cnum)
                 .iter()
                 .map(|&def_id| {
-                    let name = symbol_name(Instance::mono(scx.tcx(), def_id), scx.tcx());
+                    let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id));
                     let export_level = if special_runtime_crate {
                         // We can probably do better here by just ensuring that
                         // it has hidden visibility rather than public
@@ -117,9 +111,9 @@ impl ExportedSymbols {
                         //
                         // In general though we won't link right if these
                         // symbols are stripped, and LTO currently strips them.
-                        if name == "rust_eh_personality" ||
-                           name == "rust_eh_register_frames" ||
-                           name == "rust_eh_unregister_frames" {
+                        if &*name == "rust_eh_personality" ||
+                           &*name == "rust_eh_register_frames" ||
+                           &*name == "rust_eh_unregister_frames" {
                             SymbolExportLevel::C
                         } else {
                             SymbolExportLevel::Rust
@@ -128,7 +122,7 @@ impl ExportedSymbols {
                         export_level(scx, def_id)
                     };
                     debug!("EXPORTED SYMBOL (re-export): {} ({:?})", name, export_level);
-                    (name, export_level)
+                    (str::to_owned(&name), export_level)
                 })
                 .collect();
 
@@ -213,22 +207,3 @@ pub fn is_below_threshold(level: SymbolExportLevel,
         level == SymbolExportLevel::C
     }
 }
-
-fn symbol_for_def_id<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                               def_id: DefId,
-                               symbol_map: &SymbolMap<'tcx>)
-                               -> String {
-    // Just try to look things up in the symbol map. If nothing's there, we
-    // recompute.
-    if let Some(node_id) = tcx.hir.as_local_node_id(def_id) {
-        if let Some(sym) = symbol_map.get(TransItem::Static(node_id)) {
-            return sym.to_owned();
-        }
-    }
-
-    let instance = Instance::mono(tcx, def_id);
-
-    symbol_map.get(TransItem::Fn(instance))
-              .map(str::to_owned)
-              .unwrap_or_else(|| symbol_name(instance, tcx))
-}

src/librustc_trans/back/symbol_names.rs

@@ -105,14 +105,24 @@ use rustc::hir::map as hir_map;
 use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
 use rustc::ty::fold::TypeVisitor;
 use rustc::ty::item_path::{self, ItemPathBuffer, RootMode};
+use rustc::ty::maps::Providers;
 use rustc::ty::subst::Substs;
 use rustc::hir::map::definitions::DefPathData;
 use rustc::util::common::record_time;
 
 use syntax::attr;
+use syntax_pos::symbol::Symbol;
 
 use std::fmt::Write;
 
+pub fn provide(providers: &mut Providers) {
+    *providers = Providers {
+        def_symbol_name,
+        symbol_name,
+        ..*providers
+    };
+}
+
 fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
                              // the DefId of the item this name is for
@@ -127,7 +137,7 @@ fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                              // values for generic type parameters,
                              // if any.
                              substs: Option<&'tcx Substs<'tcx>>)
-                             -> String {
+                             -> u64 {
     debug!("get_symbol_hash(def_id={:?}, parameters={:?})", def_id, substs);
 
     let mut hasher = ty::util::TypeIdHasher::<u64>::new(tcx);
@@ -162,11 +172,28 @@ fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     });
 
     // 64 bits should be enough to avoid collisions.
-    format!("h{:016x}", hasher.finish())
+    hasher.finish()
+}
+
+fn def_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
+                             -> ty::SymbolName
+{
+    let mut buffer = SymbolPathBuffer::new();
+    item_path::with_forced_absolute_paths(|| {
+        tcx.push_item_path(&mut buffer, def_id);
+    });
+    buffer.into_interned()
 }
 
-pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>,
-                             tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
+fn symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>)
+                         -> ty::SymbolName
+{
+    ty::SymbolName { name: Symbol::intern(&compute_symbol_name(tcx, instance)).as_str() }
+}
+
+fn compute_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>)
+    -> String
+{
     let def_id = instance.def_id();
     let substs = instance.substs;
 
@@ -253,11 +280,7 @@ pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>,
 
     let hash = get_symbol_hash(tcx, Some(def_id), instance_ty, Some(substs));
 
-    let mut buffer = SymbolPathBuffer::new();
-    item_path::with_forced_absolute_paths(|| {
-        tcx.push_item_path(&mut buffer, def_id);
-    });
-    buffer.finish(&hash)
+    SymbolPathBuffer::from_interned(tcx.def_symbol_name(def_id)).finish(hash)
 }
 
 // Follow C++ namespace-mangling style, see
@@ -288,10 +311,22 @@ impl SymbolPathBuffer {
         result
     }
 
-    fn finish(mut self, hash: &str) -> String {
-        // end name-sequence
-        self.push(hash);
-        self.result.push('E');
+    fn from_interned(symbol: ty::SymbolName) -> Self {
+        let mut result = SymbolPathBuffer {
+            result: String::with_capacity(64),
+            temp_buf: String::with_capacity(16)
+        };
+        result.result.push_str(&symbol.name);
+        result
+    }
+
+    fn into_interned(self) -> ty::SymbolName {
+        ty::SymbolName { name: Symbol::intern(&self.result).as_str() }
+    }
+
+    fn finish(mut self, hash: u64) -> String {
+        // E = end name-sequence
+        let _ = write!(self.result, "17h{:016x}E", hash);
         self.result
     }
 }
@@ -320,7 +355,7 @@ pub fn exported_name_from_type_and_prefix<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     let hash = get_symbol_hash(tcx, None, t, None);
     let mut buffer = SymbolPathBuffer::new();
     buffer.push(prefix);
-    buffer.finish(&hash)
+    buffer.finish(hash)
 }
 
 // Name sanitation. LLVM will happily accept identifiers with weird names, but