Skip to content
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

[DO NOT MERGE] Revert #73291 to confirm performance improvements #73611

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
10 changes: 0 additions & 10 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ pub trait Resolver {
fn lint_buffer(&mut self) -> &mut LintBuffer;

fn next_node_id(&mut self) -> NodeId;

fn trait_map(&self) -> &NodeMap<Vec<hir::TraitCandidate>>;
}

type NtToTokenstream = fn(&Nonterminal, &ParseSess, Span) -> TokenStream;
Expand Down Expand Up @@ -557,13 +555,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let proc_macros =
c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id].unwrap()).collect();

let trait_map = self
.resolver
.trait_map()
.iter()
.map(|(&k, v)| (self.node_id_to_hir_id[k].unwrap(), v.clone()))
.collect();

self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id);

hir::Crate {
Expand All @@ -578,7 +569,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
trait_impls: self.trait_impls,
modules: self.modules,
proc_macros,
trait_map,
}
}

Expand Down
61 changes: 37 additions & 24 deletions src/librustc_hir/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,14 @@ pub struct Definitions {

def_id_to_span: IndexVec<LocalDefId, Span>,

// FIXME(eddyb) don't go through `ast::NodeId` to convert between `HirId`
// and `LocalDefId` - ideally all `LocalDefId`s would be HIR owners.
node_id_to_def_id: FxHashMap<ast::NodeId, LocalDefId>,
def_id_to_node_id: IndexVec<LocalDefId, ast::NodeId>,

// FIXME(eddyb) ideally all `LocalDefId`s would be HIR owners.
pub(super) def_id_to_hir_id: IndexVec<LocalDefId, Option<hir::HirId>>,
/// The reverse mapping of `def_id_to_hir_id`.
pub(super) hir_id_to_def_id: FxHashMap<hir::HirId, LocalDefId>,
pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, Option<hir::HirId>>,
/// The reverse mapping of `node_id_to_hir_id`.
pub(super) hir_id_to_node_id: FxHashMap<hir::HirId, ast::NodeId>,

/// If `ExpnId` is an ID of some macro expansion,
/// then `DefId` is the normal module (`mod`) in which the expanded macro was defined.
Expand Down Expand Up @@ -326,27 +327,47 @@ impl Definitions {

#[inline]
pub fn local_def_id(&self, node: ast::NodeId) -> LocalDefId {
self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node))
self.opt_local_def_id(node).unwrap_or_else(|| {
panic!("no entry for node id: `{:?}` / `{:?}`", node, self.opt_node_id_to_hir_id(node))
})
}

#[inline]
pub fn as_local_hir_id(&self, def_id: LocalDefId) -> hir::HirId {
self.local_def_id_to_hir_id(def_id)
}

#[inline]
pub fn hir_id_to_node_id(&self, hir_id: hir::HirId) -> ast::NodeId {
self.hir_id_to_node_id[&hir_id]
}

#[inline]
pub fn node_id_to_hir_id(&self, node_id: ast::NodeId) -> hir::HirId {
self.node_id_to_hir_id[node_id].unwrap()
}

#[inline]
pub fn opt_node_id_to_hir_id(&self, node_id: ast::NodeId) -> Option<hir::HirId> {
self.node_id_to_hir_id[node_id]
}

#[inline]
pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId {
self.def_id_to_hir_id[id].unwrap()
let node_id = self.def_id_to_node_id[id];
self.node_id_to_hir_id[node_id].unwrap()
}

#[inline]
pub fn opt_local_def_id_to_hir_id(&self, id: LocalDefId) -> Option<hir::HirId> {
self.def_id_to_hir_id[id]
let node_id = self.def_id_to_node_id[id];
self.node_id_to_hir_id[node_id]
}

#[inline]
pub fn opt_hir_id_to_local_def_id(&self, hir_id: hir::HirId) -> Option<LocalDefId> {
self.hir_id_to_def_id.get(&hir_id).copied()
let node_id = self.hir_id_to_node_id(hir_id);
self.opt_local_def_id(node_id)
}

/// Retrieves the span of the given `DefId` if `DefId` is in the local crate.
Expand Down Expand Up @@ -456,24 +477,16 @@ impl Definitions {
mapping: IndexVec<ast::NodeId, Option<hir::HirId>>,
) {
assert!(
self.def_id_to_hir_id.is_empty(),
"trying to initialize `LocalDefId` <-> `HirId` mappings twice"
self.node_id_to_hir_id.is_empty(),
"trying to initialize `NodeId` -> `HirId` mapping twice"
);
self.node_id_to_hir_id = mapping;

self.def_id_to_hir_id = self
.def_id_to_node_id
.iter()
.map(|&node_id| mapping.get(node_id).and_then(|&hir_id| hir_id))
.collect();

// Build the reverse mapping of `def_id_to_hir_id`.
self.hir_id_to_def_id = mapping
.into_iter_enumerated()
.filter_map(|(node_id, hir_id)| {
hir_id.and_then(|hir_id| {
self.node_id_to_def_id.get(&node_id).map(|&def_id| (hir_id, def_id))
})
})
// Build the reverse mapping of `node_id_to_hir_id`.
self.hir_id_to_node_id = self
.node_id_to_hir_id
.iter_enumerated()
.filter_map(|(node_id, &hir_id)| hir_id.map(|hir_id| (hir_id, node_id)))
.collect();
}

Expand Down
4 changes: 1 addition & 3 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,6 @@ pub struct Crate<'hir> {
/// A list of proc macro HirIds, written out in the order in which
/// they are declared in the static array generated by proc_macro_harness.
pub proc_macros: Vec<HirId>,

pub trait_map: BTreeMap<HirId, Vec<TraitCandidate>>,
}

impl Crate<'hir> {
Expand Down Expand Up @@ -2653,7 +2651,7 @@ pub type CaptureModeMap = NodeMap<CaptureBy>;
// The TraitCandidate's import_ids is empty if the trait is defined in the same module, and
// has length > 0 if the trait is found through an chain of imports, starting with the
// import/use statement in the scope where the trait is used.
#[derive(RustcEncodable, RustcDecodable, Clone, Debug)]
#[derive(Clone, Debug)]
pub struct TraitCandidate {
pub def_id: DefId,
pub import_ids: SmallVec<[LocalDefId; 1]>,
Expand Down
18 changes: 13 additions & 5 deletions src/librustc_middle/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
body_ids: _,
modules: _,
proc_macros: _,
trait_map: _,
} = *krate;

hash_body(&mut hcx, root_mod_def_path_hash, item, &mut hir_body_nodes)
Expand Down Expand Up @@ -242,8 +241,11 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
// Make sure that the DepNode of some node coincides with the HirId
// owner of that node.
if cfg!(debug_assertions) {
let node_id = self.definitions.hir_id_to_node_id(hir_id);
assert_eq!(self.definitions.node_id_to_hir_id(node_id), hir_id);

if hir_id.owner != self.current_dep_node_owner {
let node_str = match self.definitions.opt_hir_id_to_local_def_id(hir_id) {
let node_str = match self.definitions.opt_local_def_id(node_id) {
Some(def_id) => self.definitions.def_path(def_id).to_string_no_crate(),
None => format!("{:?}", node),
};
Expand Down Expand Up @@ -333,7 +335,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
debug!("visit_item: {:?}", i);
debug_assert_eq!(
i.hir_id.owner,
self.definitions.opt_hir_id_to_local_def_id(i.hir_id).unwrap()
self.definitions
.opt_local_def_id(self.definitions.hir_id_to_node_id(i.hir_id))
.unwrap()
);
self.with_dep_node_owner(i.hir_id.owner, i, |this, hash| {
this.insert_with_hash(i.span, i.hir_id, Node::Item(i), hash);
Expand Down Expand Up @@ -365,7 +369,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
fn visit_trait_item(&mut self, ti: &'hir TraitItem<'hir>) {
debug_assert_eq!(
ti.hir_id.owner,
self.definitions.opt_hir_id_to_local_def_id(ti.hir_id).unwrap()
self.definitions
.opt_local_def_id(self.definitions.hir_id_to_node_id(ti.hir_id))
.unwrap()
);
self.with_dep_node_owner(ti.hir_id.owner, ti, |this, hash| {
this.insert_with_hash(ti.span, ti.hir_id, Node::TraitItem(ti), hash);
Expand All @@ -379,7 +385,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
fn visit_impl_item(&mut self, ii: &'hir ImplItem<'hir>) {
debug_assert_eq!(
ii.hir_id.owner,
self.definitions.opt_hir_id_to_local_def_id(ii.hir_id).unwrap()
self.definitions
.opt_local_def_id(self.definitions.hir_id_to_node_id(ii.hir_id))
.unwrap()
);
self.with_dep_node_owner(ii.hir_id.owner, ii, |this, hash| {
this.insert_with_hash(ii.span, ii.hir_id, Node::ImplItem(ii), hash);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_middle/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,9 +1120,9 @@ impl<'tcx> TyCtxt<'tcx> {
};

let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default();
for (hir_id, v) in krate.trait_map.iter() {
for (hir_id, v) in resolutions.trait_map.into_iter() {
let map = trait_map.entry(hir_id.owner).or_default();
map.insert(hir_id.local_id, StableVec::new(v.to_vec()));
map.insert(hir_id.local_id, StableVec::new(v));
}

GlobalCtxt {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ pub struct ResolverOutputs {
pub definitions: rustc_hir::definitions::Definitions,
pub cstore: Box<CrateStoreDyn>,
pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
pub trait_map: FxHashMap<hir::HirId, Vec<hir::TraitCandidate>>,
pub maybe_unused_trait_imports: FxHashSet<LocalDefId>,
pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
pub export_map: ExportMap<LocalDefId>,
Expand Down
15 changes: 11 additions & 4 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,10 +1109,6 @@ impl rustc_ast_lowering::Resolver for Resolver<'_> {
fn next_node_id(&mut self) -> NodeId {
self.next_node_id()
}

fn trait_map(&self) -> &NodeMap<Vec<TraitCandidate>> {
&self.trait_map
}
}

impl<'a> Resolver<'a> {
Expand Down Expand Up @@ -1288,6 +1284,11 @@ impl<'a> Resolver<'a> {
let definitions = self.definitions;
let extern_crate_map = self.extern_crate_map;
let export_map = self.export_map;
let trait_map = self
.trait_map
.into_iter()
.map(|(k, v)| (definitions.node_id_to_hir_id(k), v))
.collect();
let maybe_unused_trait_imports = self.maybe_unused_trait_imports;
let maybe_unused_extern_crates = self.maybe_unused_extern_crates;
let glob_map = self.glob_map;
Expand All @@ -1296,6 +1297,7 @@ impl<'a> Resolver<'a> {
cstore: Box::new(self.crate_loader.into_cstore()),
extern_crate_map,
export_map,
trait_map,
glob_map,
maybe_unused_trait_imports,
maybe_unused_extern_crates,
Expand All @@ -1313,6 +1315,11 @@ impl<'a> Resolver<'a> {
cstore: Box::new(self.cstore().clone()),
extern_crate_map: self.extern_crate_map.clone(),
export_map: self.export_map.clone(),
trait_map: self
.trait_map
.iter()
.map(|(&k, v)| (self.definitions.node_id_to_hir_id(k), v.clone()))
.collect(),
glob_map: self.glob_map.clone(),
maybe_unused_trait_imports: self.maybe_unused_trait_imports.clone(),
maybe_unused_extern_crates: self.maybe_unused_extern_crates.clone(),
Expand Down