From f3ce14479c5297521edfe70c8e4ef7c6d1a83536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 31 Dec 2019 14:01:38 +0100 Subject: [PATCH 1/4] Run codegen unit partitioning and assert_symbols_are_distinct in parallel --- src/librustc_mir/monomorphize/partitioning.rs | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/monomorphize/partitioning.rs b/src/librustc_mir/monomorphize/partitioning.rs index 8fa41cab190..0def51a6a33 100644 --- a/src/librustc_mir/monomorphize/partitioning.rs +++ b/src/librustc_mir/monomorphize/partitioning.rs @@ -104,6 +104,7 @@ use rustc::ty::print::characteristic_def_id_of_type; use rustc::ty::query::Providers; use rustc::ty::{self, DefIdTree, InstanceDef, TyCtxt}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::sync; use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdSet, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_span::symbol::Symbol; @@ -796,6 +797,8 @@ where I: Iterator>, 'tcx: 'a, { + let _prof_timer = tcx.prof.generic_activity("assert_symbols_are_distinct"); + let mut symbols: Vec<_> = mono_items.map(|mono_item| (mono_item, mono_item.symbol_name(tcx))).collect(); @@ -869,18 +872,23 @@ fn collect_and_partition_mono_items( tcx.sess.abort_if_errors(); - assert_symbols_are_distinct(tcx, items.iter()); + let (codegen_units, _) = tcx.sess.time("partition_and_assert_distinct_symbols", || { + sync::join( + || { + let strategy = if tcx.sess.opts.incremental.is_some() { + PartitioningStrategy::PerModule + } else { + PartitioningStrategy::FixedUnitCount(tcx.sess.codegen_units()) + }; - let strategy = if tcx.sess.opts.incremental.is_some() { - PartitioningStrategy::PerModule - } else { - PartitioningStrategy::FixedUnitCount(tcx.sess.codegen_units()) - }; - - let codegen_units = partition(tcx, items.iter().cloned(), strategy, &inlining_map) - .into_iter() - .map(Arc::new) - .collect::>(); + partition(tcx, items.iter().cloned(), strategy, &inlining_map) + .into_iter() + .map(Arc::new) + .collect::>() + }, + || assert_symbols_are_distinct(tcx, items.iter()), + ) + }); let mono_items: DefIdSet = items .iter() From 4beeadda3c137c8c8d66ba6b1bb3fb0be9b37b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 31 Dec 2019 14:27:20 +0100 Subject: [PATCH 2/4] Fix a deadlock --- src/librustc_mir/monomorphize/collector.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 41fbfd22e50..422a1ccd9b5 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -194,7 +194,7 @@ use rustc_hir as hir; use rustc_hir::def_id::{DefId, DefIdMap, LOCAL_CRATE}; use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_index::bit_set::GrowableBitSet; - +use smallvec::SmallVec; use std::iter; #[derive(PartialEq)] @@ -227,9 +227,7 @@ impl<'tcx> InliningMap<'tcx> { } } - fn record_accesses(&mut self, source: MonoItem<'tcx>, new_targets: I) - where - I: Iterator, bool)> + ExactSizeIterator, + fn record_accesses(&mut self, source: MonoItem<'tcx>, new_targets: &[(MonoItem<'tcx>, bool)]) { assert!(!self.index.contains_key(&source)); @@ -240,9 +238,9 @@ impl<'tcx> InliningMap<'tcx> { self.targets.reserve(new_items_count); self.inlines.ensure(new_items_count_total); - for (i, (target, inline)) in new_targets.enumerate() { - self.targets.push(target); - if inline { + for (i, (target, inline)) in new_targets.iter().enumerate() { + self.targets.push(*target); + if *inline { self.inlines.insert(i + start_index); } } @@ -403,10 +401,12 @@ fn record_accesses<'tcx>( mono_item.instantiation_mode(tcx) == InstantiationMode::LocalCopy }; - let accesses = - callees.into_iter().map(|mono_item| (*mono_item, is_inlining_candidate(mono_item))); + let accesses: SmallVec<[_; 128]> = callees + .into_iter() + .map(|mono_item| (*mono_item, is_inlining_candidate(mono_item))) + .collect(); - inlining_map.lock_mut().record_accesses(caller, accesses); + inlining_map.lock_mut().record_accesses(caller, &accesses); } fn check_recursion_limit<'tcx>( From 51a73eb4fbbf2ec74c6a86942780fc2600811540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 31 Dec 2019 14:28:36 +0100 Subject: [PATCH 3/4] Avoid a duplicate hash map lookup --- src/librustc_mir/monomorphize/collector.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 422a1ccd9b5..e85e842031d 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -227,10 +227,7 @@ impl<'tcx> InliningMap<'tcx> { } } - fn record_accesses(&mut self, source: MonoItem<'tcx>, new_targets: &[(MonoItem<'tcx>, bool)]) - { - assert!(!self.index.contains_key(&source)); - + fn record_accesses(&mut self, source: MonoItem<'tcx>, new_targets: &[(MonoItem<'tcx>, bool)]) { let start_index = self.targets.len(); let new_items_count = new_targets.len(); let new_items_count_total = new_items_count + self.targets.len(); @@ -246,7 +243,7 @@ impl<'tcx> InliningMap<'tcx> { } let end_index = self.targets.len(); - self.index.insert(source, (start_index, end_index)); + assert!(self.index.insert(source, (start_index, end_index)).is_none()); } // Internally iterate over all items referenced by `source` which will be From 4a647167e64c77c72a074c354543bbe9dacb2d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 11 Jan 2020 00:38:10 +0100 Subject: [PATCH 4/4] Add a comment --- src/librustc_mir/monomorphize/collector.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index e85e842031d..511a5fbc617 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -398,6 +398,9 @@ fn record_accesses<'tcx>( mono_item.instantiation_mode(tcx) == InstantiationMode::LocalCopy }; + // We collect this into a `SmallVec` to avoid calling `is_inlining_candidate` in the lock. + // FIXME: Call `is_inlining_candidate` when pushing to `neighbors` in `collect_items_rec` + // instead to avoid creating this `SmallVec`. let accesses: SmallVec<[_; 128]> = callees .into_iter() .map(|mono_item| (*mono_item, is_inlining_candidate(mono_item)))