From 6cd0dcade7c873209e6c220d40f811e5e72679f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 4 Jan 2020 21:42:28 +0100 Subject: [PATCH 1/7] Prefetch queries used by the metadata encoder --- src/librustc_metadata/rmeta/encoder.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 6280fd62de9a8..625970fbfbf47 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -18,7 +18,7 @@ use rustc_ast::attr; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{join, par_for_each_in, Lrc}; use rustc_hir as hir; use rustc_hir::def::CtorKind; use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; @@ -1721,6 +1721,22 @@ impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplVisitor<'tcx> { // generated regardless of trailing bytes that end up in it. pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { + join( + || encode_metadata_impl(tcx), + || { + // Prefetch some queries used by metadata encoding + tcx.dep_graph.with_ignore(|| { + par_for_each_in(tcx.mir_keys(LOCAL_CRATE), |&def_id| { + tcx.optimized_mir(def_id); + tcx.promoted_mir(def_id); + }); + }) + }, + ) + .0 +} + +fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata { let mut encoder = opaque::Encoder::new(vec![]); encoder.emit_raw_bytes(METADATA_HEADER); From 1a34cbc2e229d3d146a08ce1dc1d76497e05337d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 11 Jan 2020 03:42:40 +0100 Subject: [PATCH 2/7] Encode exported symbols last --- src/librustc_metadata/rmeta/encoder.rs | 12 ++++++------ src/librustc_metadata/rmeta/mod.rs | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 625970fbfbf47..74824e1f91bea 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -467,12 +467,6 @@ impl<'tcx> EncodeContext<'tcx> { let impls = self.encode_impls(); let impl_bytes = self.position() - i; - // Encode exported symbols info. - i = self.position(); - let exported_symbols = self.tcx.exported_symbols(LOCAL_CRATE); - let exported_symbols = self.encode_exported_symbols(&exported_symbols); - let exported_symbols_bytes = self.position() - i; - let tcx = self.tcx; // Encode the items. @@ -513,6 +507,12 @@ impl<'tcx> EncodeContext<'tcx> { let proc_macro_data = self.encode_proc_macros(); let proc_macro_data_bytes = self.position() - i; + // Encode exported symbols info. + i = self.position(); + let exported_symbols = self.tcx.exported_symbols(LOCAL_CRATE); + let exported_symbols = self.encode_exported_symbols(&exported_symbols); + let exported_symbols_bytes = self.position() - i; + let attrs = tcx.hir().krate_attrs(); let has_default_lib_allocator = attr::contains_name(&attrs, sym::default_lib_allocator); diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 05d834e5dee12..448c1610c1368 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -196,7 +196,6 @@ crate struct CrateRoot<'tcx> { source_map: Lazy<[rustc_span::SourceFile]>, def_path_table: Lazy, impls: Lazy<[TraitImpls]>, - exported_symbols: Lazy!([(ExportedSymbol<'tcx>, SymbolExportLevel)]), interpret_alloc_index: Lazy<[u32]>, per_def: LazyPerDefTables<'tcx>, @@ -204,6 +203,8 @@ crate struct CrateRoot<'tcx> { /// The DefIndex's of any proc macros declared by this crate. proc_macro_data: Option>, + exported_symbols: Lazy!([(ExportedSymbol<'tcx>, SymbolExportLevel)]), + compiler_builtins: bool, needs_allocator: bool, needs_panic_runtime: bool, From 03af82bb0cc425ff7d4518f36a80a5cb26b6a821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 11 Jan 2020 04:02:22 +0100 Subject: [PATCH 3/7] Prefetch exported symbols --- src/librustc_metadata/rmeta/encoder.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 74824e1f91bea..4fb116b551d9c 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -1726,10 +1726,15 @@ pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { || { // Prefetch some queries used by metadata encoding tcx.dep_graph.with_ignore(|| { - par_for_each_in(tcx.mir_keys(LOCAL_CRATE), |&def_id| { - tcx.optimized_mir(def_id); - tcx.promoted_mir(def_id); - }); + join( + || { + par_for_each_in(tcx.mir_keys(LOCAL_CRATE), |&def_id| { + tcx.optimized_mir(def_id); + tcx.promoted_mir(def_id); + }) + }, + || tcx.exported_symbols(LOCAL_CRATE), + ); }) }, ) From 3d59c0ee38e1f1796e3ae9e6124f0b257a9983f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 11 Jan 2020 04:47:20 +0100 Subject: [PATCH 4/7] Make the timer more verbose --- src/librustc/ty/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 742d57fb58a51..c6b43022b775d 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1323,7 +1323,7 @@ impl<'tcx> TyCtxt<'tcx> { } pub fn encode_metadata(self) -> EncodedMetadata { - let _prof_timer = self.prof.generic_activity("generate_crate_metadata"); + let _prof_timer = self.prof.verbose_generic_activity("generate_crate_metadata"); self.cstore.encode_metadata(self) } From a2bca90077962e513330ca4a6b729f8262ccf8d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Mon, 13 Jan 2020 16:23:42 +0100 Subject: [PATCH 5/7] Make metadata prefetching more accurate --- src/librustc_metadata/rmeta/encoder.rs | 82 +++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 7 deletions(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 4fb116b551d9c..0e735eeb01c8e 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -18,12 +18,13 @@ use rustc_ast::attr; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; -use rustc_data_structures::sync::{join, par_for_each_in, Lrc}; +use rustc_data_structures::sync::{join, Lrc}; use rustc_hir as hir; use rustc_hir::def::CtorKind; +use rustc_hir::def_id::DefIdSet; use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; +use rustc_hir::itemlikevisit::{ItemLikeVisitor, ParItemLikeVisitor}; use rustc_hir::{AnonConst, GenericParamKind}; use rustc_index::vec::Idx; use rustc_serialize::{opaque, Encodable, Encoder, SpecializedEncoder}; @@ -1697,6 +1698,66 @@ impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplVisitor<'tcx> { } } +/// Used to prefetch queries which will be needed later by metadata encoding. +struct PrefetchVisitor<'tcx> { + tcx: TyCtxt<'tcx>, + mir_keys: &'tcx DefIdSet, +} + +impl<'tcx> PrefetchVisitor<'tcx> { + fn prefetch_mir(&self, def_id: DefId) { + if self.mir_keys.contains(&def_id) { + self.tcx.optimized_mir(def_id); + self.tcx.promoted_mir(def_id); + } + } +} + +impl<'tcx, 'v> ParItemLikeVisitor<'v> for PrefetchVisitor<'tcx> { + fn visit_item(&self, item: &hir::Item<'_>) { + let tcx = self.tcx; + match item.kind { + hir::ItemKind::Static(..) | hir::ItemKind::Const(..) => { + self.prefetch_mir(tcx.hir().local_def_id(item.hir_id)) + } + hir::ItemKind::Fn(ref sig, ..) => { + let def_id = tcx.hir().local_def_id(item.hir_id); + let generics = tcx.generics_of(def_id); + let needs_inline = generics.requires_monomorphization(tcx) + || tcx.codegen_fn_attrs(def_id).requests_inline(); + if needs_inline || sig.header.constness == hir::Constness::Const { + self.prefetch_mir(def_id) + } + } + _ => (), + } + } + + fn visit_trait_item(&self, trait_item: &'v hir::TraitItem<'v>) { + self.prefetch_mir(self.tcx.hir().local_def_id(trait_item.hir_id)); + } + + fn visit_impl_item(&self, impl_item: &'v hir::ImplItem<'v>) { + let tcx = self.tcx; + match impl_item.kind { + hir::ImplItemKind::Const(..) => { + self.prefetch_mir(tcx.hir().local_def_id(impl_item.hir_id)) + } + hir::ImplItemKind::Fn(ref sig, _) => { + let def_id = tcx.hir().local_def_id(impl_item.hir_id); + let generics = tcx.generics_of(def_id); + let needs_inline = generics.requires_monomorphization(tcx) + || tcx.codegen_fn_attrs(def_id).requests_inline(); + let is_const_fn = sig.header.constness == hir::Constness::Const; + if needs_inline || is_const_fn { + self.prefetch_mir(def_id) + } + } + hir::ImplItemKind::OpaqueTy(..) | hir::ImplItemKind::TyAlias(..) => (), + } + } +} + // NOTE(eddyb) The following comment was preserved for posterity, even // though it's no longer relevant as EBML (which uses nested & tagged // "documents") was replaced with a scheme that can't go out of bounds. @@ -1724,14 +1785,21 @@ pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { join( || encode_metadata_impl(tcx), || { - // Prefetch some queries used by metadata encoding + if tcx.sess.threads() == 1 { + return; + } + // Prefetch some queries used by metadata encoding. tcx.dep_graph.with_ignore(|| { join( || { - par_for_each_in(tcx.mir_keys(LOCAL_CRATE), |&def_id| { - tcx.optimized_mir(def_id); - tcx.promoted_mir(def_id); - }) + if !tcx.sess.opts.output_types.should_codegen() { + // We won't emit MIR, so don't prefetch it. + return; + } + tcx.hir().krate().par_visit_all_item_likes(&PrefetchVisitor { + tcx, + mir_keys: tcx.mir_keys(LOCAL_CRATE), + }); }, || tcx.exported_symbols(LOCAL_CRATE), ); From 801e4420b6149654885851b6e3e7a9d7837b18de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 14 Mar 2020 13:39:33 +0100 Subject: [PATCH 6/7] Add some comments --- src/librustc_metadata/rmeta/encoder.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 0e735eeb01c8e..70f7170cd324a 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -508,7 +508,8 @@ impl<'tcx> EncodeContext<'tcx> { let proc_macro_data = self.encode_proc_macros(); let proc_macro_data_bytes = self.position() - i; - // Encode exported symbols info. + // Encode exported symbols info. This is prefetched in `encode_metadata` so we encode + // this last to give the prefetching as much time as possible to complete. i = self.position(); let exported_symbols = self.tcx.exported_symbols(LOCAL_CRATE); let exported_symbols = self.encode_exported_symbols(&exported_symbols); @@ -889,6 +890,8 @@ impl EncodeContext<'tcx> { self.encode_generics(def_id); self.encode_explicit_predicates(def_id); self.encode_inferred_outlives(def_id); + + // This should be kept in sync with `PrefetchVisitor.visit_trait_item`. self.encode_optimized_mir(def_id); self.encode_promoted_mir(def_id); } @@ -960,6 +963,9 @@ impl EncodeContext<'tcx> { self.encode_generics(def_id); self.encode_explicit_predicates(def_id); self.encode_inferred_outlives(def_id); + + // The following part should be kept in sync with `PrefetchVisitor.visit_impl_item`. + let mir = match ast_item.kind { hir::ImplItemKind::Const(..) => true, hir::ImplItemKind::Fn(ref sig, _) => { @@ -1251,6 +1257,8 @@ impl EncodeContext<'tcx> { _ => {} } + // The following part should be kept in sync with `PrefetchVisitor.visit_item`. + let mir = match item.kind { hir::ItemKind::Static(..) | hir::ItemKind::Const(..) => true, hir::ItemKind::Fn(ref sig, ..) => { @@ -1699,6 +1707,7 @@ impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplVisitor<'tcx> { } /// Used to prefetch queries which will be needed later by metadata encoding. +/// Only a subset of the queries are actually prefetched to keep this code smaller. struct PrefetchVisitor<'tcx> { tcx: TyCtxt<'tcx>, mir_keys: &'tcx DefIdSet, @@ -1715,6 +1724,7 @@ impl<'tcx> PrefetchVisitor<'tcx> { impl<'tcx, 'v> ParItemLikeVisitor<'v> for PrefetchVisitor<'tcx> { fn visit_item(&self, item: &hir::Item<'_>) { + // This should be kept in sync with `encode_info_for_item`. let tcx = self.tcx; match item.kind { hir::ItemKind::Static(..) | hir::ItemKind::Const(..) => { @@ -1734,10 +1744,12 @@ impl<'tcx, 'v> ParItemLikeVisitor<'v> for PrefetchVisitor<'tcx> { } fn visit_trait_item(&self, trait_item: &'v hir::TraitItem<'v>) { + // This should be kept in sync with `encode_info_for_trait_item`. self.prefetch_mir(self.tcx.hir().local_def_id(trait_item.hir_id)); } fn visit_impl_item(&self, impl_item: &'v hir::ImplItem<'v>) { + // This should be kept in sync with `encode_info_for_impl_item`. let tcx = self.tcx; match impl_item.kind { hir::ImplItemKind::Const(..) => { @@ -1789,6 +1801,8 @@ pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { return; } // Prefetch some queries used by metadata encoding. + // This is not necessary for correctness, but is only done for performance reasons. + // It can be removed if it turns out to cause trouble or be detrimental to performance. tcx.dep_graph.with_ignore(|| { join( || { From 027c8d998e30a362319b54b80aaf8cf8ff5bc39d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 14 Mar 2020 13:41:38 +0100 Subject: [PATCH 7/7] Use `assert_ignored` when encoding metadata --- src/librustc_metadata/rmeta/encoder.rs | 75 +++++++++++++------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 70f7170cd324a..5963047fc760d 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -1794,6 +1794,10 @@ impl<'tcx, 'v> ParItemLikeVisitor<'v> for PrefetchVisitor<'tcx> { // generated regardless of trailing bytes that end up in it. pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { + // Since encoding metadata is not in a query, and nothing is cached, + // there's no need to do dep-graph tracking for any of it. + tcx.dep_graph.assert_ignored(); + join( || encode_metadata_impl(tcx), || { @@ -1803,21 +1807,19 @@ pub(super) fn encode_metadata(tcx: TyCtxt<'_>) -> EncodedMetadata { // Prefetch some queries used by metadata encoding. // This is not necessary for correctness, but is only done for performance reasons. // It can be removed if it turns out to cause trouble or be detrimental to performance. - tcx.dep_graph.with_ignore(|| { - join( - || { - if !tcx.sess.opts.output_types.should_codegen() { - // We won't emit MIR, so don't prefetch it. - return; - } - tcx.hir().krate().par_visit_all_item_likes(&PrefetchVisitor { - tcx, - mir_keys: tcx.mir_keys(LOCAL_CRATE), - }); - }, - || tcx.exported_symbols(LOCAL_CRATE), - ); - }) + join( + || { + if !tcx.sess.opts.output_types.should_codegen() { + // We won't emit MIR, so don't prefetch it. + return; + } + tcx.hir().krate().par_visit_all_item_likes(&PrefetchVisitor { + tcx, + mir_keys: tcx.mir_keys(LOCAL_CRATE), + }); + }, + || tcx.exported_symbols(LOCAL_CRATE), + ); }, ) .0 @@ -1830,29 +1832,26 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata { // Will be filled with the root position after encoding everything. encoder.emit_raw_bytes(&[0, 0, 0, 0]); - // Since encoding metadata is not in a query, and nothing is cached, - // there's no need to do dep-graph tracking for any of it. - let (root, mut result) = tcx.dep_graph.with_ignore(move || { - let mut ecx = EncodeContext { - opaque: encoder, - tcx, - per_def: Default::default(), - lazy_state: LazyState::NoNode, - type_shorthands: Default::default(), - predicate_shorthands: Default::default(), - source_file_cache: tcx.sess.source_map().files()[0].clone(), - interpret_allocs: Default::default(), - interpret_allocs_inverse: Default::default(), - }; - - // Encode the rustc version string in a predictable location. - rustc_version().encode(&mut ecx).unwrap(); - - // Encode all the entries and extra information in the crate, - // culminating in the `CrateRoot` which points to all of it. - let root = ecx.encode_crate_root(); - (root, ecx.opaque.into_inner()) - }); + let mut ecx = EncodeContext { + opaque: encoder, + tcx, + per_def: Default::default(), + lazy_state: LazyState::NoNode, + type_shorthands: Default::default(), + predicate_shorthands: Default::default(), + source_file_cache: tcx.sess.source_map().files()[0].clone(), + interpret_allocs: Default::default(), + interpret_allocs_inverse: Default::default(), + }; + + // Encode the rustc version string in a predictable location. + rustc_version().encode(&mut ecx).unwrap(); + + // Encode all the entries and extra information in the crate, + // culminating in the `CrateRoot` which points to all of it. + let root = ecx.encode_crate_root(); + + let mut result = ecx.opaque.into_inner(); // Encode the root position. let header = METADATA_HEADER.len();