From 9570023ce13d0b2fb3d5b79ce9db57e6d28c31d1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 3 Mar 2023 17:02:11 +1100 Subject: [PATCH] Only compute the crate hash when necessary. The crate hash is needed: - if debug assertions are enabled, or - if incr. comp. is enabled, or - if metadata is being generated, or - if `-C instrumentation-coverage` is enabled. This commit avoids computing the crate hash when these conditions are all false, such as when doing a release build of a binary crate. It uses `Option` to store the hashes when needed, rather than computing them on demand, because some of them are needed in multiple places and computing them on demand would make compilation slower. The commit also removes `Owner::hash_without_bodies`. There is no benefit to pre-computing that one, it can just be done in the normal fashion. --- compiler/rustc_ast_lowering/src/lib.rs | 55 ++++++++----------- compiler/rustc_hir/src/hir.rs | 18 +++--- compiler/rustc_hir/src/stable_hash_impls.rs | 17 +++--- compiler/rustc_incremental/src/persist/fs.rs | 4 +- compiler/rustc_interface/src/queries.rs | 9 ++- compiler/rustc_metadata/src/fs.rs | 26 +-------- compiler/rustc_middle/src/hir/map/mod.rs | 2 +- compiler/rustc_middle/src/hir/mod.rs | 10 ++-- .../rustc_mir_transform/src/coverage/mod.rs | 2 +- compiler/rustc_session/src/session.rs | 39 +++++++++++++ 10 files changed, 100 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index b20157f2c7c89..234d88a1ac6ce 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -463,8 +463,10 @@ pub fn lower_to_hir(tcx: TyCtxt<'_>, (): ()) -> hir::Crate<'_> { rustc_span::hygiene::clear_syntax_context_map(); } - let hir_hash = compute_hir_hash(tcx, &owners); - hir::Crate { owners, hir_hash } + // Don't hash unless necessary, because it's expensive. + let opt_hir_hash = + if tcx.sess.needs_crate_hash() { Some(compute_hir_hash(tcx, &owners)) } else { None }; + hir::Crate { owners, opt_hir_hash } } #[derive(Copy, Clone, PartialEq, Debug)] @@ -657,42 +659,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { bodies.sort_by_key(|(k, _)| *k); let bodies = SortedMap::from_presorted_elements(bodies); - let (hash_including_bodies, hash_without_bodies) = self.hash_owner(node, &bodies); - let (nodes, parenting) = - index::index_hir(self.tcx.sess, &*self.tcx.definitions_untracked(), node, &bodies); - let nodes = hir::OwnerNodes { hash_including_bodies, hash_without_bodies, nodes, bodies }; - let attrs = { - let hash = self.tcx.with_stable_hashing_context(|mut hcx| { + + // Don't hash unless necessary, because it's expensive. + let (opt_hash_including_bodies, attrs_hash) = if self.tcx.sess.needs_crate_hash() { + self.tcx.with_stable_hashing_context(|mut hcx| { + let mut stable_hasher = StableHasher::new(); + hcx.with_hir_bodies(node.def_id(), &bodies, |hcx| { + node.hash_stable(hcx, &mut stable_hasher) + }); + let h1 = stable_hasher.finish(); + let mut stable_hasher = StableHasher::new(); attrs.hash_stable(&mut hcx, &mut stable_hasher); - stable_hasher.finish() - }); - hir::AttributeMap { map: attrs, hash } + let h2 = stable_hasher.finish(); + + (Some(h1), Some(h2)) + }) + } else { + (None, None) }; + let (nodes, parenting) = + index::index_hir(self.tcx.sess, &*self.tcx.definitions_untracked(), node, &bodies); + let nodes = hir::OwnerNodes { opt_hash_including_bodies, nodes, bodies }; + let attrs = hir::AttributeMap { map: attrs, opt_hash: attrs_hash }; self.arena.alloc(hir::OwnerInfo { nodes, parenting, attrs, trait_map }) } - /// Hash the HIR node twice, one deep and one shallow hash. This allows to differentiate - /// queries which depend on the full HIR tree and those which only depend on the item signature. - fn hash_owner( - &mut self, - node: hir::OwnerNode<'hir>, - bodies: &SortedMap>, - ) -> (Fingerprint, Fingerprint) { - self.tcx.with_stable_hashing_context(|mut hcx| { - let mut stable_hasher = StableHasher::new(); - hcx.with_hir_bodies(node.def_id(), bodies, |hcx| { - node.hash_stable(hcx, &mut stable_hasher) - }); - let hash_including_bodies = stable_hasher.finish(); - let mut stable_hasher = StableHasher::new(); - hcx.without_hir_bodies(|hcx| node.hash_stable(hcx, &mut stable_hasher)); - let hash_without_bodies = stable_hasher.finish(); - (hash_including_bodies, hash_without_bodies) - }) - } - /// This method allocates a new `HirId` for the given `NodeId` and stores it in /// the `LoweringContext`'s `NodeId => HirId` map. /// Take care not to call this method if the resulting `HirId` is then not diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 19d3d41c9841c..f00a5f24e4545 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -815,12 +815,13 @@ pub struct ParentedNode<'tcx> { #[derive(Debug)] pub struct AttributeMap<'tcx> { pub map: SortedMap, - pub hash: Fingerprint, + // Only present when the crate hash is needed. + pub opt_hash: Option, } impl<'tcx> AttributeMap<'tcx> { pub const EMPTY: &'static AttributeMap<'static> = - &AttributeMap { map: SortedMap::new(), hash: Fingerprint::ZERO }; + &AttributeMap { map: SortedMap::new(), opt_hash: Some(Fingerprint::ZERO) }; #[inline] pub fn get(&self, id: ItemLocalId) -> &'tcx [Attribute] { @@ -832,10 +833,9 @@ impl<'tcx> AttributeMap<'tcx> { /// These nodes are mapped by `ItemLocalId` alongside the index of their parent node. /// The HIR tree, including bodies, is pre-hashed. pub struct OwnerNodes<'tcx> { - /// Pre-computed hash of the full HIR. - pub hash_including_bodies: Fingerprint, - /// Pre-computed hash of the item signature, without recursing into the body. - pub hash_without_bodies: Fingerprint, + /// Pre-computed hash of the full HIR. Used in the crate hash. Only present + /// when incr. comp. is enabled. + pub opt_hash_including_bodies: Option, /// Full HIR for the current owner. // The zeroth node's parent should never be accessed: the owner's parent is computed by the // hir_owner_parent query. It is set to `ItemLocalId::INVALID` to force an ICE if accidentally @@ -872,8 +872,7 @@ impl fmt::Debug for OwnerNodes<'_> { .collect::>(), ) .field("bodies", &self.bodies) - .field("hash_without_bodies", &self.hash_without_bodies) - .field("hash_including_bodies", &self.hash_including_bodies) + .field("opt_hash_including_bodies", &self.opt_hash_including_bodies) .finish() } } @@ -940,7 +939,8 @@ impl MaybeOwner { #[derive(Debug)] pub struct Crate<'hir> { pub owners: IndexVec>>, - pub hir_hash: Fingerprint, + // Only present when incr. comp. is enabled. + pub opt_hir_hash: Option, } #[derive(Debug, HashStable_Generic)] diff --git a/compiler/rustc_hir/src/stable_hash_impls.rs b/compiler/rustc_hir/src/stable_hash_impls.rs index 85d0e02d0b683..97fa710b35499 100644 --- a/compiler/rustc_hir/src/stable_hash_impls.rs +++ b/compiler/rustc_hir/src/stable_hash_impls.rs @@ -100,24 +100,23 @@ impl<'tcx, HirCtx: crate::HashStableContext> HashStable for OwnerNodes<' // `local_id_to_def_id` is also ignored because is dependent on the body, then just hashing // the body satisfies the condition of two nodes being different have different // `hash_stable` results. - let OwnerNodes { hash_including_bodies, hash_without_bodies: _, nodes: _, bodies: _ } = - *self; - hash_including_bodies.hash_stable(hcx, hasher); + let OwnerNodes { opt_hash_including_bodies, nodes: _, bodies: _ } = *self; + opt_hash_including_bodies.unwrap().hash_stable(hcx, hasher); } } impl<'tcx, HirCtx: crate::HashStableContext> HashStable for AttributeMap<'tcx> { fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) { - // We ignore the `map` since it refers to information included in `hash` which is hashed in - // the collector and used for the crate hash. - let AttributeMap { hash, map: _ } = *self; - hash.hash_stable(hcx, hasher); + // We ignore the `map` since it refers to information included in `opt_hash` which is + // hashed in the collector and used for the crate hash. + let AttributeMap { opt_hash, map: _ } = *self; + opt_hash.unwrap().hash_stable(hcx, hasher); } } impl HashStable for Crate<'_> { fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) { - let Crate { owners: _, hir_hash } = self; - hir_hash.hash_stable(hcx, hasher) + let Crate { owners: _, opt_hir_hash } = self; + opt_hir_hash.unwrap().hash_stable(hcx, hasher) } } diff --git a/compiler/rustc_incremental/src/persist/fs.rs b/compiler/rustc_incremental/src/persist/fs.rs index 73d7e3becab48..4deae9f41c712 100644 --- a/compiler/rustc_incremental/src/persist/fs.rs +++ b/compiler/rustc_incremental/src/persist/fs.rs @@ -297,10 +297,12 @@ pub fn prepare_session_directory( /// renaming it to `s-{timestamp}-{svh}` and releasing the file lock. /// If there have been compilation errors, however, this function will just /// delete the presumably invalid session directory. -pub fn finalize_session_directory(sess: &Session, svh: Svh) { +pub fn finalize_session_directory(sess: &Session, svh: Option) { if sess.opts.incremental.is_none() { return; } + // The svh is always produced when incr. comp. is enabled. + let svh = svh.unwrap(); let _timer = sess.timer("incr_comp_finalize_session_directory"); diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index a96cc95a38446..58ad044b399b6 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -284,7 +284,11 @@ impl<'tcx> Queries<'tcx> { let codegen_backend = self.codegen_backend().clone(); let (crate_hash, prepare_outputs, dep_graph) = self.global_ctxt()?.enter(|tcx| { - (tcx.crate_hash(LOCAL_CRATE), tcx.output_filenames(()).clone(), tcx.dep_graph.clone()) + ( + if tcx.sess.needs_crate_hash() { Some(tcx.crate_hash(LOCAL_CRATE)) } else { None }, + tcx.output_filenames(()).clone(), + tcx.dep_graph.clone(), + ) }); let ongoing_codegen = self.ongoing_codegen()?.steal(); @@ -308,7 +312,8 @@ pub struct Linker { // compilation outputs dep_graph: DepGraph, prepare_outputs: Arc, - crate_hash: Svh, + // Only present when incr. comp. is enabled. + crate_hash: Option, ongoing_codegen: Box, } diff --git a/compiler/rustc_metadata/src/fs.rs b/compiler/rustc_metadata/src/fs.rs index f6431899731ff..08de828fbdba2 100644 --- a/compiler/rustc_metadata/src/fs.rs +++ b/compiler/rustc_metadata/src/fs.rs @@ -6,9 +6,9 @@ use crate::{encode_metadata, EncodedMetadata}; use rustc_data_structures::temp_dir::MaybeTempDir; use rustc_hir::def_id::LOCAL_CRATE; use rustc_middle::ty::TyCtxt; -use rustc_session::config::{CrateType, OutputType}; +use rustc_session::config::OutputType; use rustc_session::output::filename_for_metadata; -use rustc_session::Session; +use rustc_session::{MetadataKind, Session}; use tempfile::Builder as TempFileBuilder; use std::fs; @@ -39,27 +39,6 @@ pub fn emit_wrapper_file( } pub fn encode_and_write_metadata(tcx: TyCtxt<'_>) -> (EncodedMetadata, bool) { - #[derive(PartialEq, Eq, PartialOrd, Ord)] - enum MetadataKind { - None, - Uncompressed, - Compressed, - } - - let metadata_kind = tcx - .sess - .crate_types() - .iter() - .map(|ty| match *ty { - CrateType::Executable | CrateType::Staticlib | CrateType::Cdylib => MetadataKind::None, - - CrateType::Rlib => MetadataKind::Uncompressed, - - CrateType::Dylib | CrateType::ProcMacro => MetadataKind::Compressed, - }) - .max() - .unwrap_or(MetadataKind::None); - let crate_name = tcx.crate_name(LOCAL_CRATE); let out_filename = filename_for_metadata(tcx.sess, crate_name, tcx.output_filenames(())); // To avoid races with another rustc process scanning the output directory, @@ -76,6 +55,7 @@ pub fn encode_and_write_metadata(tcx: TyCtxt<'_>) -> (EncodedMetadata, bool) { // Always create a file at `metadata_filename`, even if we have nothing to write to it. // This simplifies the creation of the output `out_filename` when requested. + let metadata_kind = tcx.sess.metadata_kind(); match metadata_kind { MetadataKind::None => { std::fs::File::create(&metadata_filename).unwrap_or_else(|err| { diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 43eef1c770c96..746cf48858932 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -1134,7 +1134,7 @@ impl<'hir> intravisit::Map<'hir> for Map<'hir> { pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh { debug_assert_eq!(crate_num, LOCAL_CRATE); let krate = tcx.hir_crate(()); - let hir_body_hash = krate.hir_hash; + let hir_body_hash = krate.opt_hir_hash.expect("HIR hash missing while computing crate hash"); let upstream_crates = upstream_crates(tcx); diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 6706b9db3f544..6c0566cd9e8a9 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -8,7 +8,6 @@ pub mod place; use crate::ty::query::Providers; use crate::ty::{ImplSubject, TyCtxt}; -use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -24,14 +23,15 @@ use rustc_span::{ExpnId, DUMMY_SP}; #[derive(Copy, Clone, Debug)] pub struct Owner<'tcx> { node: OwnerNode<'tcx>, - hash_without_bodies: Fingerprint, } impl<'a, 'tcx> HashStable> for Owner<'tcx> { #[inline] fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - let Owner { node: _, hash_without_bodies } = self; - hash_without_bodies.hash_stable(hcx, hasher) + // Perform a shallow hash instead using the deep hash saved in `OwnerNodes`. This lets us + // differentiate queries that depend on the full HIR tree from those that only depend on + // the item signature. + hcx.without_hir_bodies(|hcx| self.node.hash_stable(hcx, hasher)); } } @@ -123,7 +123,7 @@ pub fn provide(providers: &mut Providers) { providers.hir_owner = |tcx, id| { let owner = tcx.hir_crate(()).owners.get(id.def_id)?.as_owner()?; let node = owner.node(); - Some(Owner { node, hash_without_bodies: owner.nodes.hash_without_bodies }) + Some(Owner { node }) }; providers.opt_local_def_id_to_hir_id = |tcx, id| { let owner = tcx.hir_crate(()).owners[id].map(|_| ()); diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 9a617159813ca..5ecb2d6a6313e 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -577,5 +577,5 @@ fn get_body_span<'tcx>( fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { // FIXME(cjgillot) Stop hashing HIR manually here. let owner = hir_body.id().hir_id.owner; - tcx.hir_owner_nodes(owner).unwrap().hash_including_bodies.to_smaller_hash() + tcx.hir_owner_nodes(owner).unwrap().opt_hash_including_bodies.unwrap().to_smaller_hash() } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 12634f67185fd..f91a1cdb70fc6 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -224,6 +224,13 @@ pub struct PerfStats { pub normalize_projection_ty: AtomicUsize, } +#[derive(PartialEq, Eq, PartialOrd, Ord)] +pub enum MetadataKind { + None, + Uncompressed, + Compressed, +} + impl Session { pub fn miri_unleashed_feature(&self, span: Span, feature_gate: Option) { self.miri_unleashed_features.lock().push((span, feature_gate)); @@ -287,6 +294,38 @@ impl Session { self.crate_types.get().unwrap().as_slice() } + pub fn needs_crate_hash(&self) -> bool { + // Why is the crate hash needed for these configurations? + // - debug_assertions: for the "fingerprint the result" check in + // `rustc_query_system::query::plumbing::execute_job`. + // - incremental: for query lookups. + // - needs_metadata: for putting into crate metadata. + // - instrument_coverage: for putting into coverage data (see + // `hash_mir_source`). + cfg!(debug_assertions) + || self.opts.incremental.is_some() + || self.needs_metadata() + || self.instrument_coverage() + } + + pub fn metadata_kind(&self) -> MetadataKind { + self.crate_types() + .iter() + .map(|ty| match *ty { + CrateType::Executable | CrateType::Staticlib | CrateType::Cdylib => { + MetadataKind::None + } + CrateType::Rlib => MetadataKind::Uncompressed, + CrateType::Dylib | CrateType::ProcMacro => MetadataKind::Compressed, + }) + .max() + .unwrap_or(MetadataKind::None) + } + + pub fn needs_metadata(&self) -> bool { + self.metadata_kind() != MetadataKind::None + } + pub fn init_crate_types(&self, crate_types: Vec) { self.crate_types.set(crate_types).expect("`crate_types` was initialized twice") }