Skip to content

Do not compute the shallow hash twice. #119976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,25 +659,28 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let bodies = SortedMap::from_presorted_elements(bodies);

// Don't hash unless necessary, because it's expensive.
let (opt_hash_including_bodies, attrs_hash) = if self.tcx.needs_crate_hash() {
let (opt_hash_without_bodies, opt_bodies_hash, attrs_hash) = if self.tcx.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)
});
hcx.without_hir_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);
hcx.without_hir_bodies(|hcx| bodies.hash_stable(hcx, &mut stable_hasher));
let h2 = stable_hasher.finish();

(Some(h1), Some(h2))
let mut stable_hasher = StableHasher::new();
attrs.hash_stable(&mut hcx, &mut stable_hasher);
let h3 = stable_hasher.finish();

(Some(h1), Some(h2), Some(h3))
})
} else {
(None, None)
(None, None, None)
};
let (nodes, parenting) = index::index_hir(self.tcx, node, &bodies);
let nodes = hir::OwnerNodes { opt_hash_including_bodies, nodes, bodies };
let nodes = hir::OwnerNodes { opt_hash_without_bodies, opt_bodies_hash, nodes, bodies };
let attrs = hir::AttributeMap { map: attrs, opt_hash: attrs_hash };

self.arena.alloc(hir::OwnerInfo { nodes, parenting, attrs, trait_map })
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,11 @@ impl<'tcx> AttributeMap<'tcx> {
pub struct OwnerNodes<'tcx> {
/// 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>,
pub opt_bodies_hash: Option<Fingerprint>,
/// 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.
pub opt_hash_without_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 @@ -867,7 +871,8 @@ impl fmt::Debug for OwnerNodes<'_> {
.collect::<Vec<_>>(),
)
.field("bodies", &self.bodies)
.field("opt_hash_including_bodies", &self.opt_hash_including_bodies)
.field("opt_hash_without_bodies", &self.opt_hash_without_bodies)
.field("opt_bodies_hash", &self.opt_bodies_hash)
.finish()
}
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_hir/src/stable_hash_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ 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 { opt_hash_including_bodies, nodes: _, bodies: _ } = *self;
opt_hash_including_bodies.unwrap().hash_stable(hcx, hasher);
let OwnerNodes { opt_hash_without_bodies, opt_bodies_hash, nodes: _, bodies: _ } = *self;
opt_hash_without_bodies.unwrap().hash_stable(hcx, hasher);
opt_bodies_hash.unwrap().hash_stable(hcx, hasher);
}
}

Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub mod place;

use crate::query::Providers;
use crate::ty::{EarlyBinder, ImplSubject, TyCtxt};
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::{try_par_for_each_in, DynSend, DynSync};
use rustc_hir::def::DefKind;
Expand All @@ -24,15 +25,13 @@ use rustc_span::{ErrorGuaranteed, ExpnId, DUMMY_SP};
#[derive(Copy, Clone, Debug)]
pub struct Owner<'tcx> {
node: OwnerNode<'tcx>,
opt_hash_without_bodies: Option<Fingerprint>,
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for Owner<'tcx> {
#[inline]
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
// 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));
self.opt_hash_without_bodies.hash_stable(hcx, hasher)
}
}

Expand Down Expand Up @@ -152,7 +151,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 })
Some(Owner { node, opt_hash_without_bodies: owner.nodes.opt_hash_without_bodies })
};
providers.opt_local_def_id_to_hir_id = |tcx, id| {
let owner = tcx.hir_crate(()).owners[id].map(|_| ());
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans};

use crate::MirPass;

use rustc_data_structures::fingerprint::Fingerprint;
use rustc_middle::hir;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::coverage::*;
Expand Down Expand Up @@ -419,12 +420,9 @@ 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()
.opt_hash_including_bodies
.unwrap()
let owner = tcx.hir_owner_nodes(owner).unwrap();
Fingerprint::combine(owner.opt_hash_without_bodies.unwrap(), owner.opt_bodies_hash.unwrap())
.to_smaller_hash()
.as_u64()
}
23 changes: 2 additions & 21 deletions compiler/rustc_query_system/src/ich/hcx.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use crate::ich;

use rustc_ast as ast;
use rustc_data_structures::sorted_map::SortedMap;
use rustc_data_structures::stable_hasher::{HashStable, HashingControls, StableHasher};
use rustc_data_structures::sync::Lrc;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::definitions::DefPathHash;
use rustc_session::cstore::Untracked;
Expand All @@ -23,7 +21,7 @@ pub struct StableHashingContext<'a> {
// The value of `-Z incremental-ignore-spans`.
// This field should only be used by `unstable_opts_incremental_ignore_span`
incremental_ignore_spans: bool,
pub(super) body_resolver: BodyResolver<'a>,
pub(super) body_resolver: BodyResolver,
// Very often, we are hashing something that does not need the
// `CachingSourceMapView`, so we initialize it lazily.
raw_source_map: &'a SourceMap,
Expand All @@ -35,13 +33,9 @@ pub struct StableHashingContext<'a> {
/// We could also just store a plain reference to the `hir::Crate` but we want
/// to avoid that the crate is used to get untracked access to all of the HIR.
#[derive(Clone, Copy)]
pub(super) enum BodyResolver<'tcx> {
pub(super) enum BodyResolver {
Forbidden,
Ignore,
Traverse {
owner: hir::OwnerId,
bodies: &'tcx SortedMap<hir::ItemLocalId, &'tcx hir::Body<'tcx>>,
},
}

impl<'a> StableHashingContext<'a> {
Expand All @@ -64,19 +58,6 @@ impl<'a> StableHashingContext<'a> {
f(&mut StableHashingContext { body_resolver: BodyResolver::Ignore, ..self.clone() });
}

#[inline]
pub fn with_hir_bodies(
&mut self,
owner: hir::OwnerId,
bodies: &SortedMap<hir::ItemLocalId, &hir::Body<'_>>,
f: impl FnOnce(&mut StableHashingContext<'_>),
) {
f(&mut StableHashingContext {
body_resolver: BodyResolver::Traverse { owner, bodies },
..self.clone()
});
}

#[inline]
pub fn while_hashing_spans<F: FnOnce(&mut Self)>(&mut self, hash_spans: bool, f: F) {
let prev_hash_spans = self.hashing_controls.hash_spans;
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_query_system/src/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,11 @@ use rustc_hir as hir;

impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
#[inline]
fn hash_body_id(&mut self, id: hir::BodyId, hasher: &mut StableHasher) {
fn hash_body_id(&mut self, hir::BodyId { hir_id }: hir::BodyId, hasher: &mut StableHasher) {
let hcx = self;
match hcx.body_resolver {
BodyResolver::Forbidden => panic!("Hashing HIR bodies is forbidden."),
BodyResolver::Ignore => {}
BodyResolver::Traverse { owner, bodies } => {
assert_eq!(id.hir_id.owner, owner);
bodies[&id.hir_id.local_id].hash_stable(hcx, hasher);
}
BodyResolver::Ignore => hir_id.hash_stable(hcx, hasher),
}
}
}