Skip to content

Commit 6bd80d1

Browse files
committed
compute and cache HIR hashes at beginning
This avoids the compile-time overhead of computing them twice. It also fixes an issue where the hash computed after typeck is differen than the hash before, because typeck mutates the def-map in place. Fixes #35549. Fixes #35593.
1 parent 413ada3 commit 6bd80d1

File tree

13 files changed

+182
-157
lines changed

13 files changed

+182
-157
lines changed

src/librustc/hir/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1621,7 +1621,7 @@ pub type FreevarMap = NodeMap<Vec<Freevar>>;
16211621

16221622
pub type CaptureModeMap = NodeMap<CaptureClause>;
16231623

1624-
#[derive(Clone)]
1624+
#[derive(Clone, Debug)]
16251625
pub struct TraitCandidate {
16261626
pub def_id: DefId,
16271627
pub import_id: Option<NodeId>,

src/librustc_driver/driver.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use rustc::util::common::time;
2626
use rustc::util::nodemap::NodeSet;
2727
use rustc_back::sha2::{Sha256, Digest};
2828
use rustc_borrowck as borrowck;
29-
use rustc_incremental;
29+
use rustc_incremental::{self, HashesMap};
3030
use rustc_resolve::{MakeGlobMap, Resolver};
3131
use rustc_metadata::macro_import;
3232
use rustc_metadata::creader::read_local_crates;
@@ -172,7 +172,7 @@ pub fn compile_input(sess: &Session,
172172
resolutions,
173173
&arenas,
174174
&crate_name,
175-
|tcx, mir_map, analysis, result| {
175+
|tcx, mir_map, analysis, hashes_map, result| {
176176
{
177177
// Eventually, we will want to track plugins.
178178
let _ignore = tcx.dep_graph.in_ignore();
@@ -202,7 +202,8 @@ pub fn compile_input(sess: &Session,
202202
}
203203
let trans = phase_4_translate_to_llvm(tcx,
204204
mir_map.unwrap(),
205-
analysis);
205+
analysis,
206+
&hashes_map);
206207

207208
if log_enabled!(::log::INFO) {
208209
println!("Post-trans");
@@ -797,14 +798,15 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
797798
where F: for<'a> FnOnce(TyCtxt<'a, 'tcx, 'tcx>,
798799
Option<MirMap<'tcx>>,
799800
ty::CrateAnalysis,
801+
HashesMap,
800802
CompileResult) -> R
801803
{
802804
macro_rules! try_with_f {
803-
($e: expr, ($t: expr, $m: expr, $a: expr)) => {
805+
($e: expr, ($t: expr, $m: expr, $a: expr, $h: expr)) => {
804806
match $e {
805807
Ok(x) => x,
806808
Err(x) => {
807-
f($t, $m, $a, Err(x));
809+
f($t, $m, $a, $h, Err(x));
808810
return Err(x);
809811
}
810812
}
@@ -860,12 +862,16 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
860862
index,
861863
name,
862864
|tcx| {
865+
let hashes_map =
866+
time(time_passes,
867+
"compute_hashes_map",
868+
|| rustc_incremental::compute_hashes_map(tcx));
863869
time(time_passes,
864870
"load_dep_graph",
865-
|| rustc_incremental::load_dep_graph(tcx));
871+
|| rustc_incremental::load_dep_graph(tcx, &hashes_map));
866872

867873
// passes are timed inside typeck
868-
try_with_f!(typeck::check_crate(tcx), (tcx, None, analysis));
874+
try_with_f!(typeck::check_crate(tcx), (tcx, None, analysis, hashes_map));
869875

870876
time(time_passes,
871877
"const checking",
@@ -935,7 +941,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
935941
// lint warnings and so on -- kindck used to do this abort, but
936942
// kindck is gone now). -nmatsakis
937943
if sess.err_count() > 0 {
938-
return Ok(f(tcx, Some(mir_map), analysis, Err(sess.err_count())));
944+
return Ok(f(tcx, Some(mir_map), analysis, hashes_map, Err(sess.err_count())));
939945
}
940946

941947
analysis.reachable =
@@ -963,17 +969,18 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
963969

964970
// The above three passes generate errors w/o aborting
965971
if sess.err_count() > 0 {
966-
return Ok(f(tcx, Some(mir_map), analysis, Err(sess.err_count())));
972+
return Ok(f(tcx, Some(mir_map), analysis, hashes_map, Err(sess.err_count())));
967973
}
968974

969-
Ok(f(tcx, Some(mir_map), analysis, Ok(())))
975+
Ok(f(tcx, Some(mir_map), analysis, hashes_map, Ok(())))
970976
})
971977
}
972978

973979
/// Run the translation phase to LLVM, after which the AST and analysis can
974980
pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
975981
mut mir_map: MirMap<'tcx>,
976-
analysis: ty::CrateAnalysis)
982+
analysis: ty::CrateAnalysis,
983+
hashes_map: &HashesMap)
977984
-> trans::CrateTranslation {
978985
let time_passes = tcx.sess.time_passes();
979986

@@ -1007,15 +1014,15 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
10071014
let translation =
10081015
time(time_passes,
10091016
"translation",
1010-
move || trans::trans_crate(tcx, &mir_map, analysis));
1017+
move || trans::trans_crate(tcx, &mir_map, analysis, &hashes_map));
10111018

10121019
time(time_passes,
10131020
"assert dep graph",
10141021
move || rustc_incremental::assert_dep_graph(tcx));
10151022

10161023
time(time_passes,
10171024
"serialize dep graph",
1018-
move || rustc_incremental::save_dep_graph(tcx));
1025+
move || rustc_incremental::save_dep_graph(tcx, &hashes_map));
10191026

10201027
translation
10211028
}

src/librustc_driver/pretty.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ impl PpSourceMode {
234234
resolutions.clone(),
235235
arenas,
236236
id,
237-
|tcx, _, _, _| {
237+
|tcx, _, _, _, _| {
238238
let annotation = TypedAnnotation {
239239
tcx: tcx,
240240
};
@@ -951,7 +951,7 @@ fn print_with_analysis<'tcx, 'a: 'tcx>(sess: &'a Session,
951951
resolutions.clone(),
952952
arenas,
953953
crate_name,
954-
|tcx, mir_map, _, _| {
954+
|tcx, mir_map, _, _, _| {
955955
match ppm {
956956
PpmMir | PpmMirCFG => {
957957
if let Some(mir_map) = mir_map {

src/librustc_incremental/calculate_svh/mod.rs

Lines changed: 88 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -8,106 +8,122 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
//! Calculation of a Strict Version Hash for crates. For a length
12-
//! comment explaining the general idea, see `librustc/middle/svh.rs`.
13-
11+
//! Calculation of the (misnamed) "strict version hash" for crates and
12+
//! items. This hash is used to tell when the HIR changed in such a
13+
//! way that results from previous compilations may no longer be
14+
//! applicable and hence must be recomputed. It should probably be
15+
//! renamed to the ICH (incremental compilation hash).
16+
//!
17+
//! The hashes for all items are computed once at the beginning of
18+
//! compilation and stored into a map. In addition, a hash is computed
19+
//! of the **entire crate**.
20+
//!
21+
//! Storing the hashes in a map avoids the need to compute them twice
22+
//! (once when loading prior incremental results and once when
23+
//! saving), but it is also important for correctness: at least as of
24+
//! the time of this writing, the typeck passes rewrites entries in
25+
//! the dep-map in-place to accommodate UFCS resolutions. Since name
26+
//! resolution is part of the hash, the result is that hashes computed
27+
//! at the end of compilation would be different from those computed
28+
//! at the beginning.
29+
30+
use syntax::ast;
1431
use syntax::attr::AttributeMethods;
1532
use std::hash::{Hash, SipHasher, Hasher};
33+
use rustc::dep_graph::DepNode;
34+
use rustc::hir;
1635
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
17-
use rustc::hir::map::{NodeItem, NodeForeignItem};
18-
use rustc::hir::svh::Svh;
36+
use rustc::hir::intravisit as visit;
1937
use rustc::ty::TyCtxt;
20-
use rustc::hir::intravisit::{self, Visitor};
38+
use rustc_data_structures::fnv::FnvHashMap;
2139

2240
use self::svh_visitor::StrictVersionHashVisitor;
2341

2442
mod svh_visitor;
2543

26-
pub trait SvhCalculate {
27-
/// Calculate the SVH for an entire krate.
28-
fn calculate_krate_hash(self) -> Svh;
44+
pub type HashesMap = FnvHashMap<DepNode<DefId>, u64>;
2945

30-
/// Calculate the SVH for a particular item.
31-
fn calculate_item_hash(self, def_id: DefId) -> u64;
46+
pub fn compute_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> HashesMap {
47+
let _ignore = tcx.dep_graph.in_ignore();
48+
let krate = tcx.map.krate();
49+
let mut visitor = HashItemsVisitor { tcx: tcx, hashes: FnvHashMap() };
50+
visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), |v| visit::walk_crate(v, krate));
51+
krate.visit_all_items(&mut visitor);
52+
visitor.compute_crate_hash();
53+
visitor.hashes
3254
}
3355

34-
impl<'a, 'tcx> SvhCalculate for TyCtxt<'a, 'tcx, 'tcx> {
35-
fn calculate_krate_hash(self) -> Svh {
36-
// FIXME (#14132): This is better than it used to be, but it still not
37-
// ideal. We now attempt to hash only the relevant portions of the
38-
// Crate AST as well as the top-level crate attributes. (However,
39-
// the hashing of the crate attributes should be double-checked
40-
// to ensure it is not incorporating implementation artifacts into
41-
// the hash that are not otherwise visible.)
56+
struct HashItemsVisitor<'a, 'tcx: 'a> {
57+
tcx: TyCtxt<'a, 'tcx, 'tcx>,
58+
hashes: HashesMap,
59+
}
4260

43-
let crate_disambiguator = self.sess.local_crate_disambiguator();
44-
let krate = self.map.krate();
61+
impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
62+
fn calculate_node_id<W>(&mut self, id: ast::NodeId, walk_op: W)
63+
where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'tcx>)
64+
{
65+
let def_id = self.tcx.map.local_def_id(id);
66+
self.calculate_def_id(def_id, walk_op)
67+
}
4568

46-
// FIXME: this should use SHA1, not SipHash. SipHash is not built to
47-
// avoid collisions.
69+
fn calculate_def_id<W>(&mut self, def_id: DefId, mut walk_op: W)
70+
where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'tcx>)
71+
{
72+
assert!(def_id.is_local());
73+
debug!("HashItemsVisitor::calculate(def_id={:?})", def_id);
74+
// FIXME: this should use SHA1, not SipHash. SipHash is not
75+
// built to avoid collisions.
4876
let mut state = SipHasher::new();
49-
debug!("state: {:?}", state);
77+
walk_op(&mut StrictVersionHashVisitor::new(&mut state, self.tcx));
78+
let item_hash = state.finish();
79+
self.hashes.insert(DepNode::Hir(def_id), item_hash);
80+
debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, item_hash);
81+
}
82+
83+
fn compute_crate_hash(&mut self) {
84+
let krate = self.tcx.map.krate();
5085

51-
// FIXME(#32753) -- at (*) we `to_le` for endianness, but is
52-
// this enough, and does it matter anyway?
53-
"crate_disambiguator".hash(&mut state);
54-
crate_disambiguator.len().to_le().hash(&mut state); // (*)
55-
crate_disambiguator.hash(&mut state);
86+
let mut crate_state = SipHasher::new();
5687

57-
debug!("crate_disambiguator: {:?}", crate_disambiguator);
58-
debug!("state: {:?}", state);
88+
let crate_disambiguator = self.tcx.sess.local_crate_disambiguator();
89+
"crate_disambiguator".hash(&mut crate_state);
90+
crate_disambiguator.len().hash(&mut crate_state);
91+
crate_disambiguator.hash(&mut crate_state);
5992

93+
// add each item (in some deterministic order) to the overall
94+
// crate hash.
95+
//
96+
// FIXME -- it'd be better to sort by the hash of the def-path,
97+
// so that reordering items would not affect the crate hash.
6098
{
61-
let mut visit = StrictVersionHashVisitor::new(&mut state, self);
62-
krate.visit_all_items(&mut visit);
99+
let mut keys: Vec<_> = self.hashes.keys().collect();
100+
keys.sort();
101+
for key in keys {
102+
self.hashes[key].hash(&mut crate_state);
103+
}
63104
}
64105

65-
// FIXME (#14132): This hash is still sensitive to e.g. the
66-
// spans of the crate Attributes and their underlying
67-
// MetaItems; we should make ContentHashable impl for those
68-
// types and then use hash_content. But, since all crate
69-
// attributes should appear near beginning of the file, it is
70-
// not such a big deal to be sensitive to their spans for now.
71-
//
72-
// We hash only the MetaItems instead of the entire Attribute
73-
// to avoid hashing the AttrId
74106
for attr in &krate.attrs {
75107
debug!("krate attr {:?}", attr);
76-
attr.meta().hash(&mut state);
108+
attr.meta().hash(&mut crate_state);
77109
}
78110

79-
Svh::new(state.finish())
111+
let crate_hash = crate_state.finish();
112+
self.hashes.insert(DepNode::Krate, crate_hash);
113+
debug!("calculate_crate_hash: crate_hash={:?}", crate_hash);
80114
}
115+
}
81116

82-
fn calculate_item_hash(self, def_id: DefId) -> u64 {
83-
assert!(def_id.is_local());
84-
85-
debug!("calculate_item_hash(def_id={:?})", def_id);
86-
87-
let mut state = SipHasher::new();
88-
89-
{
90-
let mut visit = StrictVersionHashVisitor::new(&mut state, self);
91-
if def_id.index == CRATE_DEF_INDEX {
92-
// the crate root itself is not registered in the map
93-
// as an item, so we have to fetch it this way
94-
let krate = self.map.krate();
95-
intravisit::walk_crate(&mut visit, krate);
96-
} else {
97-
let node_id = self.map.as_local_node_id(def_id).unwrap();
98-
match self.map.find(node_id) {
99-
Some(NodeItem(item)) => visit.visit_item(item),
100-
Some(NodeForeignItem(item)) => visit.visit_foreign_item(item),
101-
r => bug!("calculate_item_hash: expected an item for node {} not {:?}",
102-
node_id, r),
103-
}
104-
}
105-
}
106-
107-
let hash = state.finish();
108117

109-
debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, hash);
118+
impl<'a, 'tcx> visit::Visitor<'tcx> for HashItemsVisitor<'a, 'tcx> {
119+
fn visit_item(&mut self, item: &'tcx hir::Item) {
120+
self.calculate_node_id(item.id, |v| v.visit_item(item));
121+
visit::walk_item(self, item);
122+
}
110123

111-
hash
124+
fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem) {
125+
self.calculate_node_id(item.id, |v| v.visit_foreign_item(item));
126+
visit::walk_foreign_item(self, item);
112127
}
113128
}
129+

0 commit comments

Comments
 (0)