From a517343566956fb038b061f31558f088944977c3 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 24 Apr 2017 19:35:47 +0300 Subject: [PATCH] cache symbol names in ty::maps this fixes a performance regression introduced in commit 39a58c38a0b9ac9e822a1732f073abe8ddf65cfb. --- src/librustc/dep_graph/dep_node.rs | 2 + src/librustc/ty/maps.rs | 35 +++++++++++++- src/librustc/ty/mod.rs | 20 ++++++++ src/librustc_driver/driver.rs | 4 +- src/librustc_trans/back/symbol_export.rs | 17 +++---- src/librustc_trans/back/symbol_names.rs | 50 ++++++++++++++++--- src/librustc_trans/base.rs | 8 ++-- src/librustc_trans/callee.rs | 3 +- src/librustc_trans/consts.rs | 9 ++-- src/librustc_trans/context.rs | 50 ++----------------- src/librustc_trans/lib.rs | 2 +- src/librustc_trans/partitioning.rs | 61 +++++++----------------- src/librustc_trans/symbol_cache.rs | 42 ---------------- src/librustc_trans/symbol_map.rs | 4 +- src/librustc_trans/symbol_names_test.rs | 3 +- src/librustc_trans/trans_item.rs | 14 +++--- 16 files changed, 148 insertions(+), 176 deletions(-) delete mode 100644 src/librustc_trans/symbol_cache.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 33133f6834b9a..31e6a3106a438 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -99,6 +99,7 @@ pub enum DepNode { 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 DepNode { 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), diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 1407e57dc2a6a..c80ae87d941ff 100644 --- a/src/librustc/ty/maps.rs +++ b/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("").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, - pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell> + pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell>, + + 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 { @@ -532,6 +557,12 @@ fn mir_shim_dep_node(instance: ty::InstanceDef) -> DepNode { instance.dep_node() } +fn symbol_name_dep_node(instance: ty::Instance) -> DepNode { + // 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 { DepNode::TypeckBodiesKrate } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 9a8c91f5820dc..de207df7d15eb 100644 --- a/src/librustc/ty/mod.rs +++ b/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) + } +} diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 438f482fa55c7..dab2a0758a217 100644 --- a/src/librustc_driver/driver.rs +++ b/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,6 +893,7 @@ 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); @@ -902,6 +901,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, 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); diff --git a/src/librustc_trans/back/symbol_export.rs b/src/librustc_trans/back/symbol_export.rs index 221c52141a832..467bc6cbfc6d2 100644 --- a/src/librustc_trans/back/symbol_export.rs +++ b/src/librustc_trans/back/symbol_export.rs @@ -11,7 +11,6 @@ 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; @@ -56,7 +55,7 @@ impl ExportedSymbols { let name = symbol_for_def_id(scx.tcx(), def_id, symbol_map); 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 +107,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 +116,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 +127,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(); @@ -228,7 +227,5 @@ fn symbol_for_def_id<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let instance = Instance::mono(tcx, def_id); - symbol_map.get(TransItem::Fn(instance)) - .map(str::to_owned) - .unwrap_or_else(|| symbol_name(instance, tcx)) + str::to_owned(&tcx.symbol_name(instance)) } diff --git a/src/librustc_trans/back/symbol_names.rs b/src/librustc_trans/back/symbol_names.rs index a96128fcf2f53..53a1ec2bd6c70 100644 --- a/src/librustc_trans/back/symbol_names.rs +++ b/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 @@ -165,8 +175,25 @@ fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, format!("h{:016x}", hasher.finish()) } -pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>, - tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { +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() +} + +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,6 +311,19 @@ impl SymbolPathBuffer { result } + 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: &str) -> String { // end name-sequence self.push(hash); diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index e8cca7bc74f1a..12d077a550742 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -65,7 +65,6 @@ 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}; @@ -1139,8 +1138,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let cgu_name = String::from(cgu.name()); let cgu_id = cgu.work_product_id(); - let symbol_cache = SymbolCache::new(scx.tcx()); - let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &symbol_cache); + let symbol_name_hash = cgu.compute_symbol_name_hash(scx); // Check whether there is a previous work-product we can // re-use. Not only must the file exist, and the inputs not @@ -1175,11 +1173,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_cache); + let lcx = LocalCrateContext::new(scx, cgu); let module = { let ccx = CrateContext::new(scx, &lcx); let trans_items = ccx.codegen_unit() - .items_in_deterministic_order(ccx.tcx(), &symbol_cache); + .items_in_deterministic_order(ccx.tcx()); for &(trans_item, linkage) in &trans_items { trans_item.predefine(&ccx, linkage); } diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index 78e0a524ef2dc..dc788dc4b4834 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -23,7 +23,6 @@ use monomorphize::{self, Instance}; use rustc::hir::def_id::DefId; use rustc::ty::TypeFoldable; use rustc::ty::subst::Substs; -use trans_item::TransItem; use type_of; /// Translates a reference to a fn/method item, monomorphizing and @@ -50,7 +49,7 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, return llfn; } - let sym = ccx.symbol_cache().get(TransItem::Fn(instance)); + let sym = tcx.symbol_name(instance); debug!("get_fn({:?}: {:?}) => {}", instance, fn_ty, sym); // This is subtle and surprising, but sometimes we have to bitcast diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 3d614cfbcbf3c..6afb340107d66 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - -use back::symbol_names; use llvm; use llvm::{SetUnnamedAddr}; use llvm::{ValueRef, True}; @@ -93,8 +91,7 @@ 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_cache() - .get(TransItem::Static(id)); + let sym = TransItem::Static(id).symbol_name(ccx.tcx()); let defined_in_current_codegen_unit = ccx.codegen_unit() .items() @@ -113,7 +110,7 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef { hir_map::NodeForeignItem(&hir::ForeignItem { ref attrs, span, node: hir::ForeignItemStatic(..), .. }) => { - let sym = symbol_names::symbol_name(instance, ccx.tcx()); + let sym = ccx.tcx().symbol_name(instance); let g = if let Some(name) = attr::first_attr_value_str_by_name(&attrs, "linkage") { // If this is a static with a linkage specified, then we need to handle @@ -173,7 +170,7 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef { g } else { - let sym = symbol_names::symbol_name(instance, ccx.tcx()); + let sym = ccx.tcx().symbol_name(instance); // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow? // FIXME(nagisa): investigate whether it can be changed into define_global diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index bef22cf304dcb..90cda2f5cad3d 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -29,7 +29,6 @@ use rustc::ty::layout::{LayoutTyper, TyLayout}; use session::config::NoDebugInfo; use session::Session; use session::config; -use symbol_cache::SymbolCache; use util::nodemap::{NodeSet, DefIdMap, FxHashMap}; use std::ffi::{CStr, CString}; @@ -37,6 +36,7 @@ use std::cell::{Cell, RefCell}; use std::ptr; use std::iter; use std::str; +use std::marker::PhantomData; use syntax::ast; use syntax::symbol::InternedString; use syntax_pos::DUMMY_SP; @@ -94,7 +94,6 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { llcx: ContextRef, stats: Stats, codegen_unit: CodegenUnit<'tcx>, - needs_unwind_cleanup_cache: RefCell, bool>>, /// Cache instances of monomorphic and polymorphic items instances: RefCell, ValueRef>>, /// Cache generated vtables @@ -125,11 +124,6 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { /// Mapping from static definitions to their DefId's. statics: RefCell>, - impl_method_cache: RefCell>, - - /// Cache of closure wrappers for bare fn's. - closure_bare_wrapper_cache: RefCell>, - /// List of globals for static variables which need to be passed to the /// LLVM function ReplaceAllUsesWith (RAUW) when translation is complete. /// (We have to make sure we don't invalidate any ValueRefs referring @@ -141,15 +135,11 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { used_statics: RefCell>, lltypes: RefCell, Type>>, - llsizingtypes: RefCell, Type>>, type_hashcodes: RefCell, String>>, int_type: Type, opaque_vec_type: Type, str_slice_type: Type, - /// Holds the LLVM values for closure IDs. - closure_vals: RefCell, ValueRef>>, - dbg_cx: Option>, eh_personality: Cell>, @@ -164,7 +154,8 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { /// A counter that is used for generating local symbol names local_gen_sym_counter: Cell, - symbol_cache: &'a SymbolCache<'a, 'tcx>, + /// A placeholder so we can add lifetimes + placeholder: PhantomData<&'a ()>, } /// A CrateContext value binds together one LocalCrateContext with the @@ -366,8 +357,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { pub fn new(shared: &SharedCrateContext<'a, 'tcx>, - codegen_unit: CodegenUnit<'tcx>, - symbol_cache: &'a SymbolCache<'a, 'tcx>) + codegen_unit: CodegenUnit<'tcx>) -> LocalCrateContext<'a, 'tcx> { unsafe { // Append ".rs" to LLVM module identifier. @@ -396,7 +386,6 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { llcx: llcx, stats: Stats::default(), codegen_unit: codegen_unit, - needs_unwind_cleanup_cache: RefCell::new(FxHashMap()), instances: RefCell::new(FxHashMap()), vtables: RefCell::new(FxHashMap()), const_cstr_cache: RefCell::new(FxHashMap()), @@ -405,17 +394,13 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { const_values: RefCell::new(FxHashMap()), extern_const_values: RefCell::new(DefIdMap()), statics: RefCell::new(FxHashMap()), - impl_method_cache: RefCell::new(FxHashMap()), - closure_bare_wrapper_cache: RefCell::new(FxHashMap()), statics_to_rauw: RefCell::new(Vec::new()), used_statics: RefCell::new(Vec::new()), lltypes: RefCell::new(FxHashMap()), - llsizingtypes: RefCell::new(FxHashMap()), type_hashcodes: RefCell::new(FxHashMap()), int_type: Type::from_ref(ptr::null_mut()), opaque_vec_type: Type::from_ref(ptr::null_mut()), str_slice_type: Type::from_ref(ptr::null_mut()), - closure_vals: RefCell::new(FxHashMap()), dbg_cx: dbg_cx, eh_personality: Cell::new(None), eh_unwind_resume: Cell::new(None), @@ -423,7 +408,7 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { intrinsics: RefCell::new(FxHashMap()), type_of_depth: Cell::new(0), local_gen_sym_counter: Cell::new(0), - symbol_cache: symbol_cache, + placeholder: PhantomData, }; let (int_type, opaque_vec_type, str_slice_ty, mut local_ccx) = { @@ -515,10 +500,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { unsafe { llvm::LLVMRustGetModuleDataLayout(self.llmod()) } } - pub fn needs_unwind_cleanup_cache(&self) -> &RefCell, bool>> { - &self.local().needs_unwind_cleanup_cache - } - pub fn instances<'a>(&'a self) -> &'a RefCell, ValueRef>> { &self.local().instances } @@ -554,15 +535,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.local().statics } - pub fn impl_method_cache<'a>(&'a self) - -> &'a RefCell> { - &self.local().impl_method_cache - } - - pub fn closure_bare_wrapper_cache<'a>(&'a self) -> &'a RefCell> { - &self.local().closure_bare_wrapper_cache - } - pub fn statics_to_rauw<'a>(&'a self) -> &'a RefCell> { &self.local().statics_to_rauw } @@ -575,10 +547,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.local().lltypes } - pub fn llsizingtypes<'a>(&'a self) -> &'a RefCell, Type>> { - &self.local().llsizingtypes - } - pub fn type_hashcodes<'a>(&'a self) -> &'a RefCell, String>> { &self.local().type_hashcodes } @@ -599,10 +567,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.local().str_slice_type } - pub fn closure_vals<'a>(&'a self) -> &'a RefCell, ValueRef>> { - &self.local().closure_vals - } - pub fn dbg_cx<'a>(&'a self) -> &'a Option> { &self.local().dbg_cx } @@ -644,10 +608,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.shared.use_dll_storage_attrs() } - 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 /// a suitable "empty substs" for it. pub fn empty_substs_for_def_id(&self, item_def_id: DefId) -> &'tcx Substs<'tcx> { diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index 117d8568500b8..faddffb65fae2 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -67,6 +67,7 @@ pub use rustc::lint; pub use rustc::util; pub use base::trans_crate; +pub use back::symbol_names::provide; pub mod back { pub use rustc::hir::svh; @@ -124,7 +125,6 @@ mod meth; mod mir; mod monomorphize; mod partitioning; -mod symbol_cache; mod symbol_map; mod symbol_names_test; mod trans_item; diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 6b89d11cfb68f..2c76cdeb48cdf 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -113,10 +113,8 @@ use rustc::session::config::NUMBERED_CODEGEN_UNIT_MARKER; use rustc::ty::{self, TyCtxt}; use rustc::ty::item_path::characteristic_def_id_of_type; use rustc_incremental::IchHasher; -use std::cmp::Ordering; use std::hash::Hash; use std::sync::Arc; -use symbol_cache::SymbolCache; use syntax::ast::NodeId; use syntax::symbol::{Symbol, InternedString}; use trans_item::{TransItem, InstantiationMode}; @@ -175,14 +173,13 @@ impl<'tcx> CodegenUnit<'tcx> { } pub fn compute_symbol_name_hash<'a>(&self, - scx: &SharedCrateContext<'a, 'tcx>, - symbol_cache: &SymbolCache<'a, 'tcx>) + scx: &SharedCrateContext<'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_cache); + let all_items = self.items_in_deterministic_order(scx.tcx()); for (item, _) in all_items { - let symbol_name = symbol_cache.get(item); + let symbol_name = item.symbol_name(scx.tcx()); symbol_name.len().hash(&mut state); symbol_name.hash(&mut state); let exported = match item { @@ -203,53 +200,30 @@ impl<'tcx> CodegenUnit<'tcx> { } pub fn items_in_deterministic_order<'a>(&self, - tcx: TyCtxt, - symbol_cache: &SymbolCache<'a, 'tcx>) + tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Vec<(TransItem<'tcx>, llvm::Linkage)> { - let mut items: Vec<(TransItem<'tcx>, llvm::Linkage)> = - self.items.iter().map(|(item, linkage)| (*item, *linkage)).collect(); - // The codegen tests rely on items being process in the same order as // they appear in the file, so for local items, we sort by node_id first - items.sort_by(|&(trans_item1, _), &(trans_item2, _)| { - let node_id1 = local_node_id(tcx, trans_item1); - let node_id2 = local_node_id(tcx, trans_item2); - - match (node_id1, node_id2) { - (None, None) => { - 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, - (Some(_), None) => Ordering::Greater, - (Some(node_id1), Some(node_id2)) => { - let ordering = node_id1.cmp(&node_id2); - - if ordering != Ordering::Equal { - return ordering; - } - - let symbol_name1 = symbol_cache.get(trans_item1); - let symbol_name2 = symbol_cache.get(trans_item2); - symbol_name1.cmp(&symbol_name2) - } - } - }); - - return items; + #[derive(PartialEq, Eq, PartialOrd, Ord)] + pub struct ItemSortKey(Option, ty::SymbolName); - fn local_node_id(tcx: TyCtxt, trans_item: TransItem) -> Option { - match trans_item { + fn item_sort_key<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + item: TransItem<'tcx>) -> ItemSortKey { + ItemSortKey(match item { TransItem::Fn(instance) => { tcx.hir.as_local_node_id(instance.def_id()) } TransItem::Static(node_id) | TransItem::GlobalAsm(node_id) => { Some(node_id) } - } + }, item.symbol_name(tcx)) } + + let items: Vec<_> = self.items.iter().map(|(&i, &l)| (i, l)).collect(); + let mut items : Vec<_> = items.iter() + .map(|il| (il, item_sort_key(tcx, il.0))).collect(); + items.sort_by(|&(_, ref key1), &(_, ref key2)| key1.cmp(key2)); + items.into_iter().map(|(&item_linkage, _)| item_linkage).collect() } } @@ -537,12 +511,11 @@ fn debug_dump<'a, 'b, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, { if cfg!(debug_assertions) { debug!("{}", label); - let symbol_cache = SymbolCache::new(tcx); for cgu in cgus { debug!("CodegenUnit {}:", cgu.name); for (trans_item, linkage) in &cgu.items { - let symbol_name = symbol_cache.get(*trans_item); + let symbol_name = trans_item.symbol_name(tcx); let symbol_hash_start = symbol_name.rfind('h'); let symbol_hash = symbol_hash_start.map(|i| &symbol_name[i ..]) .unwrap_or(""); diff --git a/src/librustc_trans/symbol_cache.rs b/src/librustc_trans/symbol_cache.rs deleted file mode 100644 index ddc1ef537a55f..0000000000000 --- a/src/librustc_trans/symbol_cache.rs +++ /dev/null @@ -1,42 +0,0 @@ -// 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use rustc::ty::TyCtxt; -use std::cell::RefCell; -use syntax_pos::symbol::{InternedString, Symbol}; -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> { - tcx: TyCtxt<'a, 'tcx, 'tcx>, - index: RefCell, Symbol>>, -} - -impl<'a, 'tcx> SymbolCache<'a, 'tcx> { - pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Self { - SymbolCache { - tcx: tcx, - index: RefCell::new(FxHashMap()) - } - } - - pub fn get(&self, trans_item: TransItem<'tcx>) -> InternedString { - let mut index = self.index.borrow_mut(); - index.entry(trans_item) - .or_insert_with(|| Symbol::intern(&trans_item.compute_symbol_name(self.tcx))) - .as_str() - } -} diff --git a/src/librustc_trans/symbol_map.rs b/src/librustc_trans/symbol_map.rs index 9d3e62888a2df..85a8d501753f2 100644 --- a/src/librustc_trans/symbol_map.rs +++ b/src/librustc_trans/symbol_map.rs @@ -36,7 +36,7 @@ impl<'tcx> SymbolMap<'tcx> { // Check for duplicate symbol names let tcx = scx.tcx(); let mut symbols: Vec<_> = trans_items.map(|trans_item| { - (trans_item, trans_item.compute_symbol_name(tcx)) + (trans_item, trans_item.symbol_name(tcx)) }).collect(); (&mut symbols[..]).sort_by(|&(_, ref sym1), &(_, ref sym2)|{ @@ -125,7 +125,7 @@ impl<'tcx> SymbolMap<'tcx> { if let Some(sym) = self.get(trans_item) { Cow::from(sym) } else { - Cow::from(trans_item.compute_symbol_name(scx.tcx())) + Cow::from(str::to_owned(&trans_item.symbol_name(scx.tcx()))) } } } diff --git a/src/librustc_trans/symbol_names_test.rs b/src/librustc_trans/symbol_names_test.rs index fd817cb94c1c1..d96757be9f3a5 100644 --- a/src/librustc_trans/symbol_names_test.rs +++ b/src/librustc_trans/symbol_names_test.rs @@ -14,7 +14,6 @@ //! item-path. This is used for unit testing the code that generates //! paths etc in all kinds of annoying scenarios. -use back::symbol_names; use rustc::hir; use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap}; use rustc::ty::TyCtxt; @@ -52,7 +51,7 @@ impl<'a, 'tcx> SymbolNamesTest<'a, 'tcx> { if attr.check_name(SYMBOL_NAME) { // for now, can only use on monomorphic names let instance = Instance::mono(tcx, def_id); - let name = symbol_names::symbol_name(instance, self.tcx); + let name = self.tcx.symbol_name(instance); tcx.sess.span_err(attr.span, &format!("symbol-name({})", name)); } else if attr.check_name(ITEM_PATH) { let path = tcx.item_path_str(def_id); diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index d8e139dc505b6..f953db94fffba 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -28,10 +28,10 @@ use rustc::hir; use rustc::hir::def_id::DefId; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::subst::Substs; +use syntax_pos::symbol::Symbol; use syntax::ast::{self, NodeId}; use syntax::attr; use type_of; -use back::symbol_names; use std::fmt::Write; use std::iter; @@ -118,7 +118,7 @@ impl<'a, 'tcx> TransItem<'tcx> { self.to_raw_string(), ccx.codegen_unit().name()); - let symbol_name = ccx.symbol_cache().get(*self); + let symbol_name = self.symbol_name(ccx.tcx()); debug!("symbol {}", &symbol_name); @@ -184,16 +184,18 @@ impl<'a, 'tcx> TransItem<'tcx> { ccx.instances().borrow_mut().insert(instance, lldecl); } - pub fn compute_symbol_name(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { + pub fn symbol_name(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> ty::SymbolName { match *self { - TransItem::Fn(instance) => symbol_names::symbol_name(instance, tcx), + TransItem::Fn(instance) => tcx.symbol_name(instance), TransItem::Static(node_id) => { let def_id = tcx.hir.local_def_id(node_id); - symbol_names::symbol_name(Instance::mono(tcx, def_id), tcx) + tcx.symbol_name(Instance::mono(tcx, def_id)) } TransItem::GlobalAsm(node_id) => { let def_id = tcx.hir.local_def_id(node_id); - format!("global_asm_{:?}", def_id) + ty::SymbolName { + name: Symbol::intern(&format!("global_asm_{:?}", def_id)).as_str() + } } } }