Skip to content

Reduce the amount of untracked state in TyCtxt -- Take 2 #85941

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

Merged
merged 2 commits into from
Jul 11, 2021
Merged
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
4 changes: 2 additions & 2 deletions compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use tracing::debug;
/// Internally the `DefPathTable` holds a tree of `DefKey`s, where each `DefKey`
/// stores the `DefIndex` of its parent.
/// There is one `DefPathTable` for each crate.
#[derive(Clone, Default)]
#[derive(Clone, Default, Debug)]
pub struct DefPathTable {
index_to_key: IndexVec<DefIndex, DefKey>,
def_path_hashes: IndexVec<DefIndex, DefPathHash>,
Expand Down Expand Up @@ -96,7 +96,7 @@ impl DefPathTable {
/// The definition table containing node definitions.
/// It holds the `DefPathTable` for `LocalDefId`s/`DefPath`s.
/// It also stores mappings to convert `LocalDefId`s to/from `HirId`s.
#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct Definitions {
table: DefPathTable,

Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ pub struct CStore {
unused_externs: Vec<Symbol>,
}

impl std::fmt::Debug for CStore {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("CStore").finish_non_exhaustive()
}
}

pub struct CrateLoader<'a> {
// Immutable configuration.
sess: &'a Session,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}

fn encode_def_path_table(&mut self) {
let table = self.tcx.hir().definitions().def_path_table();
let table = self.tcx.resolutions(()).definitions.def_path_table();
if self.is_proc_macro {
for def_index in std::iter::once(CRATE_DEF_INDEX)
.chain(self.tcx.hir().krate().proc_macros.iter().map(|p| p.owner.local_def_index))
Expand Down Expand Up @@ -1062,7 +1062,7 @@ impl EncodeContext<'a, 'tcx> {

let data = ModData {
reexports,
expansion: tcx.hir().definitions().expansion_that_defined(local_def_id),
expansion: tcx.resolutions(()).definitions.expansion_that_defined(local_def_id),
};

record!(self.tables.kind[def_id] <- EntryKind::Mod(self.lazy(data)));
Expand Down Expand Up @@ -1759,7 +1759,7 @@ impl EncodeContext<'a, 'tcx> {
.map(|(trait_def_id, mut impls)| {
// Bring everything into deterministic order for hashing
impls.sort_by_cached_key(|&(index, _)| {
tcx.hir().definitions().def_path_hash(LocalDefId { local_def_index: index })
tcx.hir().def_path_hash(LocalDefId { local_def_index: index })
});

TraitImpls {
Expand Down
45 changes: 30 additions & 15 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::svh::Svh;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, Definitions};
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
use rustc_hir::intravisit;
use rustc_hir::intravisit::Visitor;
use rustc_hir::itemlikevisit::ItemLikeVisitor;
Expand Down Expand Up @@ -154,21 +154,24 @@ impl<'hir> Map<'hir> {
self.tcx.hir_crate(())
}

#[inline]
pub fn definitions(&self) -> &'hir Definitions {
&self.tcx.definitions
}

pub fn def_key(&self, def_id: LocalDefId) -> DefKey {
self.tcx.definitions.def_key(def_id)
// Accessing the DefKey is ok, since it is part of DefPathHash.
self.tcx.untracked_resolutions.definitions.def_key(def_id)
}

pub fn def_path_from_hir_id(&self, id: HirId) -> Option<DefPath> {
self.opt_local_def_id(id).map(|def_id| self.def_path(def_id))
}

pub fn def_path(&self, def_id: LocalDefId) -> DefPath {
self.tcx.definitions.def_path(def_id)
// Accessing the DefPath is ok, since it is part of DefPathHash.
self.tcx.untracked_resolutions.definitions.def_path(def_id)
}

#[inline]
pub fn def_path_hash(self, def_id: LocalDefId) -> DefPathHash {
// Accessing the DefPathHash is ok, it is incr. comp. stable.
self.tcx.untracked_resolutions.definitions.def_path_hash(def_id)
}

#[inline]
Expand All @@ -184,16 +187,21 @@ impl<'hir> Map<'hir> {

#[inline]
pub fn opt_local_def_id(&self, hir_id: HirId) -> Option<LocalDefId> {
self.tcx.definitions.opt_hir_id_to_local_def_id(hir_id)
// FIXME(#85914) is this access safe for incr. comp.?
self.tcx.untracked_resolutions.definitions.opt_hir_id_to_local_def_id(hir_id)
}

#[inline]
pub fn local_def_id_to_hir_id(&self, def_id: LocalDefId) -> HirId {
self.tcx.definitions.local_def_id_to_hir_id(def_id)
// FIXME(#85914) is this access safe for incr. comp.?
self.tcx.untracked_resolutions.definitions.local_def_id_to_hir_id(def_id)
}

pub fn iter_local_def_id(&self) -> impl Iterator<Item = LocalDefId> + '_ {
self.tcx.definitions.iter_local_def_id()
// Create a dependency to the crate to be sure we reexcute this when the amount of
// definitions change.
self.tcx.ensure().hir_crate(());
self.tcx.untracked_resolutions.definitions.iter_local_def_id()
}

pub fn opt_def_kind(&self, local_def_id: LocalDefId) -> Option<DefKind> {
Expand Down Expand Up @@ -932,9 +940,15 @@ impl<'hir> intravisit::Map<'hir> for Map<'hir> {
pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tcx> {
let _prof_timer = tcx.sess.prof.generic_activity("build_hir_map");

// We can access untracked state since we are an eval_always query.
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to enforce this in a follow-up PR - for example, moving the 'untracked' fields behind methods, which debug-assert that we're in an eval_always query.

let hcx = tcx.create_stable_hashing_context();
let mut collector =
NodeCollector::root(tcx.sess, &**tcx.arena, tcx.untracked_crate, &tcx.definitions, hcx);
let mut collector = NodeCollector::root(
tcx.sess,
&**tcx.arena,
tcx.untracked_crate,
&tcx.untracked_resolutions.definitions,
hcx,
);
intravisit::walk_crate(&mut collector, tcx.untracked_crate);

let map = collector.finalize_and_compute_crate_hash();
Expand All @@ -944,14 +958,15 @@ pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tc
pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
assert_eq!(crate_num, LOCAL_CRATE);

// We can access untracked state since we are an eval_always query.
let mut hcx = tcx.create_stable_hashing_context();

let mut hir_body_nodes: Vec<_> = tcx
.index_hir(())
.map
.iter_enumerated()
.filter_map(|(def_id, hod)| {
let def_path_hash = tcx.definitions.def_path_hash(def_id);
let def_path_hash = tcx.untracked_resolutions.definitions.def_path_hash(def_id);
let mut hasher = StableHasher::new();
hod.as_ref()?.hash_stable(&mut hcx, &mut hasher);
AttributeMap { map: &tcx.untracked_crate.attrs, prefix: def_id }
Expand All @@ -968,7 +983,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
},
);

let upstream_crates = upstream_crates(&*tcx.cstore);
let upstream_crates = upstream_crates(&*tcx.untracked_resolutions.cstore);

// We hash the final, remapped names of all local source files so we
// don't have to include the path prefix remapping commandline args.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,6 @@ pub fn provide(providers: &mut Providers) {
providers.all_local_trait_impls = |tcx, ()| &tcx.hir_crate(()).trait_impls;
providers.expn_that_defined = |tcx, id| {
let id = id.expect_local();
tcx.definitions.expansion_that_defined(id)
tcx.resolutions(()).definitions.expansion_that_defined(id)
};
}
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub type MetadataLoaderDyn = dyn MetadataLoader + Sync;
/// that it's *not* tracked for dependency information throughout compilation
/// (it'd break incremental compilation) and should only be called pre-HIR (e.g.
/// during resolve)
pub trait CrateStore {
pub trait CrateStore: std::fmt::Debug {
fn as_any(&self) -> &dyn Any;

// resolve
Expand Down
18 changes: 7 additions & 11 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ rustc_queries! {
desc { "trigger a delay span bug" }
}

query resolutions(_: ()) -> &'tcx ty::ResolverOutputs {
eval_always
no_hash
desc { "get the resolver outputs" }
}

/// Represents crate as a whole (as distinct from the top-level crate module).
/// If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`),
/// we will have to assume that any change means that you need to be recompiled.
Expand Down Expand Up @@ -207,7 +213,6 @@ rustc_queries! {
}

query expn_that_defined(key: DefId) -> rustc_span::ExpnId {
eval_always
desc { |tcx| "expansion that defined `{}`", tcx.def_path_str(key) }
}

Expand Down Expand Up @@ -1129,7 +1134,6 @@ rustc_queries! {

query module_exports(def_id: LocalDefId) -> Option<&'tcx [Export<LocalDefId>]> {
desc { |tcx| "looking up items exported by `{}`", tcx.def_path_str(def_id.to_def_id()) }
eval_always
}

query impl_defaultness(def_id: DefId) -> hir::Defaultness {
Expand Down Expand Up @@ -1319,7 +1323,6 @@ rustc_queries! {
}

query visibility(def_id: DefId) -> ty::Visibility {
eval_always
desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
}

Expand All @@ -1344,8 +1347,6 @@ rustc_queries! {
desc { |tcx| "collecting child items of `{}`", tcx.def_path_str(def_id) }
}
query extern_mod_stmt_cnum(def_id: LocalDefId) -> Option<CrateNum> {
// This depends on untracked global state (`tcx.extern_crate_map`)
eval_always
desc { |tcx| "computing crate imported by `{}`", tcx.def_path_str(def_id.to_def_id()) }
}

Expand Down Expand Up @@ -1422,16 +1423,12 @@ rustc_queries! {
eval_always
}
query maybe_unused_trait_import(def_id: LocalDefId) -> bool {
eval_always
desc { |tcx| "maybe_unused_trait_import for `{}`", tcx.def_path_str(def_id.to_def_id()) }
}
query maybe_unused_extern_crates(_: ()) -> &'tcx [(LocalDefId, Span)] {
eval_always
desc { "looking up all possibly unused extern crates" }
}
query names_imported_by_glob_use(def_id: LocalDefId)
-> &'tcx FxHashSet<Symbol> {
eval_always
query names_imported_by_glob_use(def_id: LocalDefId) -> &'tcx FxHashSet<Symbol> {
desc { |tcx| "names_imported_by_glob_use for `{}`", tcx.def_path_str(def_id.to_def_id()) }
}

Expand All @@ -1441,7 +1438,6 @@ rustc_queries! {
desc { "calculating the stability index for the local crate" }
}
query crates(_: ()) -> &'tcx [CrateNum] {
eval_always
desc { "fetching all foreign CrateNum instances" }
}

Expand Down
Loading