introduce the rather simpler symbol-cache in place of symbol-map

The symbol map is not good for incremental: it has inputs from every fn
in existence, and it will change if anything changes. One could imagine
cheating with the symbol-map and exempting it from the usual dependency
tracking, since the results are fully deterministic. Instead, I opted to
just add a per-CGU cache, on the premise that recomputing some symbol
names is not going to be so very expensive.

Author: nikomatsakis

src/librustc_trans/base.rs

@@ -65,6 +65,7 @@ use meth;
 use mir;
 use monomorphize::{self, Instance};
 use partitioning::{self, PartitioningStrategy, CodegenUnit};
+use symbol_cache::SymbolCache;
 use symbol_map::SymbolMap;
 use symbol_names_test;
 use trans_item::{TransItem, DefPathBasedNames};
@@ -75,7 +76,6 @@ use util::nodemap::{NodeSet, FxHashMap, FxHashSet};
 
 use libc::c_uint;
 use std::ffi::{CStr, CString};
-use std::rc::Rc;
 use std::str;
 use std::i32;
 use syntax_pos::Span;
@@ -1113,8 +1113,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     let (translation_items, codegen_units, symbol_map) =
         collect_and_partition_translation_items(&shared_ccx);
 
-    let symbol_map = Rc::new(symbol_map);
-
     let mut all_stats = Stats::default();
     let modules: Vec<ModuleTranslation> = codegen_units
         .into_iter()
@@ -1123,7 +1121,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             let (stats, module) =
                 tcx.dep_graph.with_task(dep_node,
                                         AssertDepGraphSafe(&shared_ccx),
-                                        AssertDepGraphSafe((cgu, symbol_map.clone())),
+                                        AssertDepGraphSafe(cgu),
                                         module_translation);
             all_stats.extend(stats);
             module
@@ -1132,16 +1130,17 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     fn module_translation<'a, 'tcx>(
         scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>,
-        args: AssertDepGraphSafe<(CodegenUnit<'tcx>, Rc<SymbolMap<'tcx>>)>)
+        args: AssertDepGraphSafe<CodegenUnit<'tcx>>)
         -> (Stats, ModuleTranslation)
     {
         // FIXME(#40304): We ought to be using the id as a key and some queries, I think.
         let AssertDepGraphSafe(scx) = scx;
-        let AssertDepGraphSafe((cgu, symbol_map)) = args;
+        let AssertDepGraphSafe(cgu) = args;
 
         let cgu_name = String::from(cgu.name());
         let cgu_id = cgu.work_product_id();
-        let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &symbol_map);
+        let symbol_cache = SymbolCache::new(scx);
+        let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &symbol_cache);
 
         // Check whether there is a previous work-product we can
         // re-use.  Not only must the file exist, and the inputs not
@@ -1176,11 +1175,11 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         }
 
         // Instantiate translation items without filling out definitions yet...
-        let lcx = LocalCrateContext::new(scx, cgu, symbol_map.clone());
+        let lcx = LocalCrateContext::new(scx, cgu, &symbol_cache);
         let module = {
             let ccx = CrateContext::new(scx, &lcx);
             let trans_items = ccx.codegen_unit()
-                                 .items_in_deterministic_order(ccx.tcx(), &symbol_map);
+                                 .items_in_deterministic_order(ccx.tcx(), &symbol_cache);
             for &(trans_item, linkage) in &trans_items {
                 trans_item.predefine(&ccx, linkage);
             }

src/librustc_trans/callee.rs

@@ -51,8 +51,7 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
         return llfn;
     }
 
-    let sym = ccx.symbol_map().get_or_compute(ccx.shared(),
-                                              TransItem::Fn(instance));
+    let sym = ccx.symbol_cache().get(TransItem::Fn(instance));
     debug!("get_fn({:?}: {:?}) => {}", instance, fn_ty, sym);
 
     // This is subtle and surprising, but sometimes we have to bitcast

src/librustc_trans/consts.rs

@@ -93,20 +93,19 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef {
             hir_map::NodeItem(&hir::Item {
                 ref attrs, span, node: hir::ItemStatic(..), ..
             }) => {
-                let sym = ccx.symbol_map()
-                             .get(TransItem::Static(id))
-                             .expect("Local statics should always be in the SymbolMap");
+                let sym = ccx.symbol_cache()
+                             .get(TransItem::Static(id));
 
                 let defined_in_current_codegen_unit = ccx.codegen_unit()
                                                          .items()
                                                          .contains_key(&TransItem::Static(id));
                 assert!(!defined_in_current_codegen_unit);
 
-                if declare::get_declared_value(ccx, sym).is_some() {
+                if declare::get_declared_value(ccx, &sym[..]).is_some() {
                     span_bug!(span, "trans: Conflicting symbol names for static?");
                 }
 
-                let g = declare::define_global(ccx, sym, llty).unwrap();
+                let g = declare::define_global(ccx, &sym[..], llty).unwrap();
 
                 (g, attrs)
             }

src/librustc_trans/context.rs

@@ -29,15 +29,14 @@ use rustc::ty::layout::{LayoutTyper, TyLayout};
 use session::config::NoDebugInfo;
 use session::Session;
 use session::config;
-use symbol_map::SymbolMap;
+use symbol_cache::SymbolCache;
 use util::nodemap::{NodeSet, DefIdMap, FxHashMap};
 
 use std::ffi::{CStr, CString};
 use std::cell::{Cell, RefCell};
 use std::marker::PhantomData;
 use std::ptr;
 use std::iter;
-use std::rc::Rc;
 use std::str;
 use syntax::ast;
 use syntax::symbol::InternedString;
@@ -94,7 +93,7 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> {
 /// per compilation unit.  Each one has its own LLVM `ContextRef` so that
 /// several compilation units may be optimized in parallel.  All other LLVM
 /// data structures in the `LocalCrateContext` are tied to that `ContextRef`.
-pub struct LocalCrateContext<'tcx> {
+pub struct LocalCrateContext<'a, 'tcx: 'a> {
     llmod: ModuleRef,
     llcx: ContextRef,
     stats: Stats,
@@ -166,10 +165,10 @@ pub struct LocalCrateContext<'tcx> {
     /// Depth of the current type-of computation - used to bail out
     type_of_depth: Cell<usize>,
 
-    symbol_map: Rc<SymbolMap<'tcx>>,
-
     /// A counter that is used for generating local symbol names
     local_gen_sym_counter: Cell<usize>,
+
+    symbol_cache: &'a SymbolCache<'a, 'tcx>,
 }
 
 // Implement DepTrackingMapConfig for `trait_cache`
@@ -227,12 +226,12 @@ impl<'gcx> DepTrackingMapConfig for ProjectionCache<'gcx> {
 /// pass around (SharedCrateContext, LocalCrateContext) tuples all over trans.
 pub struct CrateContext<'a, 'tcx: 'a> {
     shared: &'a SharedCrateContext<'a, 'tcx>,
-    local_ccx: &'a LocalCrateContext<'tcx>,
+    local_ccx: &'a LocalCrateContext<'a, 'tcx>,
 }
 
 impl<'a, 'tcx> CrateContext<'a, 'tcx> {
     pub fn new(shared: &'a SharedCrateContext<'a, 'tcx>,
-               local_ccx: &'a LocalCrateContext<'tcx>)
+               local_ccx: &'a LocalCrateContext<'a, 'tcx>)
                -> Self {
         CrateContext { shared, local_ccx }
     }
@@ -429,11 +428,11 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> {
     }
 }
 
-impl<'tcx> LocalCrateContext<'tcx> {
-    pub fn new<'a>(shared: &SharedCrateContext<'a, 'tcx>,
-                   codegen_unit: CodegenUnit<'tcx>,
-                   symbol_map: Rc<SymbolMap<'tcx>>)
-                   -> LocalCrateContext<'tcx> {
+impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> {
+    pub fn new(shared: &SharedCrateContext<'a, 'tcx>,
+               codegen_unit: CodegenUnit<'tcx>,
+               symbol_cache: &'a SymbolCache<'a, 'tcx>)
+               -> LocalCrateContext<'a, 'tcx> {
         unsafe {
             // Append ".rs" to LLVM module identifier.
             //
@@ -487,8 +486,8 @@ impl<'tcx> LocalCrateContext<'tcx> {
                 rust_try_fn: Cell::new(None),
                 intrinsics: RefCell::new(FxHashMap()),
                 type_of_depth: Cell::new(0),
-                symbol_map: symbol_map,
                 local_gen_sym_counter: Cell::new(0),
+                symbol_cache: symbol_cache,
             };
 
             let (int_type, opaque_vec_type, str_slice_ty, mut local_ccx) = {
@@ -522,9 +521,9 @@ impl<'tcx> LocalCrateContext<'tcx> {
     /// This is used in the `LocalCrateContext` constructor to allow calling
     /// functions that expect a complete `CrateContext`, even before the local
     /// portion is fully initialized and attached to the `SharedCrateContext`.
-    fn dummy_ccx<'a>(shared: &'a SharedCrateContext<'a, 'tcx>,
-                     local_ccxs: &'a [LocalCrateContext<'tcx>])
-                     -> CrateContext<'a, 'tcx> {
+    fn dummy_ccx(shared: &'a SharedCrateContext<'a, 'tcx>,
+                 local_ccxs: &'a [LocalCrateContext<'a, 'tcx>])
+                 -> CrateContext<'a, 'tcx> {
         assert!(local_ccxs.len() == 1);
         CrateContext {
             shared: shared,
@@ -542,7 +541,7 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         self.shared
     }
 
-    fn local(&self) -> &'b LocalCrateContext<'tcx> {
+    fn local(&self) -> &'b LocalCrateContext<'b, 'tcx> {
         self.local_ccx
     }
 
@@ -709,8 +708,8 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         self.shared.use_dll_storage_attrs()
     }
 
-    pub fn symbol_map(&self) -> &SymbolMap<'tcx> {
-        &*self.local().symbol_map
+    pub fn symbol_cache(&self) -> &'b SymbolCache<'b, 'tcx> {
+        self.local().symbol_cache
     }
 
     /// Given the def-id of some item that has no type parameters, make
@@ -856,7 +855,7 @@ impl<'a, 'tcx> LayoutTyper<'tcx> for &'a CrateContext<'a, 'tcx> {
     }
 }
 
-pub struct TypeOfDepthLock<'a, 'tcx: 'a>(&'a LocalCrateContext<'tcx>);
+pub struct TypeOfDepthLock<'a, 'tcx: 'a>(&'a LocalCrateContext<'a, 'tcx>);
 
 impl<'a, 'tcx> Drop for TypeOfDepthLock<'a, 'tcx> {
     fn drop(&mut self) {

src/librustc_trans/lib.rs

@@ -124,6 +124,7 @@ mod meth;
 mod mir;
 mod monomorphize;
 mod partitioning;
+mod symbol_cache;
 mod symbol_map;
 mod symbol_names_test;
 mod trans_item;

src/librustc_trans/partitioning.rs

@@ -116,7 +116,7 @@ use rustc_incremental::IchHasher;
 use std::cmp::Ordering;
 use std::hash::Hash;
 use std::sync::Arc;
-use symbol_map::SymbolMap;
+use symbol_cache::SymbolCache;
 use syntax::ast::NodeId;
 use syntax::symbol::{Symbol, InternedString};
 use trans_item::{TransItem, InstantiationMode};
@@ -174,14 +174,15 @@ impl<'tcx> CodegenUnit<'tcx> {
         DepNode::WorkProduct(self.work_product_id())
     }
 
-    pub fn compute_symbol_name_hash(&self,
-                                    scx: &SharedCrateContext,
-                                    symbol_map: &SymbolMap) -> u64 {
+    pub fn compute_symbol_name_hash<'a>(&self,
+                                        scx: &SharedCrateContext<'a, 'tcx>,
+                                        symbol_cache: &SymbolCache<'a, 'tcx>)
+                                    -> u64 {
         let mut state = IchHasher::new();
         let exported_symbols = scx.exported_symbols();
-        let all_items = self.items_in_deterministic_order(scx.tcx(), symbol_map);
+        let all_items = self.items_in_deterministic_order(scx.tcx(), symbol_cache);
         for (item, _) in all_items {
-            let symbol_name = symbol_map.get(item).unwrap();
+            let symbol_name = symbol_cache.get(item);
             symbol_name.len().hash(&mut state);
             symbol_name.hash(&mut state);
             let exported = match item {
@@ -201,10 +202,10 @@ impl<'tcx> CodegenUnit<'tcx> {
         state.finish().to_smaller_hash()
     }
 
-    pub fn items_in_deterministic_order(&self,
-                                        tcx: TyCtxt,
-                                        symbol_map: &SymbolMap)
-                                        -> Vec<(TransItem<'tcx>, llvm::Linkage)> {
+    pub fn items_in_deterministic_order<'a>(&self,
+                                            tcx: TyCtxt,
+                                            symbol_cache: &SymbolCache<'a, 'tcx>)
+                                            -> Vec<(TransItem<'tcx>, llvm::Linkage)> {
         let mut items: Vec<(TransItem<'tcx>, llvm::Linkage)> =
             self.items.iter().map(|(item, linkage)| (*item, *linkage)).collect();
 
@@ -216,9 +217,9 @@ impl<'tcx> CodegenUnit<'tcx> {
 
             match (node_id1, node_id2) {
                 (None, None) => {
-                    let symbol_name1 = symbol_map.get(trans_item1).unwrap();
-                    let symbol_name2 = symbol_map.get(trans_item2).unwrap();
-                    symbol_name1.cmp(symbol_name2)
+                    let symbol_name1 = symbol_cache.get(trans_item1);
+                    let symbol_name2 = symbol_cache.get(trans_item2);
+                    symbol_name1.cmp(&symbol_name2)
                 }
                 // In the following two cases we can avoid looking up the symbol
                 (None, Some(_)) => Ordering::Less,
@@ -230,9 +231,9 @@ impl<'tcx> CodegenUnit<'tcx> {
                         return ordering;
                     }
 
-                    let symbol_name1 = symbol_map.get(trans_item1).unwrap();
-                    let symbol_name2 = symbol_map.get(trans_item2).unwrap();
-                    symbol_name1.cmp(symbol_name2)
+                    let symbol_name1 = symbol_cache.get(trans_item1);
+                    let symbol_name2 = symbol_cache.get(trans_item2);
+                    symbol_name1.cmp(&symbol_name2)
                 }
             }
         });
@@ -536,14 +537,12 @@ fn debug_dump<'a, 'b, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>,
 {
     if cfg!(debug_assertions) {
         debug!("{}", label);
+        let symbol_cache = SymbolCache::new(scx);
         for cgu in cgus {
-            let symbol_map = SymbolMap::build(scx, cgu.items
-                                                      .iter()
-                                                      .map(|(&trans_item, _)| trans_item));
             debug!("CodegenUnit {}:", cgu.name);
 
             for (trans_item, linkage) in &cgu.items {
-                let symbol_name = symbol_map.get_or_compute(scx, *trans_item);
+                let symbol_name = symbol_cache.get(*trans_item);
                 let symbol_hash_start = symbol_name.rfind('h');
                 let symbol_hash = symbol_hash_start.map(|i| &symbol_name[i ..])
                                                    .unwrap_or("<no hash>");

src/librustc_trans/symbol_cache.rs

@@ -0,0 +1,42 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use context::SharedCrateContext;
+use std::cell::RefCell;
+use std::rc::Rc;
+use trans_item::TransItem;
+use util::nodemap::FxHashMap;
+
+// In the SymbolCache we collect the symbol names of translation items
+// and cache them for later reference. This is just a performance
+// optimization and the cache is populated lazilly; symbol names of
+// translation items are deterministic and fully defined by the item.
+// Thus they can always be recomputed if needed.
+
+pub struct SymbolCache<'a, 'tcx: 'a> {
+    scx: &'a SharedCrateContext<'a, 'tcx>,
+    index: RefCell<FxHashMap<TransItem<'tcx>, Rc<String>>>,
+}
+
+impl<'a, 'tcx> SymbolCache<'a, 'tcx> {
+    pub fn new(scx: &'a SharedCrateContext<'a, 'tcx>) -> Self {
+        SymbolCache {
+            scx,
+            index: RefCell::new(FxHashMap())
+        }
+    }
+
+    pub fn get(&self, trans_item: TransItem<'tcx>) -> Rc<String> {
+        let mut index = self.index.borrow_mut();
+        index.entry(trans_item)
+             .or_insert_with(|| Rc::new(trans_item.compute_symbol_name(self.scx)))
+             .clone()
+    }
+}

src/librustc_trans/trans_item.rs

@@ -118,8 +118,7 @@ impl<'a, 'tcx> TransItem<'tcx> {
                self.to_raw_string(),
                ccx.codegen_unit().name());
 
-        let symbol_name = ccx.symbol_map()
-                             .get_or_compute(ccx.shared(), *self);
+        let symbol_name = ccx.symbol_cache().get(*self);
 
         debug!("symbol {}", &symbol_name);