Skip to content

Prehash visibilities in resolver #143371

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+12 −7
Open

Prehash visibilities in resolver #143371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 1 addition & 5 deletions compiler/rustc_middle/src/hir/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,11 +1178,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
}
tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher);
tcx.stable_crate_id(LOCAL_CRATE).hash_stable(&mut hcx, &mut stable_hasher);
// Hash visibility information since it does not appear in HIR.
// FIXME: Figure out how to remove `visibilities_for_hashing` by hashing visibilities on
// the fly in the resolver, storing only their accumulated hash in `ResolverGlobalCtxt`,
// and combining it with other hashes here.
resolutions.visibilities_for_hashing.hash_stable(&mut hcx, &mut stable_hasher);
resolutions.visibilities_hash.hash_stable(&mut hcx, &mut stable_hasher);
with_metavar_spans(|mspans| {
mspans.freeze_and_get_read_spans().hash_stable(&mut hcx, &mut stable_hasher);
});
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub use intrinsic::IntrinsicDef;
use rustc_abi::{Align, FieldIdx, Integer, IntegerType, ReprFlags, ReprOptions, VariantIdx};
use rustc_ast::node_id::NodeMap;
pub use rustc_ast_ir::{Movability, Mutability, try_visit};
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
Expand Down Expand Up @@ -170,7 +171,7 @@ mod visit;

#[derive(Debug, HashStable)]
pub struct ResolverGlobalCtxt {
pub visibilities_for_hashing: Vec<(LocalDefId, Visibility)>,
pub visibilities_hash: Fingerprint,
/// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`.
pub expn_that_defined: UnordMap<LocalDefId, ExpnId>,
pub effective_visibilities: EffectiveVisibilities,
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use rustc_ast::{
};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{FreezeReadGuard, FreezeWriteGuard};
use rustc_data_structures::unord::{UnordMap, UnordSet};
Expand Down Expand Up @@ -1701,6 +1702,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}

pub fn into_outputs(self) -> ResolverOutputs {
let visibilities_hash = {
let mut hasher = StableHasher::new();
let mut hcx = self.create_stable_hashing_context();
self.visibilities_for_hashing.hash_stable(&mut hcx, &mut hasher);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about doing this operation in fn feed_visibility and keeping only the hash in Resolver instead of the whole visibilities_for_hashing table.

Is creating the hashing context a more or less expensive operation than adding a table element?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a hashing context is very cheap. It's just moving some pointers around. I guess we could keep a StableHasher around, feed it the individual items, and finish into a hash here.

hasher.finish()
};

let proc_macros = self.proc_macros;
let expn_that_defined = self.expn_that_defined;
let extern_crate_map = self.extern_crate_map;
Expand All @@ -1722,7 +1730,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

let global_ctxt = ResolverGlobalCtxt {
expn_that_defined,
visibilities_for_hashing: self.visibilities_for_hashing,
visibilities_hash,
effective_visibilities,
extern_crate_map,
module_children: self.module_children,
Expand Down
Loading