Rollup merge of #57232 - Zoxc:par-collector-misc, r=michaelwoerister

Parallelize and optimize parts of HIR map creation

r? @michaelwoerister

4 files changed, 57 insertions, 44 deletions

diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs
index 925a5fb85b8..ae9bb378429 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 a8f5f93ee6c..91c8c291444 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<DefIndex>,
     hir_ids_seen: FxHashMap<ItemLocalId, NodeId>,
-    errors: Vec<String>,
+    errors: &'a Lock<Vec<String>>,
 }
 
 struct OuterVisitor<'a, 'hir: 'a> {
     hir_map: &'a hir::map::Map<'hir>,
-    errors: Vec<String>,
+    errors: &'a Lock<Vec<String>>,
 }
 
 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<F: FnOnce(&mut HirIdValidator<'a, 'hir>)>(&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 513e18b1373..869baef1f5a 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 ba09480f93f..fa0dfc4b38c 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
     }