From cbb5a001792618a2f100e3198f1f874cdce6900f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Jun 2018 22:13:52 +0200 Subject: [PATCH] Parallelize and optimize parts of HIR map creation --- src/librustc/hir/map/collector.rs | 15 ++++---- src/librustc/hir/map/hir_id_validator.rs | 45 ++++++++++++++---------- src/librustc/hir/map/mod.rs | 38 +++++++++++--------- src/librustc/session/mod.rs | 3 ++ 4 files changed, 57 insertions(+), 44 deletions(-) diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index 925a5fb85b81b..ae9bb37842990 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -6,6 +6,7 @@ use rustc_data_structures::svh::Svh; use ich::Fingerprint; use middle::cstore::CrateStore; use session::CrateDisambiguator; +use session::Session; use std::iter::repeat; use syntax::ast::{NodeId, CRATE_NODE_ID}; use syntax::source_map::SourceMap; @@ -92,11 +93,11 @@ where } impl<'a, 'hir> NodeCollector<'a, 'hir> { - pub(super) fn root(krate: &'hir Crate, + pub(super) fn root(sess: &'a Session, + krate: &'hir Crate, dep_graph: &'a DepGraph, definitions: &'a definitions::Definitions, - mut hcx: StableHashingContext<'a>, - source_map: &'a SourceMap) + mut hcx: StableHashingContext<'a>) -> NodeCollector<'a, 'hir> { let root_mod_def_path_hash = definitions.def_path_hash(CRATE_DEF_INDEX); @@ -141,8 +142,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { let mut collector = NodeCollector { krate, - source_map, - map: vec![], + source_map: sess.source_map(), + map: repeat(None).take(sess.current_node_id_count()).collect(), parent_node: CRATE_NODE_ID, current_signature_dep_index: root_mod_sig_dep_index, current_full_dep_index: root_mod_full_dep_index, @@ -219,10 +220,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { fn insert_entry(&mut self, id: NodeId, entry: Entry<'hir>) { debug!("hir_map: {:?} => {:?}", id, entry); - let len = self.map.len(); - if id.as_usize() >= len { - self.map.extend(repeat(None).take(id.as_usize() - len + 1)); - } self.map[id.as_usize()] = Some(entry); } diff --git a/src/librustc/hir/map/hir_id_validator.rs b/src/librustc/hir/map/hir_id_validator.rs index a8f5f93ee6cb1..91c8c29144406 100644 --- a/src/librustc/hir/map/hir_id_validator.rs +++ b/src/librustc/hir/map/hir_id_validator.rs @@ -3,19 +3,24 @@ use hir::{self, intravisit, HirId, ItemLocalId}; use syntax::ast::NodeId; use hir::itemlikevisit::ItemLikeVisitor; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::sync::{Lock, ParallelIterator, par_iter}; pub fn check_crate<'hir>(hir_map: &hir::map::Map<'hir>) { - let mut outer_visitor = OuterVisitor { - hir_map, - errors: vec![], - }; - hir_map.dep_graph.assert_ignored(); - hir_map.krate().visit_all_item_likes(&mut outer_visitor); - if !outer_visitor.errors.is_empty() { - let message = outer_visitor - .errors + let errors = Lock::new(Vec::new()); + + par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| { + hir_map.visit_item_likes_in_module(hir_map.local_def_id(*module_id), &mut OuterVisitor { + hir_map, + errors: &errors, + }); + }); + + let errors = errors.into_inner(); + + if !errors.is_empty() { + let message = errors .iter() .fold(String::new(), |s1, s2| s1 + "\n" + s2); bug!("{}", message); @@ -26,12 +31,12 @@ struct HirIdValidator<'a, 'hir: 'a> { hir_map: &'a hir::map::Map<'hir>, owner_def_index: Option, hir_ids_seen: FxHashMap, - errors: Vec, + errors: &'a Lock>, } struct OuterVisitor<'a, 'hir: 'a> { hir_map: &'a hir::map::Map<'hir>, - errors: Vec, + errors: &'a Lock>, } impl<'a, 'hir: 'a> OuterVisitor<'a, 'hir> { @@ -42,7 +47,7 @@ impl<'a, 'hir: 'a> OuterVisitor<'a, 'hir> { hir_map, owner_def_index: None, hir_ids_seen: Default::default(), - errors: Vec::new(), + errors: self.errors, } } } @@ -51,23 +56,25 @@ impl<'a, 'hir: 'a> ItemLikeVisitor<'hir> for OuterVisitor<'a, 'hir> { fn visit_item(&mut self, i: &'hir hir::Item) { let mut inner_visitor = self.new_inner_visitor(self.hir_map); inner_visitor.check(i.id, |this| intravisit::walk_item(this, i)); - self.errors.extend(inner_visitor.errors.drain(..)); } fn visit_trait_item(&mut self, i: &'hir hir::TraitItem) { let mut inner_visitor = self.new_inner_visitor(self.hir_map); inner_visitor.check(i.id, |this| intravisit::walk_trait_item(this, i)); - self.errors.extend(inner_visitor.errors.drain(..)); } fn visit_impl_item(&mut self, i: &'hir hir::ImplItem) { let mut inner_visitor = self.new_inner_visitor(self.hir_map); inner_visitor.check(i.id, |this| intravisit::walk_impl_item(this, i)); - self.errors.extend(inner_visitor.errors.drain(..)); } } impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> { + #[cold] + #[inline(never)] + fn error(&self, f: impl FnOnce() -> String) { + self.errors.lock().push(f()); + } fn check)>(&mut self, node_id: NodeId, @@ -119,7 +126,7 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> { local_id, self.hir_map.node_to_string(node_id))); } - self.errors.push(format!( + self.error(|| format!( "ItemLocalIds not assigned densely in {}. \ Max ItemLocalId = {}, missing IDs = {:?}; seens IDs = {:?}", self.hir_map.def_path(DefId::local(owner_def_index)).to_string_no_crate(), @@ -145,14 +152,14 @@ impl<'a, 'hir: 'a> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { let stable_id = self.hir_map.definitions().node_to_hir_id[node_id]; if stable_id == hir::DUMMY_HIR_ID { - self.errors.push(format!("HirIdValidator: No HirId assigned for NodeId {}: {:?}", + self.error(|| format!("HirIdValidator: No HirId assigned for NodeId {}: {:?}", node_id, self.hir_map.node_to_string(node_id))); return; } if owner != stable_id.owner { - self.errors.push(format!( + self.error(|| format!( "HirIdValidator: The recorded owner of {} is {} instead of {}", self.hir_map.node_to_string(node_id), self.hir_map.def_path(DefId::local(stable_id.owner)).to_string_no_crate(), @@ -161,7 +168,7 @@ impl<'a, 'hir: 'a> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { if let Some(prev) = self.hir_ids_seen.insert(stable_id.local_id, node_id) { if prev != node_id { - self.errors.push(format!( + self.error(|| format!( "HirIdValidator: Same HirId {}/{} assigned for nodes {} and {}", self.hir_map.def_path(DefId::local(stable_id.owner)).to_string_no_crate(), stable_id.local_id.as_usize(), diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 513e18b137371..869baef1f5afc 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -7,10 +7,11 @@ use dep_graph::{DepGraph, DepNode, DepKind, DepNodeIndex}; use hir::def_id::{CRATE_DEF_INDEX, DefId, LocalDefId, DefIndexAddressSpace}; -use middle::cstore::CrateStore; +use middle::cstore::CrateStoreDyn; use rustc_target::spec::abi::Abi; use rustc_data_structures::svh::Svh; +use rustc_data_structures::sync::join; use syntax::ast::{self, Name, NodeId, CRATE_NODE_ID}; use syntax::source_map::Spanned; use syntax::ext::base::MacroKind; @@ -20,6 +21,7 @@ use hir::*; use hir::itemlikevisit::ItemLikeVisitor; use hir::print::Nested; use util::nodemap::FxHashMap; +use util::common::time; use std::io; use std::result::Result::Err; @@ -1045,26 +1047,32 @@ impl Named for TraitItem { fn name(&self) -> Name { self.ident.name } } impl Named for ImplItem { fn name(&self) -> Name { self.ident.name } } pub fn map_crate<'hir>(sess: &::session::Session, - cstore: &dyn CrateStore, - forest: &'hir mut Forest, + cstore: &CrateStoreDyn, + forest: &'hir Forest, definitions: &'hir Definitions) -> Map<'hir> { - let (map, crate_hash) = { + let ((map, crate_hash), hir_to_node_id) = join(|| { let hcx = ::ich::StableHashingContext::new(sess, &forest.krate, definitions, cstore); - let mut collector = NodeCollector::root(&forest.krate, + let mut collector = NodeCollector::root(sess, + &forest.krate, &forest.dep_graph, &definitions, - hcx, - sess.source_map()); + hcx); intravisit::walk_crate(&mut collector, &forest.krate); let crate_disambiguator = sess.local_crate_disambiguator(); let cmdline_args = sess.opts.dep_tracking_hash(); - collector.finalize_and_compute_crate_hash(crate_disambiguator, - cstore, - cmdline_args) - }; + collector.finalize_and_compute_crate_hash( + crate_disambiguator, + cstore, + cmdline_args + ) + }, || { + // Build the reverse mapping of `node_to_hir_id`. + definitions.node_to_hir_id.iter_enumerated() + .map(|(node_id, &hir_id)| (hir_id, node_id)).collect() + }); if log_enabled!(::log::Level::Debug) { // This only makes sense for ordered stores; note the @@ -1078,10 +1086,6 @@ pub fn map_crate<'hir>(sess: &::session::Session, entries, vector_length, (entries as f64 / vector_length as f64) * 100.); } - // Build the reverse mapping of `node_to_hir_id`. - let hir_to_node_id = definitions.node_to_hir_id.iter_enumerated() - .map(|(node_id, &hir_id)| (hir_id, node_id)).collect(); - let map = Map { forest, dep_graph: forest.dep_graph.clone(), @@ -1091,7 +1095,9 @@ pub fn map_crate<'hir>(sess: &::session::Session, definitions, }; - hir_id_validator::check_crate(&map); + time(sess, "validate hir map", || { + hir_id_validator::check_crate(&map); + }); map } diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index ba09480f93f3d..fa0dfc4b38c8c 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -407,6 +407,9 @@ impl Session { pub fn next_node_id(&self) -> NodeId { self.reserve_node_ids(1) } + pub(crate) fn current_node_id_count(&self) -> usize { + self.next_node_id.get().as_u32() as usize + } pub fn diagnostic<'a>(&'a self) -> &'a errors::Handler { &self.parse_sess.span_diagnostic }