Auto merge of rust-lang#108794 - nnethercote:avoid-unnecessary-hashing, r=cjgillot

Avoid unnecessary hashing

I noticed some stable hashing being done in a non-incremental build. It turns out that some of this is necessary to compute the crate hash, but some of it is not. Removing the unnecessary hashing is a perf win.

r? `@cjgillot`

Author: bors

compiler/rustc_ast_lowering/src/lib.rs

@@ -464,8 +464,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)]
@@ -658,42 +660,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<hir::ItemLocalId, &'hir hir::Body<'hir>>,
-    ) -> (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

compiler/rustc_hir/src/hir.rs

@@ -815,12 +815,13 @@ pub struct ParentedNode<'tcx> {
 #[derive(Debug)]
 pub struct AttributeMap<'tcx> {
     pub map: SortedMap<ItemLocalId, &'tcx [Attribute]>,
-    pub hash: Fingerprint,
+    // Only present when the crate hash is needed.
+    pub opt_hash: Option<Fingerprint>,
 }
 
 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<Fingerprint>,
     /// 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::<Vec<_>>(),
             )
             .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<T> MaybeOwner<T> {
 #[derive(Debug)]
 pub struct Crate<'hir> {
     pub owners: IndexVec<LocalDefId, MaybeOwner<&'hir OwnerInfo<'hir>>>,
-    pub hir_hash: Fingerprint,
+    // Only present when incr. comp. is enabled.
+    pub opt_hir_hash: Option<Fingerprint>,
 }
 
 #[derive(Debug, HashStable_Generic)]

compiler/rustc_hir/src/stable_hash_impls.rs

@@ -100,24 +100,23 @@ impl<'tcx, HirCtx: crate::HashStableContext> HashStable<HirCtx> 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<HirCtx> 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<HirCtx: crate::HashStableContext> HashStable<HirCtx> 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)
     }
 }

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<Svh>) {
     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");
 

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<OutputFilenames>,
-    crate_hash: Svh,
+    // Only present when incr. comp. is enabled.
+    crate_hash: Option<Svh>,
     ongoing_codegen: Box<dyn Any>,
 }
 

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| {

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);
 

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<StableHashingContext<'a>> 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(|_| ());

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()
 }

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<Symbol>) {
         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<CrateType>) {
         self.crate_types.set(crate_types).expect("`crate_types` was initialized twice")
     }