Skip to content

Commit a517343

Browse files
committed
cache symbol names in ty::maps
this fixes a performance regression introduced in commit 39a58c3.
1 parent b0a4074 commit a517343

File tree

16 files changed

+148
-176
lines changed

16 files changed

+148
-176
lines changed

src/librustc/dep_graph/dep_node.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub enum DepNode<D: Clone + Debug> {
9999
TypeckTables(D),
100100
UsedTraitImports(D),
101101
ConstEval(D),
102+
SymbolName(D),
102103

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

src/librustc/ty/maps.rs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use std::cell::{RefCell, RefMut};
2424
use std::ops::Deref;
2525
use std::rc::Rc;
2626
use syntax_pos::{Span, DUMMY_SP};
27+
use syntax::symbol::Symbol;
2728

2829
trait Key {
2930
fn map_crate(&self) -> CrateNum;
@@ -40,6 +41,16 @@ impl<'tcx> Key for ty::InstanceDef<'tcx> {
4041
}
4142
}
4243

44+
impl<'tcx> Key for ty::Instance<'tcx> {
45+
fn map_crate(&self) -> CrateNum {
46+
LOCAL_CRATE
47+
}
48+
49+
fn default_span(&self, tcx: TyCtxt) -> Span {
50+
tcx.def_span(self.def_id())
51+
}
52+
}
53+
4354
impl Key for CrateNum {
4455
fn map_crate(&self) -> CrateNum {
4556
*self
@@ -108,13 +119,18 @@ impl<'tcx> Value<'tcx> for Ty<'tcx> {
108119
}
109120
}
110121

111-
112122
impl<'tcx> Value<'tcx> for ty::DtorckConstraint<'tcx> {
113123
fn from_cycle_error<'a>(_: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
114124
Self::empty()
115125
}
116126
}
117127

128+
impl<'tcx> Value<'tcx> for ty::SymbolName {
129+
fn from_cycle_error<'a>(_: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
130+
ty::SymbolName { name: Symbol::intern("<error>").as_str() }
131+
}
132+
}
133+
118134
pub struct CycleError<'a, 'tcx: 'a> {
119135
span: Span,
120136
cycle: RefMut<'a, [(Span, Query<'tcx>)]>,
@@ -242,6 +258,12 @@ impl<'tcx> QueryDescription for queries::const_eval<'tcx> {
242258
}
243259
}
244260

261+
impl<'tcx> QueryDescription for queries::symbol_name<'tcx> {
262+
fn describe(_tcx: TyCtxt, instance: ty::Instance<'tcx>) -> String {
263+
format!("computing the symbol for `{}`", instance)
264+
}
265+
}
266+
245267
macro_rules! define_maps {
246268
(<$tcx:tt>
247269
$($(#[$attr:meta])*
@@ -513,7 +535,10 @@ define_maps! { <'tcx>
513535

514536
pub reachable_set: reachability_dep_node(CrateNum) -> Rc<NodeSet>,
515537

516-
pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>
538+
pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>,
539+
540+
pub def_symbol_name: SymbolName(DefId) -> ty::SymbolName,
541+
pub symbol_name: symbol_name_dep_node(ty::Instance<'tcx>) -> ty::SymbolName
517542
}
518543

519544
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> {
532557
instance.dep_node()
533558
}
534559

560+
fn symbol_name_dep_node(instance: ty::Instance) -> DepNode<DefId> {
561+
// symbol_name uses the substs only to traverse them to find the
562+
// hash, and that does not create any new dep-nodes.
563+
DepNode::SymbolName(instance.def.def_id())
564+
}
565+
535566
fn typeck_item_bodies_dep_node(_: CrateNum) -> DepNode<DefId> {
536567
DepNode::TypeckBodiesKrate
537568
}

src/librustc/ty/mod.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use serialize::{self, Encodable, Encoder};
3838
use std::cell::{Cell, RefCell, Ref};
3939
use std::collections::BTreeMap;
4040
use std::cmp;
41+
use std::fmt;
4142
use std::hash::{Hash, Hasher};
4243
use std::iter::FromIterator;
4344
use std::ops::Deref;
@@ -2745,3 +2746,22 @@ impl<'tcx> DtorckConstraint<'tcx> {
27452746
self.dtorck_types.retain(|&val| dtorck_types.replace(val).is_none());
27462747
}
27472748
}
2749+
2750+
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
2751+
pub struct SymbolName {
2752+
// FIXME: we don't rely on interning or equality here - better have
2753+
// this be a `&'tcx str`.
2754+
pub name: InternedString
2755+
}
2756+
2757+
impl Deref for SymbolName {
2758+
type Target = str;
2759+
2760+
fn deref(&self) -> &str { &self.name }
2761+
}
2762+
2763+
impl fmt::Display for SymbolName {
2764+
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
2765+
fmt::Display::fmt(&self.name, fmt)
2766+
}
2767+
}

src/librustc_driver/driver.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,6 @@ pub fn compile_input(sess: &Session,
225225
sess.code_stats.borrow().print_type_sizes();
226226
}
227227

228-
if ::std::env::var("SKIP_LLVM").is_ok() { ::std::process::exit(0); }
229-
230228
let phase5_result = phase_5_run_llvm_passes(sess, &trans, &outputs);
231229

232230
controller_entry_point!(after_llvm,
@@ -895,13 +893,15 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
895893
mir::provide(&mut local_providers);
896894
reachable::provide(&mut local_providers);
897895
rustc_privacy::provide(&mut local_providers);
896+
trans::provide(&mut local_providers);
898897
typeck::provide(&mut local_providers);
899898
ty::provide(&mut local_providers);
900899
reachable::provide(&mut local_providers);
901900
rustc_const_eval::provide(&mut local_providers);
902901

903902
let mut extern_providers = ty::maps::Providers::default();
904903
cstore::provide(&mut extern_providers);
904+
trans::provide(&mut extern_providers);
905905
ty::provide_extern(&mut extern_providers);
906906
// FIXME(eddyb) get rid of this once we replace const_eval with miri.
907907
rustc_const_eval::provide(&mut extern_providers);

src/librustc_trans/back/symbol_export.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use context::SharedCrateContext;
1212
use monomorphize::Instance;
1313
use symbol_map::SymbolMap;
14-
use back::symbol_names::symbol_name;
1514
use util::nodemap::FxHashMap;
1615
use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE};
1716
use rustc::session::config;
@@ -56,7 +55,7 @@ impl ExportedSymbols {
5655
let name = symbol_for_def_id(scx.tcx(), def_id, symbol_map);
5756
let export_level = export_level(scx, def_id);
5857
debug!("EXPORTED SYMBOL (local): {} ({:?})", name, export_level);
59-
(name, export_level)
58+
(str::to_owned(&name), export_level)
6059
})
6160
.collect();
6261

@@ -108,7 +107,7 @@ impl ExportedSymbols {
108107
.exported_symbols(cnum)
109108
.iter()
110109
.map(|&def_id| {
111-
let name = symbol_name(Instance::mono(scx.tcx(), def_id), scx.tcx());
110+
let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id));
112111
let export_level = if special_runtime_crate {
113112
// We can probably do better here by just ensuring that
114113
// it has hidden visibility rather than public
@@ -117,9 +116,9 @@ impl ExportedSymbols {
117116
//
118117
// In general though we won't link right if these
119118
// symbols are stripped, and LTO currently strips them.
120-
if name == "rust_eh_personality" ||
121-
name == "rust_eh_register_frames" ||
122-
name == "rust_eh_unregister_frames" {
119+
if &*name == "rust_eh_personality" ||
120+
&*name == "rust_eh_register_frames" ||
121+
&*name == "rust_eh_unregister_frames" {
123122
SymbolExportLevel::C
124123
} else {
125124
SymbolExportLevel::Rust
@@ -128,7 +127,7 @@ impl ExportedSymbols {
128127
export_level(scx, def_id)
129128
};
130129
debug!("EXPORTED SYMBOL (re-export): {} ({:?})", name, export_level);
131-
(name, export_level)
130+
(str::to_owned(&name), export_level)
132131
})
133132
.collect();
134133

@@ -228,7 +227,5 @@ fn symbol_for_def_id<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
228227

229228
let instance = Instance::mono(tcx, def_id);
230229

231-
symbol_map.get(TransItem::Fn(instance))
232-
.map(str::to_owned)
233-
.unwrap_or_else(|| symbol_name(instance, tcx))
230+
str::to_owned(&tcx.symbol_name(instance))
234231
}

src/librustc_trans/back/symbol_names.rs

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,24 @@ use rustc::hir::map as hir_map;
105105
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
106106
use rustc::ty::fold::TypeVisitor;
107107
use rustc::ty::item_path::{self, ItemPathBuffer, RootMode};
108+
use rustc::ty::maps::Providers;
108109
use rustc::ty::subst::Substs;
109110
use rustc::hir::map::definitions::DefPathData;
110111
use rustc::util::common::record_time;
111112

112113
use syntax::attr;
114+
use syntax_pos::symbol::Symbol;
113115

114116
use std::fmt::Write;
115117

118+
pub fn provide(providers: &mut Providers) {
119+
*providers = Providers {
120+
def_symbol_name,
121+
symbol_name,
122+
..*providers
123+
};
124+
}
125+
116126
fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
117127

118128
// the DefId of the item this name is for
@@ -165,8 +175,25 @@ fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
165175
format!("h{:016x}", hasher.finish())
166176
}
167177

168-
pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>,
169-
tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
178+
fn def_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
179+
-> ty::SymbolName
180+
{
181+
let mut buffer = SymbolPathBuffer::new();
182+
item_path::with_forced_absolute_paths(|| {
183+
tcx.push_item_path(&mut buffer, def_id);
184+
});
185+
buffer.into_interned()
186+
}
187+
188+
fn symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>)
189+
-> ty::SymbolName
190+
{
191+
ty::SymbolName { name: Symbol::intern(&compute_symbol_name(tcx, instance)).as_str() }
192+
}
193+
194+
fn compute_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>)
195+
-> String
196+
{
170197
let def_id = instance.def_id();
171198
let substs = instance.substs;
172199

@@ -253,11 +280,7 @@ pub fn symbol_name<'a, 'tcx>(instance: Instance<'tcx>,
253280

254281
let hash = get_symbol_hash(tcx, Some(def_id), instance_ty, Some(substs));
255282

256-
let mut buffer = SymbolPathBuffer::new();
257-
item_path::with_forced_absolute_paths(|| {
258-
tcx.push_item_path(&mut buffer, def_id);
259-
});
260-
buffer.finish(&hash)
283+
SymbolPathBuffer::from_interned(tcx.def_symbol_name(def_id)).finish(&hash)
261284
}
262285

263286
// Follow C++ namespace-mangling style, see
@@ -288,6 +311,19 @@ impl SymbolPathBuffer {
288311
result
289312
}
290313

314+
fn from_interned(symbol: ty::SymbolName) -> Self {
315+
let mut result = SymbolPathBuffer {
316+
result: String::with_capacity(64),
317+
temp_buf: String::with_capacity(16)
318+
};
319+
result.result.push_str(&symbol.name);
320+
result
321+
}
322+
323+
fn into_interned(self) -> ty::SymbolName {
324+
ty::SymbolName { name: Symbol::intern(&self.result).as_str() }
325+
}
326+
291327
fn finish(mut self, hash: &str) -> String {
292328
// end name-sequence
293329
self.push(hash);

src/librustc_trans/base.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ use meth;
6565
use mir;
6666
use monomorphize::{self, Instance};
6767
use partitioning::{self, PartitioningStrategy, CodegenUnit};
68-
use symbol_cache::SymbolCache;
6968
use symbol_map::SymbolMap;
7069
use symbol_names_test;
7170
use trans_item::{TransItem, DefPathBasedNames};
@@ -1139,8 +1138,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
11391138

11401139
let cgu_name = String::from(cgu.name());
11411140
let cgu_id = cgu.work_product_id();
1142-
let symbol_cache = SymbolCache::new(scx.tcx());
1143-
let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &symbol_cache);
1141+
let symbol_name_hash = cgu.compute_symbol_name_hash(scx);
11441142

11451143
// Check whether there is a previous work-product we can
11461144
// 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>,
11751173
}
11761174

11771175
// Instantiate translation items without filling out definitions yet...
1178-
let lcx = LocalCrateContext::new(scx, cgu, &symbol_cache);
1176+
let lcx = LocalCrateContext::new(scx, cgu);
11791177
let module = {
11801178
let ccx = CrateContext::new(scx, &lcx);
11811179
let trans_items = ccx.codegen_unit()
1182-
.items_in_deterministic_order(ccx.tcx(), &symbol_cache);
1180+
.items_in_deterministic_order(ccx.tcx());
11831181
for &(trans_item, linkage) in &trans_items {
11841182
trans_item.predefine(&ccx, linkage);
11851183
}

src/librustc_trans/callee.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use monomorphize::{self, Instance};
2323
use rustc::hir::def_id::DefId;
2424
use rustc::ty::TypeFoldable;
2525
use rustc::ty::subst::Substs;
26-
use trans_item::TransItem;
2726
use type_of;
2827

2928
/// Translates a reference to a fn/method item, monomorphizing and
@@ -50,7 +49,7 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
5049
return llfn;
5150
}
5251

53-
let sym = ccx.symbol_cache().get(TransItem::Fn(instance));
52+
let sym = tcx.symbol_name(instance);
5453
debug!("get_fn({:?}: {:?}) => {}", instance, fn_ty, sym);
5554

5655
// This is subtle and surprising, but sometimes we have to bitcast

src/librustc_trans/consts.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
12-
use back::symbol_names;
1311
use llvm;
1412
use llvm::{SetUnnamedAddr};
1513
use llvm::{ValueRef, True};
@@ -93,8 +91,7 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef {
9391
hir_map::NodeItem(&hir::Item {
9492
ref attrs, span, node: hir::ItemStatic(..), ..
9593
}) => {
96-
let sym = ccx.symbol_cache()
97-
.get(TransItem::Static(id));
94+
let sym = TransItem::Static(id).symbol_name(ccx.tcx());
9895

9996
let defined_in_current_codegen_unit = ccx.codegen_unit()
10097
.items()
@@ -113,7 +110,7 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef {
113110
hir_map::NodeForeignItem(&hir::ForeignItem {
114111
ref attrs, span, node: hir::ForeignItemStatic(..), ..
115112
}) => {
116-
let sym = symbol_names::symbol_name(instance, ccx.tcx());
113+
let sym = ccx.tcx().symbol_name(instance);
117114
let g = if let Some(name) =
118115
attr::first_attr_value_str_by_name(&attrs, "linkage") {
119116
// 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 {
173170

174171
g
175172
} else {
176-
let sym = symbol_names::symbol_name(instance, ccx.tcx());
173+
let sym = ccx.tcx().symbol_name(instance);
177174

178175
// FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow?
179176
// FIXME(nagisa): investigate whether it can be changed into define_global

0 commit comments

Comments
 (0)