Auto merge of #50528 - whitfin:issue-50508, r=michaelwoerister

Remove attribute_cache from CrateMetadata

This PR will fix #50508 by removing the `attribute_cache` from the `CrateMetadata` struct. Seeing as performance was referenced in the original issue, I also cleaned up a `self.entry(node_id);` call which might have occasionally happened redundantly.

r? @michaelwoerister
This commit is contained in:
bors 2018-05-23 09:50:54 +00:00
commit c3733a770e
8 changed files with 86 additions and 153 deletions

View File

@ -453,16 +453,20 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
out
}
fn dep_nodes(&self, labels: &Labels, def_id: DefId) -> Vec<DepNode> {
let mut out = Vec::with_capacity(labels.len());
fn dep_nodes<'l>(
&self,
labels: &'l Labels,
def_id: DefId
) -> impl Iterator<Item = DepNode> + 'l {
let def_path_hash = self.tcx.def_path_hash(def_id);
for label in labels.iter() {
match DepNode::from_label_string(label, def_path_hash) {
Ok(dep_node) => out.push(dep_node),
Err(()) => unreachable!(),
}
}
out
labels
.iter()
.map(move |label| {
match DepNode::from_label_string(label, def_path_hash) {
Ok(dep_node) => dep_node,
Err(()) => unreachable!(),
}
})
}
fn dep_node_str(&self, dep_node: &DepNode) -> String {

View File

@ -28,7 +28,6 @@ pub fn copy_cgu_workproducts_to_incr_comp_cache_dir(
if sess.opts.incremental.is_none() {
return None
}
let work_product_id = WorkProductId::from_cgu_name(cgu_name);
let saved_files: Option<Vec<_>> =
files.iter()
@ -63,6 +62,7 @@ pub fn copy_cgu_workproducts_to_incr_comp_cache_dir(
saved_files,
};
let work_product_id = WorkProductId::from_cgu_name(cgu_name);
Some((work_product_id, work_product))
}

View File

@ -58,9 +58,9 @@ pub struct CrateLoader<'a> {
fn dump_crates(cstore: &CStore) {
info!("resolved crates:");
cstore.iter_crate_data(|_, data| {
info!(" name: {}", data.name());
info!(" name: {}", data.root.name);
info!(" cnum: {}", data.cnum);
info!(" hash: {}", data.hash());
info!(" hash: {}", data.root.hash);
info!(" reqd: {:?}", *data.dep_kind.lock());
let CrateSource { dylib, rlib, rmeta } = data.source.clone();
dylib.map(|dl| info!(" dylib: {}", dl.0.display()));
@ -113,7 +113,7 @@ impl<'a> CrateLoader<'a> {
if data.name != name { return }
match hash {
Some(hash) if *hash == data.hash() => { ret = Some(cnum); return }
Some(hash) if *hash == data.root.hash => { ret = Some(cnum); return }
Some(..) => return,
None => {}
}
@ -172,9 +172,9 @@ impl<'a> CrateLoader<'a> {
// Check for conflicts with any crate loaded so far
self.cstore.iter_crate_data(|_, other| {
if other.name() == root.name && // same crate-name
other.disambiguator() == root.disambiguator && // same crate-disambiguator
other.hash() != root.hash { // but different SVH
if other.root.name == root.name && // same crate-name
other.root.disambiguator == root.disambiguator && // same crate-disambiguator
other.root.hash != root.hash { // but different SVH
span_fatal!(self.sess, span, E0523,
"found two different crates with name `{}` that are \
not distinguished by differing `-C metadata`. This \
@ -214,7 +214,6 @@ impl<'a> CrateLoader<'a> {
let root = if root.is_some() { root } else { &crate_paths };
let Library { dylib, rlib, rmeta, metadata } = lib;
let cnum_map = self.resolve_crate_deps(root, &crate_root, &metadata, cnum, span, dep_kind);
let dependencies: Vec<CrateNum> = cnum_map.iter().cloned().collect();
@ -243,13 +242,12 @@ impl<'a> CrateLoader<'a> {
cnum,
dependencies: Lock::new(dependencies),
codemap_import_info: RwLock::new(vec![]),
attribute_cache: Lock::new([Vec::new(), Vec::new()]),
dep_kind: Lock::new(dep_kind),
source: cstore::CrateSource {
dylib,
rlib,
rmeta,
},
}
};
let cmeta = Lrc::new(cmeta);
@ -345,7 +343,7 @@ impl<'a> CrateLoader<'a> {
if locate_ctxt.triple == &self.sess.opts.target_triple {
let mut result = LoadResult::Loaded(library);
self.cstore.iter_crate_data(|cnum, data| {
if data.name() == root.name && root.hash == data.hash() {
if data.root.name == root.name && root.hash == data.root.hash {
assert!(locate_ctxt.hash.is_none());
info!("load success, going to previous cnum: {}", cnum);
result = LoadResult::Previous(cnum);
@ -642,15 +640,14 @@ impl<'a> CrateLoader<'a> {
let mut needs_panic_runtime = attr::contains_name(&krate.attrs,
"needs_panic_runtime");
let sess = self.sess;
self.cstore.iter_crate_data(|cnum, data| {
needs_panic_runtime = needs_panic_runtime ||
data.needs_panic_runtime(sess);
if data.is_panic_runtime(sess) {
data.root.needs_panic_runtime;
if data.root.panic_runtime {
// Inject a dependency from all #![needs_panic_runtime] to this
// #![panic_runtime] crate.
self.inject_dependency_if(cnum, "a panic runtime",
&|data| data.needs_panic_runtime(sess));
&|data| data.root.needs_panic_runtime);
runtime_found = runtime_found || *data.dep_kind.lock() == DepKind::Explicit;
}
});
@ -687,11 +684,11 @@ impl<'a> CrateLoader<'a> {
// Sanity check the loaded crate to ensure it is indeed a panic runtime
// and the panic strategy is indeed what we thought it was.
if !data.is_panic_runtime(self.sess) {
if !data.root.panic_runtime {
self.sess.err(&format!("the crate `{}` is not a panic runtime",
name));
}
if data.panic_strategy() != desired_strategy {
if data.root.panic_strategy != desired_strategy {
self.sess.err(&format!("the crate `{}` does not have the panic \
strategy `{}`",
name, desired_strategy.desc()));
@ -699,7 +696,7 @@ impl<'a> CrateLoader<'a> {
self.sess.injected_panic_runtime.set(Some(cnum));
self.inject_dependency_if(cnum, "a panic runtime",
&|data| data.needs_panic_runtime(self.sess));
&|data| data.root.needs_panic_runtime);
}
fn inject_sanitizer_runtime(&mut self) {
@ -794,7 +791,7 @@ impl<'a> CrateLoader<'a> {
PathKind::Crate, dep_kind);
// Sanity check the loaded crate to ensure it is indeed a sanitizer runtime
if !data.is_sanitizer_runtime(self.sess) {
if !data.root.sanitizer_runtime {
self.sess.err(&format!("the crate `{}` is not a sanitizer runtime",
name));
}
@ -817,7 +814,7 @@ impl<'a> CrateLoader<'a> {
PathKind::Crate, dep_kind);
// Sanity check the loaded crate to ensure it is indeed a profiler runtime
if !data.is_profiler_runtime(self.sess) {
if !data.root.profiler_runtime {
self.sess.err(&format!("the crate `profiler_builtins` is not \
a profiler runtime"));
}
@ -834,7 +831,7 @@ impl<'a> CrateLoader<'a> {
let mut needs_allocator = attr::contains_name(&krate.attrs,
"needs_allocator");
self.cstore.iter_crate_data(|_, data| {
needs_allocator = needs_allocator || data.needs_allocator(self.sess);
needs_allocator = needs_allocator || data.root.needs_allocator;
});
if !needs_allocator {
self.sess.injected_allocator.set(None);
@ -876,7 +873,7 @@ impl<'a> CrateLoader<'a> {
None
};
self.cstore.iter_crate_data(|_, data| {
if !data.has_global_allocator() {
if !data.root.has_global_allocator {
return
}
match global_allocator {
@ -885,14 +882,14 @@ impl<'a> CrateLoader<'a> {
conflicts with this global \
allocator in: {}",
other_crate,
data.name()));
data.root.name));
}
Some(None) => {
self.sess.err(&format!("the #[global_allocator] in this \
crate conflicts with global \
allocator in: {}", data.name()));
allocator in: {}", data.root.name));
}
None => global_allocator = Some(Some(data.name())),
None => global_allocator = Some(Some(data.root.name)),
}
});
if global_allocator.is_some() {
@ -954,7 +951,7 @@ impl<'a> CrateLoader<'a> {
// error.
let mut allocator = None;
self.cstore.iter_crate_data(|_, data| {
if allocator.is_none() && data.has_default_lib_allocator() {
if allocator.is_none() && data.root.has_default_lib_allocator {
allocator = Some(data.clone());
}
});
@ -1030,9 +1027,9 @@ impl<'a> CrateLoader<'a> {
self.sess.err(&format!("the crate `{}` cannot depend \
on a crate that needs {}, but \
it depends on `{}`",
self.cstore.get_crate_data(krate).name(),
self.cstore.get_crate_data(krate).root.name,
what,
data.name()));
data.root.name));
}
}

View File

@ -13,18 +13,14 @@
use schema;
use rustc::hir::def_id::{CRATE_DEF_INDEX, CrateNum, DefIndex};
use rustc::hir::def_id::{CrateNum, DefIndex};
use rustc::hir::map::definitions::DefPathTable;
use rustc::hir::svh::Svh;
use rustc::middle::cstore::{DepKind, ExternCrate, MetadataLoader};
use rustc::session::{Session, CrateDisambiguator};
use rustc_target::spec::PanicStrategy;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc::util::nodemap::{FxHashMap, NodeMap};
use rustc_data_structures::sync::{Lrc, RwLock, Lock};
use syntax::{ast, attr};
use syntax::edition::Edition;
use syntax::ast;
use syntax::ext::base::SyntaxExtension;
use syntax::symbol::Symbol;
use syntax_pos;
@ -69,7 +65,6 @@ pub struct CrateMetadata {
pub cnum: CrateNum,
pub dependencies: Lock<Vec<CrateNum>>,
pub codemap_import_info: RwLock<Vec<ImportedFileMap>>,
pub attribute_cache: Lock<[Vec<Option<Lrc<[ast::Attribute]>>>; 2]>,
pub root: schema::CrateRoot,
@ -177,66 +172,3 @@ impl CStore {
self.extern_mod_crate_map.borrow().get(&emod_id).cloned()
}
}
impl CrateMetadata {
pub fn name(&self) -> Symbol {
self.root.name
}
pub fn hash(&self) -> Svh {
self.root.hash
}
pub fn disambiguator(&self) -> CrateDisambiguator {
self.root.disambiguator
}
pub fn needs_allocator(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "needs_allocator")
}
pub fn has_global_allocator(&self) -> bool {
self.root.has_global_allocator.clone()
}
pub fn has_default_lib_allocator(&self) -> bool {
self.root.has_default_lib_allocator.clone()
}
pub fn is_panic_runtime(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "panic_runtime")
}
pub fn needs_panic_runtime(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "needs_panic_runtime")
}
pub fn is_compiler_builtins(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "compiler_builtins")
}
pub fn is_sanitizer_runtime(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "sanitizer_runtime")
}
pub fn is_profiler_runtime(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "profiler_runtime")
}
pub fn is_no_builtins(&self, sess: &Session) -> bool {
let attrs = self.get_item_attrs(CRATE_DEF_INDEX, sess);
attr::contains_name(&attrs, "no_builtins")
}
pub fn panic_strategy(&self) -> PanicStrategy {
self.root.panic_strategy.clone()
}
pub fn edition(&self) -> Edition {
self.root.edition
}
}

View File

@ -170,17 +170,17 @@ provide! { <'tcx> tcx, def_id, other, cdata,
is_mir_available => { cdata.is_item_mir_available(def_id.index) }
dylib_dependency_formats => { Lrc::new(cdata.get_dylib_dependency_formats()) }
is_panic_runtime => { cdata.is_panic_runtime(tcx.sess) }
is_compiler_builtins => { cdata.is_compiler_builtins(tcx.sess) }
has_global_allocator => { cdata.has_global_allocator() }
is_sanitizer_runtime => { cdata.is_sanitizer_runtime(tcx.sess) }
is_profiler_runtime => { cdata.is_profiler_runtime(tcx.sess) }
panic_strategy => { cdata.panic_strategy() }
is_panic_runtime => { cdata.root.panic_runtime }
is_compiler_builtins => { cdata.root.compiler_builtins }
has_global_allocator => { cdata.root.has_global_allocator }
is_sanitizer_runtime => { cdata.root.sanitizer_runtime }
is_profiler_runtime => { cdata.root.profiler_runtime }
panic_strategy => { cdata.root.panic_strategy }
extern_crate => {
let r = Lrc::new(*cdata.extern_crate.lock());
r
}
is_no_builtins => { cdata.is_no_builtins(tcx.sess) }
is_no_builtins => { cdata.root.no_builtins }
impl_defaultness => { cdata.get_impl_defaultness(def_id.index) }
reachable_non_generics => {
let reachable_non_generics = tcx
@ -209,9 +209,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,
DefId { krate: def_id.krate, index }
})
}
crate_disambiguator => { cdata.disambiguator() }
crate_hash => { cdata.hash() }
original_crate_name => { cdata.name() }
crate_disambiguator => { cdata.root.disambiguator }
crate_hash => { cdata.root.hash }
original_crate_name => { cdata.root.name }
extra_filename => { cdata.root.extra_filename.clone() }
@ -457,17 +457,17 @@ impl CrateStore for cstore::CStore {
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator
{
self.get_crate_data(cnum).disambiguator()
self.get_crate_data(cnum).root.disambiguator
}
fn crate_hash_untracked(&self, cnum: CrateNum) -> hir::svh::Svh
{
self.get_crate_data(cnum).hash()
self.get_crate_data(cnum).root.hash
}
fn crate_edition_untracked(&self, cnum: CrateNum) -> Edition
{
self.get_crate_data(cnum).edition()
self.get_crate_data(cnum).root.edition
}
/// Returns the `DefKey` for a given `DefId`. This indicates the
@ -519,7 +519,7 @@ impl CrateStore for cstore::CStore {
} else if data.name == "proc_macro" &&
self.get_crate_data(id.krate).item_name(id.index) == "quote" {
let ext = SyntaxExtension::ProcMacro(Box::new(::proc_macro::__internal::Quoter),
data.edition());
data.root.edition);
return LoadedMacro::ProcMacro(Lrc::new(ext));
}

View File

@ -557,12 +557,14 @@ impl<'a, 'tcx> CrateMetadata {
-> &'tcx ty::AdtDef {
let item = self.entry(item_id);
let did = self.local_def_id(item_id);
let kind = match item.kind {
EntryKind::Enum(_) => ty::AdtKind::Enum,
EntryKind::Struct(_, _) => ty::AdtKind::Struct,
EntryKind::Union(_, _) => ty::AdtKind::Union,
let (kind, repr) = match item.kind {
EntryKind::Enum(repr) => (ty::AdtKind::Enum, repr),
EntryKind::Struct(_, repr) => (ty::AdtKind::Struct, repr),
EntryKind::Union(_, repr) => (ty::AdtKind::Union, repr),
_ => bug!("get_adt_def called on a non-ADT {:?}", did),
};
let variants = if let ty::AdtKind::Enum = kind {
item.children
.decode(self)
@ -573,12 +575,6 @@ impl<'a, 'tcx> CrateMetadata {
} else {
vec![self.get_variant(&item, item_id)]
};
let (kind, repr) = match item.kind {
EntryKind::Enum(repr) => (ty::AdtKind::Enum, repr),
EntryKind::Struct(_, repr) => (ty::AdtKind::Struct, repr),
EntryKind::Union(_, repr) => (ty::AdtKind::Union, repr),
_ => bug!("get_adt_def called on a non-ADT {:?}", did),
};
tcx.alloc_adt_def(did, kind, variants, repr)
}
@ -880,34 +876,22 @@ impl<'a, 'tcx> CrateMetadata {
}
pub fn get_item_attrs(&self, node_id: DefIndex, sess: &Session) -> Lrc<[ast::Attribute]> {
let (node_as, node_index) =
(node_id.address_space().index(), node_id.as_array_index());
if self.is_proc_macro(node_id) {
return Lrc::new([]);
}
if let Some(&Some(ref val)) =
self.attribute_cache.borrow()[node_as].get(node_index) {
return val.clone();
}
// The attributes for a tuple struct are attached to the definition, not the ctor;
// we assume that someone passing in a tuple struct ctor is actually wanting to
// look at the definition
let mut item = self.entry(node_id);
let def_key = self.def_key(node_id);
if def_key.disambiguated_data.data == DefPathData::StructCtor {
item = self.entry(def_key.parent.unwrap());
}
let result: Lrc<[ast::Attribute]> = Lrc::from(self.get_attributes(&item, sess));
let vec_ = &mut self.attribute_cache.borrow_mut()[node_as];
if vec_.len() < node_index + 1 {
vec_.resize(node_index + 1, None);
}
// This can overwrite the result produced by another thread, but the value
// written should be the same
vec_[node_index] = Some(result.clone());
result
let item_id = if def_key.disambiguated_data.data == DefPathData::StructCtor {
def_key.parent.unwrap()
} else {
node_id
};
let item = self.entry(item_id);
Lrc::from(self.get_attributes(&item, sess))
}
pub fn get_struct_field_names(&self, id: DefIndex) -> Vec<ast::Name> {

View File

@ -483,10 +483,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let index = items.write_index(&mut self.opaque.cursor);
let index_bytes = self.position() - i;
let attrs = tcx.hir.krate_attrs();
let link_meta = self.link_meta;
let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro);
let has_default_lib_allocator =
attr::contains_name(tcx.hir.krate_attrs(), "default_lib_allocator");
let has_default_lib_allocator = attr::contains_name(&attrs, "default_lib_allocator");
let has_global_allocator = *tcx.sess.has_global_allocator.get();
let root = self.lazy(&CrateRoot {
@ -510,6 +510,14 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
None
},
compiler_builtins: attr::contains_name(&attrs, "compiler_builtins"),
needs_allocator: attr::contains_name(&attrs, "needs_allocator"),
needs_panic_runtime: attr::contains_name(&attrs, "needs_panic_runtime"),
no_builtins: attr::contains_name(&attrs, "no_builtins"),
panic_runtime: attr::contains_name(&attrs, "panic_runtime"),
profiler_runtime: attr::contains_name(&attrs, "profiler_runtime"),
sanitizer_runtime: attr::contains_name(&attrs, "sanitizer_runtime"),
crate_deps,
dylib_dependency_formats,
lang_items,

View File

@ -210,6 +210,14 @@ pub struct CrateRoot {
pub interpret_alloc_index: LazySeq<u32>,
pub index: LazySeq<index::Index>,
pub compiler_builtins: bool,
pub needs_allocator: bool,
pub needs_panic_runtime: bool,
pub no_builtins: bool,
pub panic_runtime: bool,
pub profiler_runtime: bool,
pub sanitizer_runtime: bool,
}
#[derive(RustcEncodable, RustcDecodable)]