Skip to content

Commit

Permalink
Only compute the crate hash when necessary.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nnethercote committed Mar 7, 2023
1 parent 816f958 commit 9570023
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 82 deletions.
55 changes: 24 additions & 31 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<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
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
Expand All @@ -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
Expand Down Expand Up @@ -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()
}
}
Expand Down Expand Up @@ -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)]
Expand Down
17 changes: 8 additions & 9 deletions compiler/rustc_hir/src/stable_hash_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
4 changes: 3 additions & 1 deletion compiler/rustc_incremental/src/persist/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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>,
}

Expand Down
26 changes: 3 additions & 23 deletions compiler/rustc_metadata/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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(|_| ());
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
39 changes: 39 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit 9570023

Please sign in to comment.