Skip to content

Commit

Permalink
Rollup merge of rust-lang#50532 - michaelwoerister:lockless-cnum-map,…
Browse files Browse the repository at this point in the history
… r=Zoxc

Don't use Lock for heavily accessed CrateMetadata::cnum_map.

The `cnum_map` in `CrateMetadata` is used for two things:
1. to map `CrateNums` between crates (used a lot during decoding)
2. to construct the (reverse) post order of the crate graph

For the second case, we need to modify the map after the fact, which is why the map is wrapped in a `Lock`. This is bad for the first case, which does not need the modification and does lots of small reads from the map.

This PR splits case (2) out into a separate `dependencies` field. This allows to make the `cnum_map` immutable (and shifts the interior mutability to a less busy data structure).

Fixes rust-lang#50502

r? @Zoxc
  • Loading branch information
alexcrichton committed May 10, 2018
2 parents 072bfa6 + ea49428 commit 8982c67
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
9 changes: 6 additions & 3 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ impl<'a> CrateLoader<'a> {

let cnum_map = self.resolve_crate_deps(root, &crate_root, &metadata, cnum, span, dep_kind);

let dependencies: Vec<CrateNum> = cnum_map.iter().cloned().collect();

let def_path_table = record_time(&self.sess.perf_stats.decode_def_path_tables_time, || {
crate_root.def_path_table.decode((&metadata, self.sess))
});
Expand All @@ -239,8 +241,9 @@ impl<'a> CrateLoader<'a> {
}),
root: crate_root,
blob: metadata,
cnum_map: Lock::new(cnum_map),
cnum_map,
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),
Expand Down Expand Up @@ -392,7 +395,7 @@ impl<'a> CrateLoader<'a> {

// Propagate the extern crate info to dependencies.
extern_crate.direct = false;
for &dep_cnum in cmeta.cnum_map.borrow().iter() {
for &dep_cnum in cmeta.dependencies.borrow().iter() {
self.update_extern_crate(dep_cnum, extern_crate, visited);
}
}
Expand Down Expand Up @@ -1040,7 +1043,7 @@ impl<'a> CrateLoader<'a> {
}

info!("injecting a dep from {} to {}", cnum, krate);
data.cnum_map.borrow_mut().push(krate);
data.dependencies.borrow_mut().push(krate);
});
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_metadata/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ pub struct CrateMetadata {
pub extern_crate: Lock<Option<ExternCrate>>,

pub blob: MetadataBlob,
pub cnum_map: Lock<CrateNumMap>,
pub cnum_map: CrateNumMap,
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]>,

Expand Down Expand Up @@ -144,7 +145,7 @@ impl CStore {
}

let data = self.get_crate_data(krate);
for &dep in data.cnum_map.borrow().iter() {
for &dep in data.dependencies.borrow().iter() {
if dep != krate {
self.push_dependencies_in_postorder(ordering, dep);
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl<'a, 'tcx: 'a> TyDecoder<'a, 'tcx> for DecodeContext<'a, 'tcx> {
if cnum == LOCAL_CRATE {
self.cdata().cnum
} else {
self.cdata().cnum_map.borrow()[cnum]
self.cdata().cnum_map[cnum]
}
}
}
Expand Down Expand Up @@ -932,7 +932,7 @@ impl<'a, 'tcx> CrateMetadata {
// Translate a DefId from the current compilation environment to a DefId
// for an external crate.
fn reverse_translate_def_id(&self, did: DefId) -> Option<DefId> {
for (local, &global) in self.cnum_map.borrow().iter_enumerated() {
for (local, &global) in self.cnum_map.iter_enumerated() {
if global == did.krate {
return Some(DefId {
krate: local,
Expand Down Expand Up @@ -1007,7 +1007,7 @@ impl<'a, 'tcx> CrateMetadata {
.enumerate()
.flat_map(|(i, link)| {
let cnum = CrateNum::new(i + 1);
link.map(|link| (self.cnum_map.borrow()[cnum], link))
link.map(|link| (self.cnum_map[cnum], link))
})
.collect()
}
Expand Down

0 comments on commit 8982c67

Please sign in to comment.